From 575a423b6ad28dd455520a7bc6e647b543816382 Mon Sep 17 00:00:00 2001 From: jacobjmc Date: Sun, 8 Mar 2026 17:40:22 +1100 Subject: [PATCH 1/2] Reduce thread memory pressure --- src-tauri/src/shared/codex_core.rs | 75 +++++- .../messages/components/Messages.test.tsx | 86 +++++++ src/features/messages/components/Messages.tsx | 222 ++++++++++++++---- .../hooks/threadReducer/threadItemsSlice.ts | 21 ++ .../hooks/useThreads.integration.test.tsx | 86 +++++++ src/features/threads/hooks/useThreads.ts | 82 ++++++- .../threads/hooks/useThreadsReducer.ts | 1 + 7 files changed, 517 insertions(+), 56 deletions(-) diff --git a/src-tauri/src/shared/codex_core.rs b/src-tauri/src/shared/codex_core.rs index 7887b4a..35b7464 100644 --- a/src-tauri/src/shared/codex_core.rs +++ b/src-tauri/src/shared/codex_core.rs @@ -1,5 +1,8 @@ +use base64::Engine; use serde_json::{json, Value}; +use std::collections::hash_map::DefaultHasher; use std::collections::{HashMap, HashSet}; +use std::hash::{Hash, Hasher}; use std::net::IpAddr; use std::path::PathBuf; use std::sync::Arc; @@ -576,7 +579,10 @@ pub(crate) async fn resume_thread_core( } "file" => { if let Some(url) = part.get("url").and_then(|v| v.as_str()) { - content_parts.push(json!({ "type": "image", "value": url })); + content_parts.push(json!({ + "type": "image", + "value": frontend_image_value(url) + })); } } _ => {} @@ -1102,6 +1108,55 @@ pub(crate) async fn set_thread_name_core( const URL_IMAGE_FETCH_TIMEOUT: Duration = Duration::from_secs(10); const URL_IMAGE_MAX_BYTES: usize = 8 * 1024 * 1024; +fn extension_from_mime(mime: &str) -> &'static str { + match mime.trim().to_ascii_lowercase().as_str() { + "image/png" => "png", + "image/jpeg" => "jpg", + "image/gif" => "gif", + "image/webp" => "webp", + "image/svg+xml" => "svg", + "image/bmp" => "bmp", + "image/tiff" => "tiff", + _ => "bin", + } +} + +fn persist_data_image_to_temp_file(data_url: &str) -> Option { + let trimmed = data_url.trim(); + let (metadata, encoded) = trimmed + .strip_prefix("data:")? + .split_once(";base64,")?; + if !metadata.starts_with("image/") { + return None; + } + let bytes = base64::engine::general_purpose::STANDARD + .decode(encoded) + .ok()?; + + let mut hasher = DefaultHasher::new(); + metadata.hash(&mut hasher); + bytes.hash(&mut hasher); + let digest = hasher.finish(); + + let cache_dir = std::env::temp_dir().join("opencode-monitor-image-cache"); + std::fs::create_dir_all(&cache_dir).ok()?; + + let extension = extension_from_mime(metadata); + let path = cache_dir.join(format!("{digest:016x}.{extension}")); + if !path.exists() { + std::fs::write(&path, &bytes).ok()?; + } + path.to_str().map(|value| value.to_string()) +} + +fn frontend_image_value(raw: &str) -> String { + let trimmed = raw.trim(); + if trimmed.starts_with("data:image/") { + return persist_data_image_to_temp_file(trimmed).unwrap_or_else(|| trimmed.to_string()); + } + trimmed.to_string() +} + /// Build REST prompt parts from frontend input. /// /// REST uses `{ type: "file", mime, url: "data:...", filename }` for images. @@ -1141,7 +1196,6 @@ async fn build_rest_prompt_parts( // Local file path — read and base64-encode. match std::fs::read(trimmed) { Ok(bytes) => { - use base64::Engine; let encoded = base64::engine::general_purpose::STANDARD.encode(&bytes); let mime = mime_from_extension(trimmed); let filename = std::path::Path::new(trimmed) @@ -1629,7 +1683,10 @@ pub(crate) async fn send_user_message_core( if trimmed.is_empty() { continue; } - content_parts.push(json!({ "type": "image", "value": trimmed })); + content_parts.push(json!({ + "type": "image", + "value": frontend_image_value(trimmed) + })); } if !content_parts.is_empty() { let user_item_id = { @@ -2350,6 +2407,18 @@ mod tests { }); } + #[test] + fn frontend_image_value_materializes_data_urls_to_temp_files() { + let data_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jXioAAAAASUVORK5CYII="; + + let path = frontend_image_value(data_url); + let repeated = frontend_image_value(data_url); + + assert!(!path.starts_with("data:")); + assert_eq!(path, repeated); + assert!(PathBuf::from(&path).exists()); + } + #[test] fn hidden_session_ids_are_read_from_workspace_settings() { let runtime = Builder::new_current_thread() diff --git a/src/features/messages/components/Messages.test.tsx b/src/features/messages/components/Messages.test.tsx index ee9f51b..3ca239f 100644 --- a/src/features/messages/components/Messages.test.tsx +++ b/src/features/messages/components/Messages.test.tsx @@ -410,6 +410,92 @@ describe("Messages", () => { expect(useFileLinkOpenerMock).toHaveBeenCalledTimes(1); }); + it("virtualizes large message lists instead of rendering every row", async () => { + const items: ConversationItem[] = Array.from({ length: 60 }, (_, index) => ({ + id: `msg-virtual-${index}`, + kind: "message", + role: "assistant", + text: `Virtualized message ${index}`, + })); + const offsetHeightDescriptor = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "offsetHeight", + ); + const offsetWidthDescriptor = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "offsetWidth", + ); + const resizeObserverPrototype = window.ResizeObserver?.prototype; + const originalObserve = resizeObserverPrototype?.observe; + const originalUnobserve = resizeObserverPrototype?.unobserve; + const originalDisconnect = resizeObserverPrototype?.disconnect; + + Object.defineProperty(HTMLElement.prototype, "offsetHeight", { + configurable: true, + get() { + const element = this as HTMLElement; + if (element.classList.contains("messages")) { + return 720; + } + return 180; + }, + }); + Object.defineProperty(HTMLElement.prototype, "offsetWidth", { + configurable: true, + get() { + return 1024; + }, + }); + if (resizeObserverPrototype) { + resizeObserverPrototype.observe = () => undefined; + resizeObserverPrototype.unobserve = () => undefined; + resizeObserverPrototype.disconnect = () => undefined; + } + + let unmount: (() => void) | null = null; + try { + const view = render( + , + ); + unmount = view.unmount; + const { container } = view; + + await waitFor(() => { + const renderedMessages = container.querySelectorAll(".message"); + expect(renderedMessages.length).toBeGreaterThan(0); + expect(renderedMessages.length).toBeLessThan(items.length); + }); + + expect(screen.getByText("Virtualized message 0")).toBeTruthy(); + expect(screen.queryByText("Virtualized message 20")).toBeNull(); + expect(screen.queryByText("Virtualized message 59")).toBeNull(); + } finally { + unmount?.(); + if (offsetHeightDescriptor) { + Object.defineProperty(HTMLElement.prototype, "offsetHeight", offsetHeightDescriptor); + } else { + delete (HTMLElement.prototype as { offsetHeight?: number }).offsetHeight; + } + if (offsetWidthDescriptor) { + Object.defineProperty(HTMLElement.prototype, "offsetWidth", offsetWidthDescriptor); + } else { + delete (HTMLElement.prototype as { offsetWidth?: number }).offsetWidth; + } + if (resizeObserverPrototype) { + resizeObserverPrototype.observe = originalObserve ?? (() => undefined); + resizeObserverPrototype.unobserve = originalUnobserve ?? (() => undefined); + resizeObserverPrototype.disconnect = originalDisconnect ?? (() => undefined); + } + } + }); + it("renders title-only reasoning rows and keeps the working indicator generic", () => { const items: ConversationItem[] = [ { diff --git a/src/features/messages/components/Messages.tsx b/src/features/messages/components/Messages.tsx index c6ead23..3a0e3f8 100644 --- a/src/features/messages/components/Messages.tsx +++ b/src/features/messages/components/Messages.tsx @@ -1,5 +1,6 @@ import { memo, + type ReactNode, useCallback, useEffect, useLayoutEffect, @@ -7,6 +8,7 @@ import { useRef, useState, } from "react"; +import { useVirtualizer } from "@tanstack/react-virtual"; import ChevronDown from "lucide-react/dist/esm/icons/chevron-down"; import ChevronUp from "lucide-react/dist/esm/icons/chevron-up"; import type { @@ -63,8 +65,14 @@ type MessagesProps = { onQuoteMessage?: (text: string) => void; }; +type MessageListRow = { + id: string; + node: ReactNode; +}; + const SINGLE_FENCED_BLOCK_PATTERN = /^\s*(```|~~~)[^\r\n]*\r?\n([\s\S]*?)\r?\n\1\s*$/; +const VIRTUALIZATION_ROW_THRESHOLD = 30; function getCopyableMessageText(text: string) { const fencedMatch = text.match(SINGLE_FENCED_BLOCK_PATTERN); @@ -95,7 +103,6 @@ export const Messages = memo(function Messages({ onOpenThreadLink, onQuoteMessage, }: MessagesProps) { - const bottomRef = useRef(null); const containerRef = useRef(null); const autoScrollRef = useRef(true); const [expandedItems, setExpandedItems] = useState>(new Set()); @@ -144,7 +151,6 @@ export const Messages = memo(function Messages({ container.scrollTop = container.scrollHeight; return; } - bottomRef.current?.scrollIntoView({ block: "end" }); }, [isNearBottom]); useLayoutEffect(() => { @@ -333,7 +339,6 @@ export const Messages = memo(function Messages({ container.scrollTop = container.scrollHeight; return; } - bottomRef.current?.scrollIntoView({ block: "end" }); }, [scrollKey, isThinking, isNearBottom, threadId]); const groupedItems = useMemo(() => buildToolGroups(visibleItems), [visibleItems]); @@ -514,30 +519,25 @@ export const Messages = memo(function Messages({ return null; }; - return ( -
- {groupedItems.map((entry) => { - if (entry.kind === "toolGroup") { - const { group } = entry; - const isCollapsed = collapsedToolGroups.has(group.id); - const summaryParts = [ - formatCount(group.toolCount, "tool call", "tool calls"), - ]; - if (group.messageCount > 0) { - summaryParts.push(formatCount(group.messageCount, "message", "messages")); - } - const summaryText = summaryParts.join(", "); - const groupBodyId = `tool-group-${group.id}`; - const ChevronIcon = isCollapsed ? ChevronDown : ChevronUp; - return ( -
+ const rows = useMemo(() => { + const nextRows: MessageListRow[] = []; + groupedItems.forEach((entry) => { + if (entry.kind === "toolGroup") { + const { group } = entry; + const isCollapsed = collapsedToolGroups.has(group.id); + const summaryParts = [ + formatCount(group.toolCount, "tool call", "tool calls"), + ]; + if (group.messageCount > 0) { + summaryParts.push(formatCount(group.messageCount, "message", "messages")); + } + const summaryText = summaryParts.join(", "); + const groupBodyId = `tool-group-${group.id}`; + const ChevronIcon = isCollapsed ? ChevronDown : ChevronUp; + nextRows.push({ + id: `tool-group-${group.id}`, + node: ( +
- ); - } - return renderItem(entry.item); - })} - {planFollowupNode} - {userInputNode} - 0} - reasoningLabel={workingReasoningLabel} - /> - {!items.length && !userInputNode && !isThinking && !isLoadingMessages && ( -
- {threadId ? "Send a prompt to the agent." : "Send a prompt to start a new agent."} -
- )} - {!items.length && !userInputNode && !isThinking && isLoadingMessages && ( -
-
- - Loading… + ), + }); + return; + } + nextRows.push({ + id: entry.item.id, + node: renderItem(entry.item), + }); + }); + + if (planFollowupNode) { + nextRows.push({ + id: `plan-followup-${threadId ?? "none"}-${planFollowup.planItemId ?? "none"}`, + node: planFollowupNode, + }); + } + if (userInputNode) { + nextRows.push({ + id: `user-input-${activeUserInputRequestId ?? "none"}`, + node: userInputNode, + }); + } + + nextRows.push({ + id: `working-${threadId ?? "none"}-${isThinking ? "thinking" : lastDurationMs ?? "idle"}`, + node: ( + 0} + reasoningLabel={workingReasoningLabel} + /> + ), + }); + + if (!items.length && !userInputNode && !isThinking && !isLoadingMessages) { + nextRows.push({ + id: `empty-${threadId ?? "new"}`, + node: ( +
+ {threadId ? "Send a prompt to the agent." : "Send a prompt to start a new agent."}
+ ), + }); + } + + if (!items.length && !userInputNode && !isThinking && isLoadingMessages) { + nextRows.push({ + id: `loading-${threadId ?? "new"}`, + node: ( +
+
+ + Loading… +
+
+ ), + }); + } + + return nextRows; + }, [ + activeUserInputRequestId, + collapsedToolGroups, + groupedItems, + isLoadingMessages, + isThinking, + items.length, + lastDurationMs, + planFollowup.planItemId, + planFollowupNode, + processingStartedAt, + renderItem, + threadId, + toggleToolGroup, + userInputNode, + workingReasoningLabel, + ]); + const shouldVirtualize = rows.length > VIRTUALIZATION_ROW_THRESHOLD; + + const rowVirtualizer = useVirtualizer({ + count: rows.length, + enabled: shouldVirtualize, + getScrollElement: () => containerRef.current, + estimateSize: () => 180, + overscan: 6, + }); + const virtualRows = rowVirtualizer.getVirtualItems(); + + useEffect(() => { + if (!shouldVirtualize) { + return; + } + rowVirtualizer.measure(); + }, [ + activeUserInputRequestId, + collapsedToolGroups, + expandedItems, + isThinking, + lastDurationMs, + planFollowup.planItemId, + rowVirtualizer, + scrollKey, + shouldVirtualize, + ]); + + return ( +
+ {shouldVirtualize ? ( +
+ {virtualRows.map((virtualRow) => { + const row = rows[virtualRow.index]; + if (!row) { + return null; + } + return ( +
+ {row.node} +
+ ); + })}
+ ) : ( + rows.map((row) =>
{row.node}
) )} -
); }); diff --git a/src/features/threads/hooks/threadReducer/threadItemsSlice.ts b/src/features/threads/hooks/threadReducer/threadItemsSlice.ts index 352476b..12dc785 100644 --- a/src/features/threads/hooks/threadReducer/threadItemsSlice.ts +++ b/src/features/threads/hooks/threadReducer/threadItemsSlice.ts @@ -168,6 +168,27 @@ export function reduceThreadItems(state: ThreadState, action: ThreadAction): Thr threadsByWorkspace: nextThreadsByWorkspace, }; } + case "evictThreadItems": { + if (action.threadIds.length === 0) { + return state; + } + let changed = false; + const nextItemsByThread = { ...state.itemsByThread }; + action.threadIds.forEach((threadId) => { + if (!(threadId in nextItemsByThread)) { + return; + } + delete nextItemsByThread[threadId]; + changed = true; + }); + if (!changed) { + return state; + } + return { + ...state, + itemsByThread: nextItemsByThread, + }; + } case "setThreadItems": return { ...state, diff --git a/src/features/threads/hooks/useThreads.integration.test.tsx b/src/features/threads/hooks/useThreads.integration.test.tsx index 282cde9..2b1753f 100644 --- a/src/features/threads/hooks/useThreads.integration.test.tsx +++ b/src/features/threads/hooks/useThreads.integration.test.tsx @@ -261,6 +261,92 @@ describe("useThreads UX integration", () => { }); }); + it("re-resumes a loaded thread after its cached items are evicted", async () => { + vi.mocked(resumeThread).mockImplementation(async (_workspaceId, threadId) => ({ + result: { + thread: { + id: threadId, + preview: `Remote ${threadId}`, + updated_at: 9999, + turns: [], + }, + }, + })); + + const { result } = renderHook(() => + useThreads({ + activeWorkspace: workspace, + onWorkspaceConnected: vi.fn(), + }), + ); + + act(() => { + result.current.setActiveThreadId("thread-1"); + }); + + await waitFor(() => { + expect(vi.mocked(resumeThread)).toHaveBeenCalledWith("ws-1", "thread-1"); + }); + + act(() => { + for (let index = 1; index <= 7; index += 1) { + handlers?.onAgentMessageCompleted?.({ + workspaceId: "ws-1", + threadId: `thread-${index}`, + itemId: `assistant-${index}`, + text: `Message ${index}`, + isReplay: false, + }); + } + }); + + await waitFor(() => { + expect(result.current.threadsByWorkspace["ws-1"]?.length).toBe(7); + }); + + act(() => { + result.current.setActiveThreadId("thread-4"); + }); + + await waitFor(() => { + const thread4Calls = vi + .mocked(resumeThread) + .mock.calls.filter(([, threadId]) => threadId === "thread-4"); + expect(thread4Calls).toHaveLength(1); + }); + + act(() => { + result.current.setActiveThreadId("thread-1"); + }); + + act(() => { + for (let index = 8; index <= 9; index += 1) { + handlers?.onAgentMessageCompleted?.({ + workspaceId: "ws-1", + threadId: `thread-${index}`, + itemId: `assistant-${index}`, + text: `Message ${index}`, + isReplay: false, + }); + } + }); + + await waitFor(() => { + expect(result.current.threadsByWorkspace["ws-1"]?.length).toBe(9); + }); + + act(() => { + result.current.setActiveThreadId("thread-4"); + }); + + await waitFor(() => { + const thread4Calls = vi + .mocked(resumeThread) + .mock.calls.filter(([, threadId]) => threadId === "thread-4"); + expect(thread4Calls).toHaveLength(2); + }); + }); + it("clears empty plan updates to null", () => { const { result } = renderHook(() => useThreads({ diff --git a/src/features/threads/hooks/useThreads.ts b/src/features/threads/hooks/useThreads.ts index 4b17409..5e4d158 100644 --- a/src/features/threads/hooks/useThreads.ts +++ b/src/features/threads/hooks/useThreads.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useReducer, useRef } from "react"; +import { startTransition, useCallback, useEffect, useMemo, useReducer, useRef } from "react"; import type { CustomPromptOption, DebugEntry, @@ -27,6 +27,8 @@ import { saveDetachedReviewLinks, } from "@threads/utils/threadStorage"; +const MAX_INACTIVE_THREAD_CACHES = 5; + type UseThreadsOptions = { activeWorkspace: WorkspaceInfo | null; onWorkspaceConnected: (id: string) => void; @@ -79,6 +81,7 @@ export function useThreads({ const { handleUserInputSubmit, handleUserInputDismiss } = useThreadUserInput({ dispatch }); const { customNamesRef, + pinnedThreadsRef, threadActivityRef, pinnedThreadsVersion, getCustomName, @@ -88,7 +91,6 @@ export function useThreads({ isThreadPinned, getPinTimestamp, } = useThreadStorage(); - void pinnedThreadsVersion; const activeWorkspaceId = activeWorkspace?.id ?? null; const { activeThreadId, activeItems } = useThreadSelectors({ @@ -128,6 +130,82 @@ export function useThreads({ [activeThreadId, dispatch], ); + useEffect(() => { + const cachedThreadIds = Object.entries(state.itemsByThread) + .filter(([, items]) => items.length > 0) + .map(([threadId]) => threadId); + if (cachedThreadIds.length <= MAX_INACTIVE_THREAD_CACHES + 1) { + return; + } + + const protectedThreadIds = new Set( + Object.values(state.activeThreadIdByWorkspace).filter(Boolean), + ); + Object.entries(state.threadStatusById).forEach(([threadId, status]) => { + if (status?.isProcessing || status?.isReviewing) { + protectedThreadIds.add(threadId); + } + }); + + Object.entries(state.threadsByWorkspace).forEach(([workspaceId, threads]) => { + threads.forEach((thread) => { + if (pinnedThreadsRef.current[`${workspaceId}:${thread.id}`]) { + protectedThreadIds.add(thread.id); + } + }); + }); + + const updatedAtByThread = new Map(); + Object.values(threadActivityRef.current).forEach((workspaceThreads) => { + Object.entries(workspaceThreads ?? {}).forEach(([threadId, timestamp]) => { + const previous = updatedAtByThread.get(threadId) ?? 0; + if (timestamp > previous) { + updatedAtByThread.set(threadId, timestamp); + } + }); + }); + Object.values(state.threadsByWorkspace).forEach((threads) => { + threads.forEach((thread) => { + const previous = updatedAtByThread.get(thread.id) ?? 0; + if (thread.updatedAt > previous) { + updatedAtByThread.set(thread.id, thread.updatedAt); + } + }); + }); + + const evictionCandidates = cachedThreadIds + .filter((threadId) => !protectedThreadIds.has(threadId)) + .map((threadId) => ({ + threadId, + updatedAt: updatedAtByThread.get(threadId) ?? 0, + })) + .sort((a, b) => b.updatedAt - a.updatedAt); + const threadIdsToEvict = evictionCandidates + .slice(MAX_INACTIVE_THREAD_CACHES) + .map((entry) => entry.threadId); + + if (threadIdsToEvict.length === 0) { + return; + } + + threadIdsToEvict.forEach((threadId) => { + delete loadedThreadsRef.current[threadId]; + delete replaceOnResumeRef.current[threadId]; + }); + + startTransition(() => { + dispatch({ type: "evictThreadItems", threadIds: threadIdsToEvict }); + }); + }, [ + pinnedThreadsRef, + pinnedThreadsVersion, + state.activeThreadIdByWorkspace, + state.itemsByThread, + state.threadStatusById, + state.threadsByWorkspace, + threadActivityRef, + ]); + const safeMessageActivity = useCallback(() => { try { void onMessageActivity?.(); diff --git a/src/features/threads/hooks/useThreadsReducer.ts b/src/features/threads/hooks/useThreadsReducer.ts index 98b5964..eb5894d 100644 --- a/src/features/threads/hooks/useThreadsReducer.ts +++ b/src/features/threads/hooks/useThreadsReducer.ts @@ -91,6 +91,7 @@ export type ThreadAction = hasCustomName?: boolean; isReplay?: boolean; } + | { type: "evictThreadItems"; threadIds: string[] } | { type: "setThreadItems"; threadId: string; items: ConversationItem[] } | { type: "appendReasoningSummary"; From efbbf16e57c76c40ecfa26a8432bfe0bf1ef0f26 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 17:58:43 +1100 Subject: [PATCH 2/2] Guard base64 data URL image decoding against unbounded memory allocation (#10) * Initial plan * Add pre/post-decode size guards for data URL base64 images Co-authored-by: jacobjmc <111402762+jacobjmc@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jacobjmc <111402762+jacobjmc@users.noreply.github.com> --- src-tauri/src/shared/codex_core.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src-tauri/src/shared/codex_core.rs b/src-tauri/src/shared/codex_core.rs index 35b7464..1347b3b 100644 --- a/src-tauri/src/shared/codex_core.rs +++ b/src-tauri/src/shared/codex_core.rs @@ -1129,9 +1129,16 @@ fn persist_data_image_to_temp_file(data_url: &str) -> Option { if !metadata.starts_with("image/") { return None; } + let estimated_len = encoded.len().saturating_mul(3) / 4; + if estimated_len > URL_IMAGE_MAX_BYTES { + return None; + } let bytes = base64::engine::general_purpose::STANDARD .decode(encoded) .ok()?; + if bytes.len() > URL_IMAGE_MAX_BYTES { + return None; + } let mut hasher = DefaultHasher::new(); metadata.hash(&mut hasher);