diff --git a/ui/src/e2e/chat-reply-preview-recovery.e2e.test.ts b/ui/src/e2e/chat-reply-preview-recovery.e2e.test.ts new file mode 100644 index 000000000000..93d6200ff598 --- /dev/null +++ b/ui/src/e2e/chat-reply-preview-recovery.e2e.test.ts @@ -0,0 +1,204 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { expect, it } from "vitest"; +import { + createChatFlowE2eSuite, + expectRequestCountStable, + installMockGateway, + waitForRequests, +} from "./chat-flow.test-support.ts"; + +const suite = createChatFlowE2eSuite(); + +suite.define(() => { + it.each([ + { + name: "temporary lookup failure", + artifact: "temporary-failure", + response: { + __mockError: { + code: "UNAVAILABLE", + message: "Session transcript projection is rebuilding: reply-preview-session", + }, + }, + }, + { + name: "permanently unavailable source", + artifact: "unavailable-source", + response: { ok: false, unavailableReason: "not_found" }, + }, + ])( + "settles a $name without repeatedly loading the reply preview", + async ({ artifact, response }) => { + const artifactDir = process.env.OPENCLAW_UI_E2E_ARTIFACT_DIR?.trim(); + if (artifactDir) { + await mkdir(artifactDir, { recursive: true }); + } + const context = await suite.newBrowserContext({ + locale: "en-US", + ...(artifactDir + ? { recordVideo: { dir: artifactDir, size: { height: 900, width: 1280 } } } + : {}), + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + const gateway = await installMockGateway(page, { + historyMessages: [ + { + role: "user", + content: [{ type: "text", text: "Follow up on the earlier answer." }], + timestamp: 1_800_000_000_000, + __openclaw: { id: "reply-message", seq: 101, replyToId: "older-answer" }, + }, + ], + methodResponses: { "chat.message.get": response }, + sessionKey: "agent:main:reply-preview", + }); + + let firstCount = 0; + try { + await page.goto(`${suite.server.baseUrl}chat`); + const reply = page.locator(".chat-pane-cache__pane--active .chat-reply-preview--message"); + await reply.waitFor({ state: "visible" }); + expect(await reply.textContent()).toContain("Replying to message"); + await gateway.waitForRequest("chat.message.get"); + const composer = page.locator( + ".chat-pane-cache__pane--active .agent-chat__composer-combobox textarea", + ); + await composer.fill("This draft remains usable."); + expect(await composer.inputValue()).toBe("This draft remains usable."); + firstCount = (await gateway.getRequests("chat.message.get")).length; + await expectRequestCountStable(gateway, "chat.message.get", 1); + if (artifact === "unavailable-source") { + await reply.click(); + await page + .locator(".chat-pane-cache__pane--active") + .getByRole("alert") + .locator("summary") + .getByText("The original message is unavailable.", { exact: true }) + .waitFor(); + await expectRequestCountStable(gateway, "chat.message.get", 1); + } + } finally { + if (artifactDir) { + await page.screenshot({ + animations: "disabled", + path: path.join(artifactDir, `${artifact}.png`), + }); + await writeFile( + path.join(artifactDir, `${artifact}.json`), + JSON.stringify( + { + firstCount, + finalCount: (await gateway.getRequests("chat.message.get")).length, + replyText: await page.locator(".chat-reply-preview--message").allTextContents(), + }, + null, + 2, + ), + ); + } + await suite.closeBrowserContext(context); + } + }, + ); + + it.each(["temporary failure", "previous success", "not found"] as const)( + "refreshes a reply preview after reconnect from $0 and keeps source navigation working", + async (initial) => { + const context = await suite.newBrowserContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + const source = { + role: "assistant", + content: [{ type: "text", text: "The current original answer." }], + timestamp: 1_800_000_000_000, + __openclaw: { id: "reconnect-source", seq: 1 }, + }; + const reply = { + role: "user", + content: [{ type: "text", text: "A follow-up question after reconnect." }], + timestamp: 1_800_000_000_017, + __openclaw: { id: "reconnect-reply", seq: 17, replyToId: "reconnect-source" }, + }; + const messages = [ + ...Array.from({ length: 15 }, (_, index) => ({ + role: index % 2 === 0 ? "assistant" : "user", + content: [{ type: "text", text: `Conversation entry ${index + 2}.` }], + timestamp: 1_800_000_000_001 + index, + __openclaw: { id: `intervening-${index}`, seq: index + 2 }, + })), + reply, + ]; + const gateway = await installMockGateway(page, { + historyMessages: messages, + methodResponses: { + "chat.message.get": + initial === "temporary failure" + ? { + __mockError: { + code: "UNAVAILABLE", + message: "Transcript temporarily unavailable", + }, + } + : initial === "not found" + ? { ok: false, unavailableReason: "not_found" } + : { + ok: true, + message: { ...source, content: [{ type: "text", text: "Previous preview." }] }, + }, + "chat.history": { + cases: [ + { + match: { offset: messages.length }, + response: { + messages: [source], + hasMore: false, + totalMessages: 17, + sessionId: "reply-preview-history", + }, + }, + ], + }, + "chat.startup": { + messages, + hasMore: true, + nextOffset: messages.length, + totalMessages: 17, + sessionId: "reply-preview-history", + }, + }, + sessionKey: "agent:main:reply-reconnect", + }); + + try { + await page.goto(`${suite.server.baseUrl}chat`); + const preview = page.locator(".chat-pane-cache__pane--active .chat-reply-preview--message"); + await preview.waitFor(); + await gateway.waitForRequest("chat.message.get"); + await expectRequestCountStable(gateway, "chat.message.get", 1); + if (initial === "previous success") { + expect(await preview.textContent()).toContain("Previous preview."); + } + await gateway.setMethodResponse("chat.message.get", { ok: true, message: source }); + const connectCount = (await gateway.getRequests("connect")).length; + await gateway.closeLatest(1006, "reply preview recovery"); + await waitForRequests(gateway, "connect", connectCount + 1); + await expect.poll(() => preview.textContent()).toContain("The current original answer."); + await expectRequestCountStable(gateway, "chat.message.get", 2); + + await preview.click(); + await page + .locator(".chat-pane-cache__pane--active .chat-text") + .getByText("The current original answer.", { exact: true }) + .waitFor(); + } finally { + await suite.closeBrowserContext(context); + } + }, + ); +}); diff --git a/ui/src/pages/chat/chat-pane-history.test.ts b/ui/src/pages/chat/chat-pane-history.test.ts index d4ca7baad0fa..5afda517ffb0 100644 --- a/ui/src/pages/chat/chat-pane-history.test.ts +++ b/ui/src/pages/chat/chat-pane-history.test.ts @@ -178,6 +178,87 @@ describe("chat pane native history pagination", () => { }); }); + it.each(["reconnect", "replacement client"] as const)( + "retires a resolved reply preview after %s", + async (transition) => { + const oldMessage = { role: "assistant", content: "Previous connection's answer" }; + const newMessage = { role: "assistant", content: "Current connection's answer" }; + const request = vi + .fn() + .mockResolvedValueOnce({ ok: true, message: oldMessage }) + .mockResolvedValueOnce({ ok: true, message: newMessage }); + const client = { request } as unknown as GatewayBrowserClient; + const { pane, state } = createTestChatPane({ client, sessions: {} as SessionCapability }); + + pane.requestReplyMessage("source-message"); + await vi.waitFor(() => expect(pane.readReplyMessage("source-message")).toBe(oldMessage)); + if (transition === "reconnect") { + pane.connectionGeneration += 1; + state.connectionEpoch = pane.connectionGeneration; + } else { + const replacement = { request } as unknown as GatewayBrowserClient; + state.client = replacement; + pane.connectedClient = replacement; + pane.context.gateway.snapshot.client = replacement; + } + + expect(pane.readReplyMessage("source-message")).toBeUndefined(); + pane.requestReplyMessage("source-message"); + await vi.waitFor(() => expect(pane.readReplyMessage("source-message")).toBe(newMessage)); + expect(request).toHaveBeenCalledTimes(2); + }, + ); + + it.each(["success", "failure"] as const)( + "ignores an obsolete reply lookup %s while a reconnected lookup is pending", + async (outcome) => { + const stale = createDeferred<{ ok: true; message: unknown }>(); + const fresh = createDeferred<{ ok: true; message: unknown }>(); + const currentMessage = { role: "assistant", content: "Current answer" }; + const request = vi.fn().mockReturnValueOnce(stale.promise).mockReturnValueOnce(fresh.promise); + const client = { request } as unknown as GatewayBrowserClient; + const { pane, state } = createTestChatPane({ client, sessions: {} as SessionCapability }); + + pane.requestReplyMessage("source-message"); + pane.connectionGeneration += 1; + state.connectionEpoch = pane.connectionGeneration; + pane.requestReplyMessage("source-message"); + expect(request).toHaveBeenCalledTimes(2); + if (outcome === "success") { + stale.resolve({ ok: true, message: { role: "assistant", content: "Obsolete answer" } }); + } else { + stale.reject(new Error("Previous connection unavailable")); + } + await stale.promise.catch(() => {}); + expect(pane.readReplyMessage("source-message")).toBeUndefined(); + fresh.resolve({ ok: true, message: currentMessage }); + await vi.waitFor(() => expect(pane.readReplyMessage("source-message")).toBe(currentMessage)); + pane.requestReplyMessage("source-message"); + expect(request).toHaveBeenCalledTimes(2); + }, + ); + + it("retries a previously unavailable reply source after reconnect", async () => { + const message = { role: "assistant", content: "Source is available again" }; + const request = vi + .fn() + .mockResolvedValueOnce({ ok: false, unavailableReason: "not_found" }) + .mockResolvedValueOnce({ ok: true, message }); + const client = { request } as unknown as GatewayBrowserClient; + const { pane, state } = createTestChatPane({ client, sessions: {} as SessionCapability }); + + pane.requestReplyMessage("source-message"); + await Promise.resolve(); + pane.requestReplyMessage("source-message"); + expect(request).toHaveBeenCalledOnce(); + pane.connectionGeneration += 1; + state.connectionEpoch = pane.connectionGeneration; + pane.requestReplyMessage("source-message"); + + await vi.waitFor(() => expect(pane.readReplyMessage("source-message")).toBe(message)); + expect(request).toHaveBeenCalledTimes(2); + }); + it("pages backward until a clicked reply target is loaded, then reveals it", async () => { const target = { ...nativeHistoryMessage(1, "Original answer"), diff --git a/ui/src/pages/chat/chat-pane-reply-navigation.ts b/ui/src/pages/chat/chat-pane-reply-navigation.ts index 13eb290dcae7..e12049613988 100644 --- a/ui/src/pages/chat/chat-pane-reply-navigation.ts +++ b/ui/src/pages/chat/chat-pane-reply-navigation.ts @@ -14,7 +14,7 @@ export abstract class ChatPaneReplyNavigation extends ChatPaneSession { protected replyMessageRevision = 0; private readonly replyMessages = new Map< string, - { client: object; settled?: boolean; message?: unknown } + { client: object; generation: number; message?: unknown } >(); protected abstract loadOlderMessages(): Promise; @@ -24,7 +24,10 @@ export abstract class ChatPaneReplyNavigation extends ChatPaneSession { if (!state) { return undefined; } - return this.replyMessages.get(this.replyMessageCacheKey(state.sessionKey, messageId))?.message; + const cached = this.replyMessages.get(this.replyMessageCacheKey(state.sessionKey, messageId)); + return cached?.client === state.client && cached.generation === this.connectionGeneration + ? cached.message + : undefined; }; protected readonly requestReplyMessage = (messageId: string): void => { @@ -50,42 +53,36 @@ export abstract class ChatPaneReplyNavigation extends ChatPaneSession { const agentId = scopedAgentParamsForSession(scope.state, sessionKey).agentId; const cacheKey = this.replyMessageCacheKey(sessionKey, messageId); const cached = this.replyMessages.get(cacheKey); - if (cached && (cached.client === scope.client || cached.settled)) { + if (cached?.client === scope.client && cached.generation === scope.generation) { return; } while (this.replyMessages.size >= 256) { this.replyMessages.delete(this.replyMessages.keys().next().value!); } - this.replyMessages.set(cacheKey, { client: scope.client }); + const attempt = { client: scope.client, generation: scope.generation }; + this.replyMessages.set(cacheKey, attempt); + let result: ChatMessageGetResult; try { - const result = await scope.client.request("chat.message.get", { + result = await scope.client.request("chat.message.get", { sessionKey, ...(agentId ? { agentId } : {}), messageId, maxChars: 500, }); - const pending = this.replyMessages.get(cacheKey); - if (pending?.client !== scope.client || pending.settled) { - return; - } - this.replyMessages.set( - cacheKey, - result.ok && result.message - ? { client: scope.client, settled: true, message: result.message } - : { client: scope.client, settled: true }, - ); } catch { - const pending = this.replyMessages.get(cacheKey); - if (pending?.client !== scope.client || pending.settled) { - return; - } - this.replyMessages.delete(cacheKey); + // Retain the failed attempt so rendering cannot retry it in a loop. + // A new logical connection owns a fresh attempt, even with the same client. + return; } + if (!this.isConnectionScopeCurrent(scope) || this.replyMessages.get(cacheKey) !== attempt) { + return; + } + if (!result.ok || !result.message) { + return; + } + this.replyMessages.set(cacheKey, { ...attempt, message: result.message }); this.replyMessageRevision += 1; - if ( - this.isConnectionScopeCurrent(scope) && - areUiSessionKeysEquivalent(scope.state.sessionKey, sessionKey) - ) { + if (areUiSessionKeysEquivalent(scope.state.sessionKey, sessionKey)) { this.requestUpdate(); } }