From 607f50becefccdbef3c9be34fa2791fdb13328a8 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 10 Aug 2026 11:33:32 +0200 Subject: [PATCH] fix: preserve fallback attachment packages across remounts --- ui/src/app/chat-attachment-handoff.test.ts | 26 +++++++--- ui/src/app/chat-attachment-handoff.ts | 50 ++++++++++++++----- ui/src/app/context.ts | 14 ++++-- ui/src/lib/chat/chat-types.ts | 13 +++++ .../chat/chat-pane-attachment-handoff.ts | 15 +++--- ...-pane-browser-annotation-lifecycle.test.ts | 15 ++++-- ui/src/pages/chat/chat-state-host.ts | 11 +--- ui/src/pages/chat/chat-view.test.ts | 3 +- ui/src/pages/chat/composer-persistence.ts | 11 ++-- 9 files changed, 108 insertions(+), 50 deletions(-) diff --git a/ui/src/app/chat-attachment-handoff.test.ts b/ui/src/app/chat-attachment-handoff.test.ts index ed71e980bb4d..2d82762f5c56 100644 --- a/ui/src/app/chat-attachment-handoff.test.ts +++ b/ui/src/app/chat-attachment-handoff.test.ts @@ -59,12 +59,15 @@ describe("chat attachment route handoff", () => { paneId: "p1", scopeKey: "agent:main:one", attachments: staged, + fallbacks: {}, }); const consumed = handoff.consume({ owner, paneId: "p1", scopeKey: "agent:main:one" }); - expect(consumed).toEqual(staged); - expect(consumed).not.toBe(staged); - expect(consumed?.every((attachment, index) => attachment === staged[index])).toBe(true); + expect(consumed?.attachments).toEqual(staged); + expect(consumed?.attachments).not.toBe(staged); + expect(consumed?.attachments.every((attachment, index) => attachment === staged[index])).toBe( + true, + ); expect(handoff.consume({ owner, paneId: "p1", scopeKey: "agent:main:one" })).toBeNull(); for (const attachment of ordinary) { expect(getChatAttachmentDataUrl(attachment)).not.toBeNull(); @@ -89,6 +92,7 @@ describe("chat attachment route handoff", () => { paneId: "p1", scopeKey: "agent:main:one", attachments: [annotation], + fallbacks: {}, }); expect( @@ -108,10 +112,16 @@ describe("chat attachment route handoff", () => { const oversized = Array.from({ length: 33 }, (_, index) => storedAttachment(`oversized-${index}`, "image/png", false), ); - handoff.prepare({ owner, paneId: "oversized", scopeKey: "oversized", attachments: oversized }); - expect(handoff.consume({ owner, paneId: "oversized", scopeKey: "oversized" })).toEqual( - oversized, - ); + handoff.prepare({ + owner, + paneId: "oversized", + scopeKey: "oversized", + attachments: oversized, + fallbacks: {}, + }); + expect( + handoff.consume({ owner, paneId: "oversized", scopeKey: "oversized" })?.attachments, + ).toEqual(oversized); expect(getChatAttachmentDataUrl(oversized[32]!)).not.toBeNull(); const annotations = Array.from({ length: 33 }, (_, index) => @@ -123,6 +133,7 @@ describe("chat attachment route handoff", () => { paneId: `p${index}`, scopeKey: `scope-${index}`, attachments: [annotation], + fallbacks: {}, }), ); @@ -144,6 +155,7 @@ describe("chat attachment route handoff", () => { paneId: "p1", scopeKey: "agent:main:one", attachments: [annotation], + fallbacks: {}, }); expect(getChatAttachmentDataUrl(annotation)).toBeNull(); diff --git a/ui/src/app/chat-attachment-handoff.ts b/ui/src/app/chat-attachment-handoff.ts index 474ad4e8c8ae..c8338308ebce 100644 --- a/ui/src/app/chat-attachment-handoff.ts +++ b/ui/src/app/chat-attachment-handoff.ts @@ -1,4 +1,4 @@ -import type { ChatAttachment } from "../lib/chat/chat-types.ts"; +import type { ChatAttachment, ChatComposerMemoryFallback } from "../lib/chat/chat-types.ts"; import { releaseChatAttachmentPayloads } from "../pages/chat/attachment-payload-store.ts"; import type { ApplicationChatAttachmentHandoff } from "./context.ts"; @@ -10,6 +10,7 @@ type PendingChatAttachmentHandoff = { owner: NonNullable[0]["owner"]>; scopeKey: string; attachments: ChatAttachment[]; + fallbacks: Record; }; export function createChatAttachmentHandoff(): ApplicationChatAttachmentHandoff { @@ -18,6 +19,18 @@ export function createChatAttachmentHandoff(): ApplicationChatAttachmentHandoff const release = (attachments: readonly ChatAttachment[] = []) => releaseChatAttachmentPayloads(attachments); + const releaseHandoff = (handoff: PendingChatAttachmentHandoff | undefined) => { + if (!handoff) { + return; + } + const byId = new Map(handoff.attachments.map((attachment) => [attachment.id, attachment])); + for (const fallback of Object.values(handoff.fallbacks)) { + for (const attachment of fallback.attachments) { + byId.set(attachment.id, attachment); + } + } + release([...byId.values()]); + }; const take = (paneId: string) => { const handoff = pending.get(paneId); if (handoff) { @@ -27,26 +40,39 @@ export function createChatAttachmentHandoff(): ApplicationChatAttachmentHandoff }; return { - prepare: ({ owner, paneId, scopeKey, attachments }) => { + prepare: ({ owner, paneId, scopeKey, attachments, fallbacks }) => { const previous = take(paneId); - if (attachments.length === 0) { - release(previous?.attachments); + const fallbackEntries = Object.entries(fallbacks); + if (attachments.length === 0 && fallbackEntries.length === 0) { + releaseHandoff(previous); return; } - const retainedIds = new Set(attachments.map((attachment) => attachment.id)); - release(previous?.attachments.filter((attachment) => !retainedIds.has(attachment.id))); + releaseHandoff(previous); if (!owner || disposed) { release(attachments); + for (const fallback of Object.values(fallbacks)) { + release(fallback.attachments); + } return; } - pending.set(paneId, { owner, scopeKey, attachments: [...attachments] }); + pending.set(paneId, { + owner, + scopeKey, + attachments: [...attachments], + fallbacks: Object.fromEntries( + fallbackEntries.map(([key, fallback]) => [ + key, + { ...fallback, attachments: [...fallback.attachments] }, + ]), + ), + }); // Route handoffs normally consume immediately. Bounds make abandoned // split panes release their packages instead of leaking for the tab lifetime. for (const oldestPaneId of pending.keys()) { if (pending.size <= MAX_PENDING_CHAT_ATTACHMENT_ENTRIES) { break; } - release(take(oldestPaneId)?.attachments); + releaseHandoff(take(oldestPaneId)); } }, consume: ({ owner, paneId, scopeKey }) => { @@ -54,16 +80,16 @@ export function createChatAttachmentHandoff(): ApplicationChatAttachmentHandoff // Reusing a pane id with another session or Gateway is terminal for the // old owner; keeping it would allow a later remount to recover stale evidence. if (match?.owner === owner && match.scopeKey === scopeKey) { - return match.attachments; + return { attachments: match.attachments, fallbacks: match.fallbacks }; } - release(match?.attachments); + releaseHandoff(match); return null; }, - clearPane: (paneId) => release(take(paneId)?.attachments), + clearPane: (paneId) => releaseHandoff(take(paneId)), dispose: () => { disposed = true; for (const handoff of pending.values()) { - release(handoff.attachments); + releaseHandoff(handoff); } pending.clear(); }, diff --git a/ui/src/app/context.ts b/ui/src/app/context.ts index 11272c389db6..60a9d1b3162b 100644 --- a/ui/src/app/context.ts +++ b/ui/src/app/context.ts @@ -4,7 +4,7 @@ import type { RouteId } from "../app-route-paths.ts"; import type { AgentIdentityCapability } from "../lib/agents/identity.ts"; import type { AgentCapability } from "../lib/agents/index.ts"; import type { ChannelCapability } from "../lib/channels/index.ts"; -import type { ChatAttachment } from "../lib/chat/chat-types.ts"; +import type { ChatAttachment, ChatComposerMemoryFallback } from "../lib/chat/chat-types.ts"; import type { RuntimeConfigCapability } from "../lib/config/index.ts"; import type { SessionCapability } from "../lib/sessions/index.ts"; import type { WorkboardCapability } from "../lib/workboard/capability.ts"; @@ -80,8 +80,16 @@ type ChatAttachmentHandoffKey = { }; export type ApplicationChatAttachmentHandoff = { - prepare(handoff: ChatAttachmentHandoffKey & { attachments: readonly ChatAttachment[] }): void; - consume(handoff: ChatAttachmentHandoffKey): ChatAttachment[] | null; + prepare( + handoff: ChatAttachmentHandoffKey & { + attachments: readonly ChatAttachment[]; + fallbacks: Readonly>; + }, + ): void; + consume(handoff: ChatAttachmentHandoffKey): { + attachments: ChatAttachment[]; + fallbacks: Record; + } | null; clearPane(paneId: string): void; dispose(): void; }; diff --git a/ui/src/lib/chat/chat-types.ts b/ui/src/lib/chat/chat-types.ts index 35a314e560de..6e6d4a8753ce 100644 --- a/ui/src/lib/chat/chat-types.ts +++ b/ui/src/lib/chat/chat-types.ts @@ -24,6 +24,19 @@ export type ChatAttachment = { browserAnnotation?: BrowserAnnotationAttachment; }; +export type ChatComposerDraftRetry = { + expectedDraftRevision: number; + draftRevision: number; +}; + +export type ChatComposerMemoryFallback = { + message: string; + attachments: ChatAttachment[]; + storageFailed: boolean; + draftRetry?: ChatComposerDraftRetry; + sequence: number; +}; + export type ChatQueueSkillWorkshopRevision = { proposalId: string; agentId?: string; diff --git a/ui/src/pages/chat/chat-pane-attachment-handoff.ts b/ui/src/pages/chat/chat-pane-attachment-handoff.ts index 2c23045945c2..b76baef31479 100644 --- a/ui/src/pages/chat/chat-pane-attachment-handoff.ts +++ b/ui/src/pages/chat/chat-pane-attachment-handoff.ts @@ -29,13 +29,6 @@ function releaseAttachments( } } -function releaseFallbackAttachments(state: ChatPageHost, retainedIds = new Set()): void { - const releasedIds = new Set(); - for (const fallback of Object.values(state.chatComposerFallbackByScope)) { - releaseAttachments(fallback.attachments, retainedIds, releasedIds); - } -} - export function restorePaneStagedAttachments( context: ApplicationContext, paneId: string, @@ -49,8 +42,12 @@ export function restorePaneStagedAttachments( const currentIds = new Set(state.chatAttachments.map((attachment) => attachment.id)); state.chatAttachments = [ ...state.chatAttachments, - ...restored.filter((attachment) => !currentIds.has(attachment.id)), + ...restored.attachments.filter((attachment) => !currentIds.has(attachment.id)), ]; + state.chatComposerFallbackByScope = { + ...restored.fallbacks, + ...state.chatComposerFallbackByScope, + }; } export function preparePaneStagedAttachments( @@ -63,8 +60,8 @@ export function preparePaneStagedAttachments( context.chatAttachmentHandoff.prepare({ ...handoffKey(paneId, state, owner), attachments, + fallbacks: state.chatComposerFallbackByScope, }); - releaseFallbackAttachments(state, new Set(attachments.map((attachment) => attachment.id))); } export function discardStateStagedAttachments(state: ChatPageHost | undefined): void { diff --git a/ui/src/pages/chat/chat-pane-browser-annotation-lifecycle.test.ts b/ui/src/pages/chat/chat-pane-browser-annotation-lifecycle.test.ts index 2e370f2d3c06..541e48455396 100644 --- a/ui/src/pages/chat/chat-pane-browser-annotation-lifecycle.test.ts +++ b/ui/src/pages/chat/chat-pane-browser-annotation-lifecycle.test.ts @@ -108,17 +108,24 @@ describe("staged attachment composer adoption", () => { pane.disconnectedCallback(); expect(getChatAttachmentDataUrl(shared)).not.toBeNull(); - expect(getChatAttachmentDataUrl(fallback)).toBeNull(); + expect(getChatAttachmentDataUrl(fallback)).not.toBeNull(); expect(getChatAttachmentDataUrl(ordinary)).not.toBeNull(); const transferred = pane.context.chatAttachmentHandoff.consume({ owner, paneId: pane.paneId, scopeKey, }); - expect(transferred).toEqual([shared, ordinary]); - expect(transferred?.[0]).toBe(shared); - expect(transferred?.[1]).toBe(ordinary); + expect(transferred?.attachments).toEqual([shared, ordinary]); + expect(transferred?.attachments[0]).toBe(shared); + expect(transferred?.attachments[1]).toBe(ordinary); + expect(transferred?.fallbacks.fallback).toMatchObject({ + message: "", + sequence: 1, + storageFailed: false, + }); + expect(transferred?.fallbacks.fallback?.attachments).toEqual([shared, fallback, ordinary]); releaseChatAttachmentPayload(shared.id); + releaseChatAttachmentPayload(fallback.id); releaseChatAttachmentPayload(ordinary.id); }); diff --git a/ui/src/pages/chat/chat-state-host.ts b/ui/src/pages/chat/chat-state-host.ts index ab0216be8c90..090fe336cec0 100644 --- a/ui/src/pages/chat/chat-state-host.ts +++ b/ui/src/pages/chat/chat-state-host.ts @@ -8,7 +8,7 @@ import type { import type { ApplicationContext } from "../../app/context.ts"; import type { UiSettings } from "../../app/settings.ts"; import type { ImageLightboxItem } from "../../components/image-lightbox.ts"; -import type { ChatAttachment } from "../../lib/chat/chat-types.ts"; +import type { ChatComposerMemoryFallback } from "../../lib/chat/chat-types.ts"; import type { EmbedSandboxMode } from "../../lib/chat/tool-display.ts"; import type { ChatState } from "./chat-history.ts"; import type { ChatRealtimeState } from "./chat-realtime.ts"; @@ -18,7 +18,6 @@ import type { ChatProps } from "./chat-view.ts"; import type { BackgroundTasksHost } from "./components/chat-background-tasks.ts"; import type { SessionWorkspaceHost } from "./components/chat-session-workspace.ts"; import type { SidebarContent } from "./components/chat-sidebar.ts"; -import type { ChatComposerDraftRetry } from "./composer-persistence.ts"; import type { ChatInputHistoryKeyInput, ChatInputHistoryKeyResult } from "./input-history.ts"; import type { RenderLifecycle } from "./render-lifecycle.ts"; import type { PendingChatAbort } from "./run-lifecycle.ts"; @@ -32,13 +31,7 @@ import type { WaitingApprovalStatus, } from "./tool-stream.ts"; -export type ChatComposerMemoryFallback = { - message: string; - attachments: ChatAttachment[]; - storageFailed: boolean; - draftRetry?: ChatComposerDraftRetry; - sequence: number; -}; +export type { ChatComposerMemoryFallback } from "../../lib/chat/chat-types.ts"; export type ChatPageHost = ChatHost & ChatState & diff --git a/ui/src/pages/chat/chat-view.test.ts b/ui/src/pages/chat/chat-view.test.ts index b723b23186bd..f037781506ea 100644 --- a/ui/src/pages/chat/chat-view.test.ts +++ b/ui/src/pages/chat/chat-view.test.ts @@ -4478,11 +4478,12 @@ describe("chat attachment picker", () => { paneId: "p1", scopeKey: "agent:main:one", attachments, + fallbacks: {}, }); attachments = expectDefined( handoff.consume({ owner, paneId: "p1", scopeKey: "agent:main:one" }), "restored attachments", - ); + ).attachments; expect(attachments).toHaveLength(1); expect(attachments[0]).toBe(original); diff --git a/ui/src/pages/chat/composer-persistence.ts b/ui/src/pages/chat/composer-persistence.ts index 43e84588c761..367cf8353e46 100644 --- a/ui/src/pages/chat/composer-persistence.ts +++ b/ui/src/pages/chat/composer-persistence.ts @@ -1,4 +1,8 @@ -import type { ChatAttachment, ChatQueueItem } from "../../lib/chat/chat-types.ts"; +import type { + ChatAttachment, + ChatComposerDraftRetry, + ChatQueueItem, +} from "../../lib/chat/chat-types.ts"; import { INTERRUPTED_SETTINGS_WAIT_ERROR, MAX_STORED_QUEUE_ITEMS, @@ -66,10 +70,7 @@ type RestoreOptions = { sessionKey?: string; }; -export type ChatComposerDraftRetry = { - expectedDraftRevision: number; - draftRevision: number; -}; +export type { ChatComposerDraftRetry } from "../../lib/chat/chat-types.ts"; type ChatComposerPersistStatus = "persisted" | "conflict" | "storage-failed";