Bound AI conversation restoration memory growth (APP-5257) - #15833
Bound AI conversation restoration memory growth (APP-5257)#15833warp-agent-staging[bot] wants to merge 3 commits into
Conversation
Cap the per-block stylized output and the number of command blocks materialized when restoring an AI conversation, mirroring the bound already applied to real terminal block serialization. CHANGELOG-BUG-FIX: Fixed unbounded memory growth when restoring AI conversations with very large command output or long history.
|
This PR was generated with Warp. Comment |
- Evict oldest command blocks during traversal (RecentCommandBlocks) instead of collecting the full history and truncating afterward, so peak memory is bounded to the retained window rather than the whole conversation. - Locate the retained output tail by scanning backward for the line boundary instead of materializing the untruncated string first. - Restate the shared line-cap constant's doc comment without enumerating its call sites. - Add regression tests for the extraction accumulator, the tail-finding helper, and the restoration-plan placement boundary.
- Fix: the count cap bounded RecentCommandBlocks entries, but each CommandBlockInfo still held a full-size clone of its output because truncation only happened later in to_serialized_blocklist_items. Apply truncated_output() at all three extraction sites (RunShellCommand result, UserQuery attachment, deprecated context executed_shell_commands) before constructing CommandBlockInfo. - Add MAX_RESTORED_COMMAND_OUTPUT_BYTES (1 MB, mirroring the existing MAX_FILE_READ_BYTES convention) as a backstop for a single pathologically long line that a line cap alone cannot bound. - Add regression tests asserting truncation happens during extraction (not just in the final serialized output) for all three sources, plus byte-ceiling and UTF-8 boundary tests. - Fix a brace-nesting slip from an earlier revision that had silently moved the context-block extraction outside the message loop (caught by the compiler before landing).
Reviewed and accepted — one thing still needs a human on a MacThree rounds of adversarial review, each of which found the same mistake in a different place: a bound sitting downstream of the allocation it was meant to prevent. First a post-hoc drain after full extraction, then a full normalize/split before keeping the tail, then a count-capped queue holding uncapped clones. All three are fixed, and the final pass confirmed the bounds are now applied before anything full-size is materialized: Also verified: What is not verified: the change has never been run. Exercising it means restoring an agent conversation past the caps, which requires a signed-in Warp. Our macOS runner cannot sign in — staging IAP credential failure, hit on two separate attempts — so this is blocked for us rather than skipped. Before this is enabled for users, someone on a Mac with an account should restore a conversation beyond both the 100-block and 1 MB bounds, confirm the retained tail and the placement of AI blocks anchored to dropped commands, quit and reopen, and confirm the original conversation is still intact on disk. One scope note for whoever picks this up: general terminal scrollback restore has the same shape of problem — Responding as wilson: Open session · View factory task |


Description
Restoring an AI conversation (
AIConversation::to_serialized_blocklist_items) rebuilds each command block straight from the persisted task messages, with no bound on the number of blocks or on a single block's output size. A summarization subagent call is expanded recursively, so the full pre-summarization command history — output included — is resurrected on every restore even though summarization exists specifically to keep that history out of the AI's context. This is the primary driver behind APP-5257's confirmed heap profile (65.1% / 8.31 GB underrestore_pane_tree→restore_pane_leaf/create_session).This PR bounds what conversation restore materializes into command blocks, the same way real terminal blocks already are, and does so at every point in the path from raw task message to accumulated block — not after the fact:
RunShellCommandtool call, aUserQueryattachment, and the deprecatedInputContext.executed_shell_commands) now calltruncated_output()before constructing theCommandBlockInfothat gets pushed into the accumulator.truncated_outputapplies a byte ceiling (MAX_RESTORED_COMMAND_OUTPUT_BYTES, 1 MB — mirroring the existing per-fileMAX_FILE_READ_BYTESconvention) and then a line cap (MAX_SERIALIZED_STYLIZED_OUTPUT_LINES, 5000 lines, most recent — the same constantimpl From<&Block> for SerializedBlockalready applies to a live block, nowpub(crate)so both paths share one source of truth). Both bounds locate their retained tail by scanning/indexing from the end instead of materializing the untruncated string first, so a single oversized output never forces a full-size transient copy.MAX_RESTORED_COMMAND_BLOCKS(100) command blocks by evicting the oldest as they're pushed (RecentCommandBlocks, a cappedVecDeque), rather than collecting every historical command into memory and truncating afterward. Since each pushed block's output is already bounded (previous point), the accumulator's peak footprint is bounded to roughly100 × (1 MB + a few KB), not the size of the whole conversation history.Both bounds apply uniformly regardless of whether a block came from the conversation's live messages or was recursively pulled in from a summarized-away subtask.
What this still doesn't bound, stated plainly. This PR bounds the command blocks restore materializes — it does not bound total restore memory.
seen_command_ids, the per-conversation message-ID-to-exchange maps, and the already-loadedapi::Taskpayload (the full proto message list, which restoration reads from regardless) remain unbounded by this change; they scale with total exchange/message count, not with any single command's output. That's a pre-existing baseline this PR does not attempt to fix.Persistence is untouched.
to_serialized_blocklist_itemstakes&selfand only reads fromtask_store, with no interior mutability; persistence (write_updated_conversation_state) always serializes fromself.all_tasks(), independent of this method. Verified with a test that reads the underlying task message'soutputfield after calling the bounded method and confirms it is still the full, untruncated string.User-visible: a restored conversation with very long output or very long history will show less of it in the terminal transcript than before (most recent 5000 lines / 1 MB per command, most recent 100 commands). The AI's own context and the persisted conversation are unaffected. An exchange whose command fell outside the retained window still attaches to the oldest retained command block rather than vanishing or reordering (existing, unmodified logic — covered here by a new boundary test).
This is a plain bound, not a deferral, so it does not carry the ordering/correctness risk of a lazy-restore approach — it needs no feature flag.
Out of scope: plain terminal scrollback restore (
get_all_restored_blocks) has the same "load everything, then drain" shape and is tracked separately under APP-5757; not touched here.Linked Issue
APP-5257 (Linear)
Testing
./script/formatandcargo clippy -p warp --lib --all-targets --tests -- -D warningspass.cargo test -p warp --lib ai::agent::conversation::(61 tests) passes, including:RunShellCommandresult, aUserQueryattachment, and a deprecatedInputContext.executed_shell_commandsentry are each truncated before accumulation (asserted viaextract_command_blocksdirectly, not just the final serialized output).RecentCommandBlocksevicts the oldest entries once the cap is exceeded and reports the correct total-seen count.tail_linesfinds the exact truncation boundary, handles a trailing newline, and leaves short output untouched.MAX_RESTORED_COMMAND_BLOCKS.cargo test -p warp --lib terminal::conversation_restoration(17 tests) passes, including a test asserting an exchange older than the retained window attaches to the oldest retained block.cargo test -p warp --lib terminal::model::block::(45 tests) passes, confirming the shared constant's visibility change didn't affect real-block serialization.