Skip to content

Bound AI conversation restoration memory growth (APP-5257) - #15833

Draft
warp-agent-staging[bot] wants to merge 3 commits into
masterfrom
factory/app-5257-bound-conversation-restore
Draft

Bound AI conversation restoration memory growth (APP-5257)#15833
warp-agent-staging[bot] wants to merge 3 commits into
masterfrom
factory/app-5257-bound-conversation-restore

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

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 under restore_pane_treerestore_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:

  • Per-block output size, truncated before it's ever cloned: all three extraction sites (a completed RunShellCommand tool call, a UserQuery attachment, and the deprecated InputContext.executed_shell_commands) now call truncated_output() before constructing the CommandBlockInfo that gets pushed into the accumulator. truncated_output applies a byte ceiling (MAX_RESTORED_COMMAND_OUTPUT_BYTES, 1 MB — mirroring the existing per-file MAX_FILE_READ_BYTES convention) and then a line cap (MAX_SERIALIZED_STYLIZED_OUTPUT_LINES, 5000 lines, most recent — the same constant impl From<&Block> for SerializedBlock already applies to a live block, now pub(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.
  • Block count, bounded during traversal, not after: extraction retains only the most recent MAX_RESTORED_COMMAND_BLOCKS (100) command blocks by evicting the oldest as they're pushed (RecentCommandBlocks, a capped VecDeque), 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 roughly 100 × (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-loaded api::Task payload (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_items takes &self and only reads from task_store, with no interior mutability; persistence (write_updated_conversation_state) always serializes from self.all_tasks(), independent of this method. Verified with a test that reads the underlying task message's output field 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/format and cargo clippy -p warp --lib --all-targets --tests -- -D warnings pass.
  • cargo test -p warp --lib ai::agent::conversation:: (61 tests) passes, including:
    • Per-source extraction-time truncation: a RunShellCommand result, a UserQuery attachment, and a deprecated InputContext.executed_shell_commands entry are each truncated before accumulation (asserted via extract_command_blocks directly, not just the final serialized output).
    • The byte ceiling truncates a single line with no newlines at all (which the line cap alone can't bound), and correctly snaps to a UTF-8 character boundary.
    • RecentCommandBlocks evicts the oldest entries once the cap is exceeded and reports the correct total-seen count.
    • tail_lines finds the exact truncation boundary, handles a trailing newline, and leaves short output untouched.
    • The persisted task message's output is untouched after building the (bounded) restored blocklist.
    • Total command blocks are capped to the most recent MAX_RESTORED_COMMAND_BLOCKS.
    • A conversation with a summarized-away command still restores it, bounded the same as any other block.
  • 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.
  • Not independently re-verified against the original Sentry heap profile (no repro environment for that); the fix targets the exact code path and unbounded allocation pattern the profile and code review identified.

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.
@cla-bot cla-bot Bot added the cla-signed label Sep 5, 2026
@warp-agent-staging warp-agent-staging Bot added the area:performance:memory Memory usage, allocation, leaks, and memory-bound performance. label Sep 5, 2026
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

This PR was generated with Warp.

Comment @warp-staging-factory on this PR to send it follow-up work.

View run View conversation

- 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).
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

Reviewed and accepted — one thing still needs a human on a Mac

Three 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: truncated_output returns borrowed suffixes, snapping the 1 MB cut forward to a UTF-8 boundary, and the only allocation is the retained suffix.

Also verified: tail_lines is semantically equivalent to the previous normalize/split/join across trailing-LF, no-trailing-LF, under-cap, exact-cap, zero and empty inputs; RecentCommandBlocks preserves the recursive chronological traversal and global dedup; all three extraction sites truncate before push and are the only production sites; task-message persistence never serializes the bounded display output back to disk.

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 — get_all_restored_blocks loads all rows and full blobs before draining to its retention limit. That is pre-existing and tracked separately under APP-5757; this PR does not touch it.

Responding as wilson: Open session · View factory task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:performance:memory Memory usage, allocation, leaks, and memory-bound performance. cla-signed factory:wilson

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants