fix(ui): keep loading attachments in their original chat (#114568)

* fix(ui): keep pending attachments in their chat session

* test(ui): preserve native attachment-reader hooks
This commit is contained in:
Peter Steinberger
2026-07-27 08:52:08 -04:00
committed by GitHub
parent a01319b27f
commit 98490b66ec
10 changed files with 445 additions and 27 deletions

View File

@@ -0,0 +1,178 @@
import { chromium, type Browser, type Locator, type Page } from "playwright";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import {
canRunPlaywrightChromium,
controlUiSessionUrl,
installMockGateway,
resolvePlaywrightChromiumExecutablePath,
startControlUiE2eServer,
type ControlUiE2eServer,
} from "../test-helpers/control-ui-e2e.ts";
const chromiumExecutablePath = resolvePlaywrightChromiumExecutablePath(chromium.executablePath());
const chromiumAvailable = canRunPlaywrightChromium(chromiumExecutablePath);
const allowMissingChromium = process.env.OPENCLAW_UI_E2E_ALLOW_MISSING_CHROMIUM === "1";
const describeControlUiE2e = chromiumAvailable || !allowMissingChromium ? describe : describe.skip;
const ONE_PIXEL_PNG_B64 =
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/woAAn8B9FD5fHAAAAAASUVORK5CYII=";
type DeferredAttachmentProof = {
aborts: number;
finish: (() => void) | undefined;
};
async function installDeferredAttachmentReader(page: Page): Promise<void> {
await page.addInitScript(() => {
const proof = { aborts: 0, finish: undefined as (() => void) | undefined };
(globalThis as unknown as { attachmentReadProof: typeof proof }).attachmentReadProof = proof;
// Keep the native methods before overriding them so deferred completion and
// cancellation cannot recursively call their own test hooks.
const readAsDataURL = Reflect.get(
FileReader.prototype,
"readAsDataURL",
) as FileReader["readAsDataURL"];
const abort = Reflect.get(FileReader.prototype, "abort") as FileReader["abort"];
FileReader.prototype.readAsDataURL = function (blob: Blob) {
proof.finish = () => readAsDataURL.call(this, blob);
};
FileReader.prototype.abort = function () {
proof.aborts += 1;
return abort.call(this);
};
});
}
async function pastePng(composer: Locator): Promise<void> {
await composer.evaluate((element, base64) => {
const bytes = Uint8Array.from(atob(base64), (char) => char.charCodeAt(0));
const clipboard = new DataTransfer();
clipboard.items.add(new File([bytes], "pixel.png", { type: "image/png" }));
element.dispatchEvent(
new ClipboardEvent("paste", { bubbles: true, cancelable: true, clipboardData: clipboard }),
);
}, ONE_PIXEL_PNG_B64);
}
let browser: Browser;
let server: ControlUiE2eServer;
describeControlUiE2e("Control UI chat attachment read lifecycle", () => {
beforeAll(async () => {
if (!chromiumAvailable) {
throw new Error(`Playwright Chromium is unavailable at ${chromiumExecutablePath}`);
}
server = await startControlUiE2eServer();
browser = await chromium.launch({ executablePath: chromiumExecutablePath });
});
afterAll(async () => {
await browser?.close();
await server?.close();
});
it("waits for a pasted image before sending its complete gateway payload", async () => {
const context = await browser.newContext({
locale: "en-US",
serviceWorkers: "block",
viewport: { height: 900, width: 1280 },
});
const page = await context.newPage();
await installDeferredAttachmentReader(page);
const gateway = await installMockGateway(page);
try {
await page.goto(`${server.baseUrl}chat`);
const composer = page.locator(".agent-chat__composer-combobox textarea");
const send = page.getByRole("button", { name: "Send message" });
await composer.fill("Include the image that is still loading");
await pastePng(composer);
await expect.poll(() => send.isDisabled()).toBe(true);
await composer.press("Enter");
expect(await gateway.getRequests("chat.send")).toHaveLength(0);
await page.evaluate(() => {
const proof = (globalThis as unknown as { attachmentReadProof: DeferredAttachmentProof })
.attachmentReadProof;
if (!proof.finish) {
throw new Error("Pasted image read was not started");
}
proof.finish();
});
await page.locator('.chat-attachment-thumb img[alt="Attachment preview"]').waitFor();
await expect.poll(() => send.isEnabled()).toBe(true);
await send.click();
const request = await gateway.waitForRequest("chat.send");
expect(request.params).toMatchObject({
attachments: [{ content: ONE_PIXEL_PNG_B64, fileName: "pixel.png", mimeType: "image/png" }],
message: "Include the image that is still loading",
});
} finally {
await context.close();
}
});
it("aborts a session's pending image before the pane adopts another session", async () => {
const firstSession = "agent:main:attachment-session-a";
const secondSession = "agent:main:attachment-session-b";
const context = await browser.newContext({
locale: "en-US",
serviceWorkers: "block",
viewport: { height: 900, width: 1280 },
});
const page = await context.newPage();
await installDeferredAttachmentReader(page);
const gateway = await installMockGateway(page, {
methodResponses: {
"sessions.list": {
count: 2,
defaults: { contextTokens: null, model: "gpt-5.5", modelProvider: "openai" },
path: "",
sessions: [
{ key: firstSession, kind: "direct", updatedAt: 2 },
{ key: secondSession, kind: "direct", updatedAt: 1 },
],
ts: Date.now(),
},
},
sessionKey: firstSession,
});
try {
await page.goto(controlUiSessionUrl(server.baseUrl, firstSession));
const composer = page.locator(".agent-chat__composer-combobox textarea");
await composer.fill("Private session A attachment");
await pastePng(composer);
await expect
.poll(() => page.getByRole("button", { name: "Send message" }).isDisabled())
.toBe(true);
await page.locator("openclaw-chat-pane").evaluate((pane, sessionKey) => {
(pane as HTMLElement & { sessionKey: string }).sessionKey = sessionKey;
}, secondSession);
await expect
.poll(() =>
page.evaluate(
() =>
(globalThis as unknown as { attachmentReadProof: DeferredAttachmentProof })
.attachmentReadProof.aborts,
),
)
.toBe(1);
await expect.poll(() => page.locator(".chat-attachment-thumb").count()).toBe(0);
await composer.fill("Safe session B message");
await composer.press("Enter");
const request = await gateway.waitForRequest("chat.send");
expect(request.params).toMatchObject({
message: "Safe session B message",
sessionKey: secondSession,
});
expect((request.params as { attachments?: unknown }).attachments).toBeUndefined();
} finally {
await context.close();
}
});
});

View File

@@ -237,6 +237,8 @@ export class ChatPaneRender extends ChatPaneHeaderRender {
activeRunIds: selectedSession?.activeRunIds,
queue: state.chatQueue,
});
const attachmentReads = this.chatState.attachmentReads;
const attachmentReadSignal = attachmentReads.readSignal;
const props: ChatProps = {
transcript: this.transcript,
paneId: this.paneId,
@@ -470,6 +472,10 @@ export class ChatPaneRender extends ChatPaneHeaderRender {
onScrollToBottom: state.scrollToBottom,
attachments: state.chatAttachments,
getAttachments: () => state.chatAttachments,
pendingAttachmentReads: attachmentReads.pendingReads,
getPendingAttachmentReads: () => attachmentReads.pendingReads,
readSignal: attachmentReadSignal,
onPendingReadsChange: (delta) => attachmentReads.updatePending(attachmentReadSignal, delta),
onAttachmentsChange: (next) => {
state.chatAttachments = next;
state.requestUpdate?.();

View File

@@ -4,6 +4,7 @@ import { subscribeChatOutboxProjection } from "./chat-queue.ts";
import type { ChatPageHost } from "./chat-state-host.ts";
import { invalidateImageLightbox } from "./chat-state-page.ts";
import { cancelChatStreamRenderFrame } from "./chat-state-render.ts";
import { ChatAttachmentReadLifecycle } from "./components/chat-attachments.ts";
import { clearSessionWorkspaceTimers } from "./components/chat-session-workspace.ts";
import {
ChatComposerPersistence,
@@ -25,6 +26,7 @@ type ChatRenderLifecycleScope = {
};
export class ChatStateController<TState extends ChatPageHost> implements ReactiveController {
readonly attachmentReads: ChatAttachmentReadLifecycle;
private readonly composerPersistence: ChatComposerPersistence;
private stateValue: TState | undefined;
private previousChatLoading = false;
@@ -44,6 +46,9 @@ export class ChatStateController<TState extends ChatPageHost> implements Reactiv
private renderLifecycleScope: ChatRenderLifecycleScope | undefined;
constructor(private readonly host: ReactiveControllerHost) {
this.attachmentReads = new ChatAttachmentReadLifecycle(() =>
this.stateValue?.requestUpdate?.(),
);
this.composerPersistence = new ChatComposerPersistence(() => this.stateValue);
host.addController(this);
}
@@ -69,6 +74,7 @@ export class ChatStateController<TState extends ChatPageHost> implements Reactiv
attach(state: TState) {
if (this.stateValue && this.stateValue !== state) {
this.attachmentReads.abortReads();
this.composerPersistence.stop();
cancelChatStreamRenderFrame(this.stateValue);
cancelChatScroll(this.stateValue);
@@ -311,6 +317,9 @@ export class ChatStateController<TState extends ChatPageHost> implements Reactiv
}
adoptComposerRoute() {
// File reads belong to their original session; abort before a late load can
// attach its payload to the pane's newly adopted route.
this.attachmentReads.abortReads();
this.composerPersistence.adoptCurrentRoute();
}
@@ -368,6 +377,7 @@ export class ChatStateController<TState extends ChatPageHost> implements Reactiv
hostDisconnected() {
this.renderLifecycleConnected = false;
this.cancelRenderLifecycleScope();
this.attachmentReads.abortReads();
// Flush while stateValue still points at the active session. Composer
// persistence is owned here so controller registration order cannot lose it.
this.composerPersistence.stop();

View File

@@ -590,6 +590,48 @@ describe("ChatStateController render lifecycle", () => {
expect(effect).not.toHaveBeenCalled();
});
it("aborts attachment reads when a pane adopts a different session", () => {
const host = {
addController: () => undefined,
removeController: () => undefined,
requestUpdate: () => undefined,
updateComplete: Promise.resolve(true),
} satisfies ReactiveControllerHost;
const controller = new ChatStateController<ChatPageHost>(host);
const previousSignal = controller.attachmentReads.readSignal;
controller.attachmentReads.updatePending(previousSignal, 1);
expect(controller.attachmentReads.pendingReads).toBe(1);
controller.adoptComposerRoute();
expect(previousSignal.aborted).toBe(true);
expect(controller.attachmentReads.pendingReads).toBe(0);
expect(controller.attachmentReads.readSignal).not.toBe(previousSignal);
controller.attachmentReads.updatePending(previousSignal, 1);
expect(controller.attachmentReads.pendingReads).toBe(0);
});
it("aborts attachment reads when a chat pane disconnects", () => {
const host = {
addController: () => undefined,
removeController: () => undefined,
requestUpdate: () => undefined,
updateComplete: Promise.resolve(true),
} satisfies ReactiveControllerHost;
const controller = new ChatStateController<ChatPageHost>(host);
const previousSignal = controller.attachmentReads.readSignal;
controller.attachmentReads.updatePending(previousSignal, 1);
controller.hostDisconnected();
expect(previousSignal.aborted).toBe(true);
expect(controller.attachmentReads.pendingReads).toBe(0);
expect(controller.attachmentReads.readSignal).not.toBe(previousSignal);
controller.attachmentReads.updatePending(previousSignal, -1);
expect(controller.attachmentReads.pendingReads).toBe(0);
});
it("rejects lifecycle work from detached and replaced state epochs", async () => {
const requestUpdate = vi.fn();
const host = {

View File

@@ -39,6 +39,7 @@ import {
stubAnimationFrames,
} from "./chat-view.test-helpers.ts";
import { renderChat } from "./chat-view.ts";
import { ChatAttachmentReadLifecycle } from "./components/chat-attachments.ts";
import { resetChatComposerState } from "./components/chat-composer.ts";
import * as chatMessage from "./components/chat-message.ts";
import {
@@ -3632,6 +3633,143 @@ describe("chat slash menu accessibility", () => {
});
describe("chat attachment picker", () => {
it.each(["clipboard", "file picker", "drop"] as const)(
"waits for an in-flight %s attachment before accepting an immediate send",
async (entry) => {
const readers: FileReader[] = [];
vi.spyOn(FileReader.prototype, "readAsDataURL").mockImplementation(
function (this: FileReader) {
readers.push(this);
},
);
const container = document.createElement("div");
const file = new File(["attachment proof"], "proof.png", { type: "image/png" });
const draft = "Send the attachment with this message";
let attachments: ChatAttachment[] = [];
const onSend = vi.fn(() => {
expect(attachments.map((attachment) => attachment.fileName)).toEqual(["proof.png"]);
});
const redraw = () => {
const readSignal = reads.readSignal;
render(
renderChat(
createChatProps({
attachments,
draft,
getAttachments: () => attachments,
getDraft: () => draft,
getPendingAttachmentReads: () => reads.pendingReads,
onAttachmentsChange: (next) => {
attachments = next;
},
onPendingReadsChange: (delta) => reads.updatePending(readSignal, delta),
onSend,
pendingAttachmentReads: reads.pendingReads,
readSignal,
}),
),
container,
);
};
const reads = new ChatAttachmentReadLifecycle(redraw);
redraw();
if (entry === "clipboard") {
const paste = new Event("paste", { bubbles: true, cancelable: true });
Object.defineProperty(paste, "clipboardData", {
value: {
items: [{ type: file.type, getAsFile: () => file }],
getData: () => "",
},
});
getComposerTextarea(container).dispatchEvent(paste);
} else if (entry === "file picker") {
const input = requireElement(
container,
".agent-chat__file-input",
"attachment file input",
) as HTMLInputElement;
Object.defineProperty(input, "files", { configurable: true, value: [file] });
input.dispatchEvent(new Event("change", { bubbles: true }));
} else {
const drop = new Event("drop", { bubbles: true, cancelable: true });
Object.defineProperty(drop, "dataTransfer", {
value: { files: [file], types: ["Files"] },
});
requireElement(container, "section.card.chat", "chat drop target").dispatchEvent(drop);
}
expect(readers).toHaveLength(1);
expect(reads.pendingReads).toBe(1);
expect(getComposerTextarea(container).disabled).toBe(false);
const send = requireElement(
container,
'button[aria-label="Send message"]',
"send button",
) as HTMLButtonElement;
expect(send.disabled).toBe(true);
getComposerTextarea(container).dispatchEvent(
new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "Enter" }),
);
expect(onSend).not.toHaveBeenCalled();
const reader = expectDefined(readers[0], "deferred attachment reader");
Object.defineProperty(reader, "result", {
configurable: true,
value: `data:image/png;base64,${btoa("attachment proof")}`,
});
reader.dispatchEvent(new ProgressEvent("load"));
await waitForFast(() => {
expect(reads.pendingReads).toBe(0);
expect(attachments.map((attachment) => attachment.fileName)).toEqual(["proof.png"]);
});
const readySend = requireElement(
container,
'button[aria-label="Send message"]',
"ready send button",
) as HTMLButtonElement;
expect(readySend.disabled).toBe(false);
readySend.click();
expect(onSend).toHaveBeenCalledOnce();
},
);
it("does not attach an aborted file read to a newly selected session", async () => {
const readers: FileReader[] = [];
vi.spyOn(FileReader.prototype, "readAsDataURL").mockImplementation(function (this: FileReader) {
readers.push(this);
});
const reads = new ChatAttachmentReadLifecycle(() => undefined);
const oldSignal = reads.readSignal;
const onAttachmentsChange = vi.fn();
const file = new File(["private session A"], "private.png", { type: "image/png" });
const container = renderChatView({
getPendingAttachmentReads: () => reads.pendingReads,
onAttachmentsChange,
onPendingReadsChange: (delta) => reads.updatePending(oldSignal, delta),
pendingAttachmentReads: reads.pendingReads,
readSignal: oldSignal,
sessionKey: "agent:main:session-a",
});
const drop = new Event("drop", { bubbles: true, cancelable: true });
Object.defineProperty(drop, "dataTransfer", {
value: { files: [file], types: ["Files"] },
});
requireElement(container, "section.card.chat", "session A drop target").dispatchEvent(drop);
expect(readers).toHaveLength(1);
expect(reads.pendingReads).toBe(1);
reads.abortReads();
await Promise.resolve();
await Promise.resolve();
expect(oldSignal.aborted).toBe(true);
expect(reads.pendingReads).toBe(0);
expect(reads.readSignal).not.toBe(oldSignal);
expect(onAttachmentsChange).not.toHaveBeenCalled();
});
it("highlights only the chat pane receiving a file drag", () => {
const first = renderChatView();
const second = renderChatView();

View File

@@ -179,6 +179,10 @@ export type ChatProps = {
autoExpandToolCalls?: boolean;
attachments?: ChatAttachment[];
getAttachments?: () => ChatAttachment[];
pendingAttachmentReads?: number;
getPendingAttachmentReads?: () => number;
readSignal?: AbortSignal;
onPendingReadsChange?: (delta: 1 | -1) => void;
onAttachmentsChange?: (attachments: ChatAttachment[]) => void;
onAssistantAttachmentLoaded?: () => void;
onRequestOpenImage?: () => number;
@@ -392,6 +396,10 @@ export function renderChat(props: ChatProps) {
followUpMode: props.followUpMode,
attachments: props.attachments,
getAttachments: props.getAttachments,
pendingAttachmentReads: props.pendingAttachmentReads,
getPendingAttachmentReads: props.getPendingAttachmentReads,
readSignal: props.readSignal,
onPendingReadsChange: props.onPendingReadsChange,
replyTarget: props.replyTarget,
realtimeTalkActive: props.realtimeTalkActive,
realtimeTalkStatus: props.realtimeTalkStatus,

View File

@@ -37,6 +37,32 @@ type ChatAttachmentControlsProps = {
readSignal?: AbortSignal;
};
export class ChatAttachmentReadLifecycle {
pendingReads = 0;
private controller = new AbortController();
constructor(private readonly notify: () => void) {}
get readSignal(): AbortSignal {
return this.controller.signal;
}
updatePending(readSignal: AbortSignal, delta: 1 | -1): void {
if (this.controller.signal !== readSignal) {
return;
}
this.pendingReads = Math.max(0, this.pendingReads + delta);
this.notify();
}
abortReads(): void {
this.controller.abort();
this.controller = new AbortController();
this.pendingReads = 0;
this.notify();
}
}
export function isFileDrag(dataTransfer: DataTransfer | null): boolean {
return Array.from(dataTransfer?.types ?? []).includes("Files");
}
@@ -238,7 +264,6 @@ function readAttachmentFile(
if (props.readSignal?.aborted) {
return Promise.resolve(null);
}
props.onPendingReadsChange?.(1);
return new Promise((resolve) => {
const reader = new FileReader();
let settled = false;
@@ -248,7 +273,6 @@ function readAttachmentFile(
}
settled = true;
props.readSignal?.removeEventListener("abort", abort);
props.onPendingReadsChange?.(-1);
resolve(attachment);
};
const abort = () => {
@@ -277,19 +301,26 @@ async function appendAttachmentFiles(files: readonly File[], props: ChatAttachme
if (!props.onAttachmentsChange || supported.length === 0) {
return;
}
const additions = (
await Promise.all(supported.map((file) => readAttachmentFile(file, props)))
).filter((attachment): attachment is ChatAttachment => attachment !== null);
if (props.readSignal?.aborted) {
for (const attachment of additions) {
releaseChatAttachmentPayload(attachment.id);
props.onPendingReadsChange?.(1);
try {
const additions = (
await Promise.all(supported.map((file) => readAttachmentFile(file, props)))
).filter((attachment): attachment is ChatAttachment => attachment !== null);
if (props.readSignal?.aborted) {
for (const attachment of additions) {
releaseChatAttachmentPayload(attachment.id);
}
return;
}
return;
if (additions.length === 0) {
return;
}
// Keep the batch pending until its payloads are in the composer so an
// immediate send cannot slip between FileReader completion and insertion.
props.onAttachmentsChange([...currentAttachments(props), ...additions]);
} finally {
props.onPendingReadsChange?.(-1);
}
if (additions.length === 0) {
return;
}
props.onAttachmentsChange([...currentAttachments(props), ...additions]);
}
export function handleChatAttachmentPaste(e: ClipboardEvent, props: ChatAttachmentControlsProps) {

View File

@@ -55,6 +55,10 @@ export type ChatComposerProps = {
followUpMode?: ControlUiFollowUpMode;
attachments?: ChatAttachment[];
getAttachments?: () => ChatAttachment[];
pendingAttachmentReads?: number;
getPendingAttachmentReads?: () => number;
readSignal?: AbortSignal;
onPendingReadsChange?: (delta: 1 | -1) => void;
replyTarget?: {
messageId: string;
text: string;

View File

@@ -258,7 +258,9 @@ export function renderChatComposer(props: ChatComposerProps) {
// Offline text and attachments may enter the persisted reconnect queue, but
// slash commands are live controls and must not execute against stale state.
const canSubmitDraft = (draft: string) =>
canCompose && (props.connected || !draft.trimStart().startsWith("/"));
canCompose &&
(props.getPendingAttachmentReads?.() ?? props.pendingAttachmentReads ?? 0) === 0 &&
(props.connected || !draft.trimStart().startsWith("/"));
const syncComposerDraftAfterSend = (target: HTMLTextAreaElement | null) => {
const submittedDraft = target?.value ?? props.getDraft?.() ?? props.draft;

View File

@@ -1,15 +1,21 @@
import type { ChatAttachment } from "../../lib/chat/chat-types.ts";
import { releaseChatAttachmentPayloads } from "../chat/attachment-payload-store.ts";
import { ChatAttachmentReadLifecycle } from "../chat/components/chat-attachments.ts";
export class NewSessionAttachmentDraft {
attachments: ChatAttachment[] = [];
pendingReads = 0;
private readController = new AbortController();
private readonly reads: ChatAttachmentReadLifecycle;
constructor(private readonly notify: () => void) {}
constructor(private readonly notify: () => void) {
this.reads = new ChatAttachmentReadLifecycle(notify);
}
get pendingReads(): number {
return this.reads.pendingReads;
}
get readSignal() {
return this.readController.signal;
return this.reads.readSignal;
}
replace(attachments: ChatAttachment[]) {
@@ -18,18 +24,11 @@ export class NewSessionAttachmentDraft {
}
updatePending(readSignal: AbortSignal, delta: 1 | -1) {
if (this.readController.signal !== readSignal) {
return;
}
this.pendingReads = Math.max(0, this.pendingReads + delta);
this.notify();
this.reads.updatePending(readSignal, delta);
}
abortReads() {
this.readController.abort();
this.readController = new AbortController();
this.pendingReads = 0;
this.notify();
this.reads.abortReads();
}
reset(options: { release: boolean }) {