Fix turn-arena memory retention - #484
Draft
iamken1204 wants to merge 4 commits into
Draft
Conversation
iamken1204
force-pushed
the
fix-turn-arena-tool-retention
branch
2 times, most recently
from
August 28, 2026 03:18
13444a5 to
4187c96
Compare
iamken1204
pushed a commit
to iamken1204/fx
that referenced
this pull request
Aug 28, 2026
Both cherry-picked from the fix-turn-arena-tool-retention branch behind upstream PR vercel-labs#484: the per-call arena reclamation and its companion fix keeping the MCP input-required error code static. The conflict policy is to drop both together once upstream absorbs the PR. Co-authored-by: Cursor <cursoragent@cursor.com>
iamken1204
pushed a commit
to iamken1204/fx
that referenced
this pull request
Aug 28, 2026
Both cherry-picked from the fix-turn-arena-tool-retention branch behind upstream PR vercel-labs#484: the per-call arena reclamation and its companion fix keeping the MCP input-required error code static. The conflict policy is to drop both together once upstream absorbs the PR. Co-authored-by: Cursor <cursoragent@cursor.com>
iamken1204
force-pushed
the
fix-turn-arena-tool-retention
branch
2 times, most recently
from
August 29, 2026 02:47
1a394ea to
636a533
Compare
iamken1204
pushed a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
Both cherry-picked from the fix-turn-arena-tool-retention branch behind upstream PR vercel-labs#484: the per-call arena reclamation and its companion fix keeping the MCP input-required error code static. The conflict policy is to drop both together once upstream absorbs the PR. Co-authored-by: Cursor <cursoragent@cursor.com>
iamken1204
force-pushed
the
fix-turn-arena-tool-retention
branch
from
August 29, 2026 14:39
636a533 to
0821880
Compare
|
Security review: https://vercel.slack.com/archives/C0BSEH6GB18/p1788014541098329 |
iamken1204
added a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
The third turn-arena patch is on kfx as a cherry-pick of the PR vercel-labs#484 commit, so the inventory names it, its copy-out boundary, and the same drop rule as its two predecessors. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
iamken1204
added a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
The fourth turn-arena patch is on kfx as a cherry-pick of the PR vercel-labs#484 commit, so the inventory names it, its poll boundary, and the same drop rule as the three before it. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
iamken1204
pushed a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
Both cherry-picked from the fix-turn-arena-tool-retention branch behind upstream PR vercel-labs#484: the per-call arena reclamation and its companion fix keeping the MCP input-required error code static. The conflict policy is to drop both together once upstream absorbs the PR. Co-authored-by: Cursor <cursoragent@cursor.com>
iamken1204
added a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
The third turn-arena patch is on kfx as a cherry-pick of the PR vercel-labs#484 commit, so the inventory names it, its copy-out boundary, and the same drop rule as its two predecessors. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
iamken1204
added a commit
to iamken1204/fx
that referenced
this pull request
Aug 29, 2026
The fourth turn-arena patch is on kfx as a cherry-pick of the PR vercel-labs#484 commit, so the inventory names it, its poll boundary, and the same drop rule as the three before it. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
…he turn arena A turn allocates from one ArenaAllocator that is freed only when the turn returns, and two kinds of transient memory were parked in it permanently. Ordinary tool calls received the turn arena as their dispatch allocator, so every nested scratch arena inside a tool was backed by it and its deinit reclaimed nothing: grep_files retained the git grep stdout and every scanned candidate file, read_file retained its content scratch, prepareModelOutput retained the sanitize and mask copies of every raw result. File mutations already ran on a distinct per-call arena with copy-out into the result allocator; this change applies that existing contract to every call. The orchestrator (and the parallel execution hook) give each call a per-call c_allocator-backed arena, executeToolCallAuthorized threads it into the dispatch context, sinks and backend completions (run command, vision) allocate from the result allocator, and executeRegisteredTool copies the dispatch-owned survivors (body, status detail, tool result memory) to the result allocator at the bottom. The copy is skipped when both owners are the same allocator, which keeps test paths byte-identical and leak-checked. dupeToolResultMemory becomes the single shared deep copy in types.zig, with a field-count tripwire test, and a tool_runtime test executes an ordinary tool with distinct owners and reads the result after the call arena is destroyed. persistRecoveryCheckpoint deep-copied every tool result accumulated so far in the turn into the turn arena one to two times per settled provider attempt, quadratic in steps. The copy now lives in a scratch arena freed when the checkpoint returns; RecoveryCheckpointEffect.set documents that the checkpoint borrows caller scratch and every sink serializes or dupes before returning (session_event.zig applies the event through checkpoint.dupe). prepareModelOutput and toolCallPresentation keep private c_allocator-backed scratch because their callers pass the turn arena as the result target, which the dispatch boundary cannot reclaim; toolCallPresentation, toolActivityKindForCall, and activityKindForCall lose their now-unused allocator parameters. Retention compounds on long turns: arena pages are written once and never touched again, so macOS compresses them until jetsam kills the process (observed at 81 GB after a 23-minute, ~80-step turn). Measured on a 50-step turn running one repository-wide grep per step, peak RSS drops from 303 MB to 177 MB; the remaining growth is per-attempt provider request assembly, which is a separate change. Co-authored-by: Cursor <cursoragent@cursor.com>
The per-call arena change duped status_detail onto the turn arena at the dispatch boundary, which also rewrote the static "McpInputRequired" literal. fx ask borrows that pointer into PromptRunResult.error_code and serializes it after the turn arena is freed, so the final JSON render read unmapped memory and crashed with SIGSEGV on Linux (glibc munmaps the arena pages; macOS keeps them readable, hiding the bug). Assign the input-required override after the copy-out so the only finish-turn error code that outlives the turn stays a static literal, matching the lifetime contract of every other error_code assignment. Co-authored-by: Cursor <cursoragent@cursor.com>
…arena Every model attempt received the turn arena as its allocator, so the request body, the std.http.Client state, the provider-state replay parse of every prior assistant message, and the JSON DOM of every SSE event stayed allocated until the turn returned. Their free and deinit calls were no-ops against the arena. A long tool turn resends its growing context on every step, so the retained bytes grew quadratically with the step count, and three one-off subagents sharing the process tripled it. The 2026-08-29 incident reached 48 GiB of compressed pages on a build that already carried the tool-call and checkpoint fix. Each attempt-loop iteration now owns a c_allocator-backed attempt arena and passes it to streamModelCompletion, the credential replay, and the terminal request normalization. The retry paths already called stream_result.deinit; with a distinct allocator those calls reclaim. On success the result is deep copied into the turn arena by copyStreamResultToTurnArena, which replaces the ownership flag flip: both Result variants are copied, including deferred usage references and failure diagnostics, and the copy is marked borrowed because the turn arena reclaims it on exit. dupeModelCompletion in types.zig is the shared completion copy; comptime field-count assertions on ModelCompletion, ProviderBilling, Completed, Failure, FailureDiagnostics, and DeferredUsageReference fail the build when a slice-bearing field is added without extending the copy. The failed attempt is still released after the backoff wait rather than before it, because the post-wait cancel branch reads the attempt's tool calls. That holds one attempt through one sleep and does not accumulate. Measured with the offline fake Codex reproduction against the previous build: 50 grep steps 176.9 MB to 47.8 MB peak RSS, 300 steps 869.3 MB to 96.0 MB, a parent with three one-off subagents at 100 steps each 1643 MB to 323 MB footprint with 13 MB of live large allocations at the end, and a run with one 429 before every tenth step 174.6 MB to 63.7 MB with every retry admitted. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
A model waiting for a child calls subagent inspect with a wait condition. executeModelInspection then polls every 100 ms for up to a minute, and each poll ran manager.execute with the tool call's allocator. That allocator is a per-call arena, so the child graph loaded by loadLockedGraph and the target child's cloned messages and events stayed allocated until the wait returned, and result.deinit reclaimed nothing. A child's event list grows while it works, so every snapshot was larger than the last. With three one-off children reading files for several minutes, a real Codex session on the attempt-arena build reached a 57 GiB footprint, the 2026-08-29 incident's magnitude; the incident parent's last durable state was this wait. Each poll now owns a c_allocator-backed arena and passes it to manager.execute. encodeResult already serializes the returned inspection into the caller's allocator, so nothing crosses the poll boundary by reference. Measured with the fake Codex reproduction, parent issuing two inspect waits per child, three children at 30 paced steps: the attempt-arena build reached a 14,336 MB footprint at 100 requests and was killed; this build completed 139 requests with a 244 MB peak that fell to 16 MB once the children finished. The same workload without inspect waits peaks at 115 MB on either build, which is why the earlier reproductions never showed this path. Claude-Session: https://claude.ai/code/session_01Azb4NQfWH2jBa3aGKUQLxN
iamken1204
force-pushed
the
fix-turn-arena-tool-retention
branch
from
August 30, 2026 02:42
0821880 to
990a9b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #483.
A long agent turn allocates from one arena that is freed only when the turn returns. Four kinds of transient memory were parked in it, and each on its own was enough for the OS to kill the process: the first report was 81 GB after a 23-minute turn, and a build carrying the first two fixes still reached 48 GiB with three one-off subagents running. Every fix uses the same shape: a scoped
c_allocator-backed arena for the scratch, one copy-out of the survivors into the longer-lived owner.Tool-call scratch (commit 1). Ordinary tools received the turn arena as their dispatch allocator, so every nested scratch arena inside a tool was backed by it and its
deinitreclaimed nothing:grep_fileskept thegit grepstdout plus every scanned candidate file,prepareModelOutputkept the sanitize and mask copies of every raw result. File mutations already ran on a per-call arena with copy-out; this applies that contract to every call. The orchestrator and the parallel execution hook give each call its own arena,executeToolCallAuthorizedthreads it into the dispatch context, andexecuteRegisteredToolcopies the survivors (body, status detail, tool result memory) out at the boundary.dupeToolResultMemorybecomes one shared deep copy intypes.zigwith a field-count tripwire.Checkpoint copies (commit 1).
persistRecoveryCheckpointdeep-copied every accumulated tool result into the turn arena one to two times per settled provider attempt, quadratic in step count. The copy now lives in a scratch arena freed when the checkpoint returns; every sink already serializes or dupes before returning.MCP error code lifetime (commit 2).
PromptRunResult.error_codeis a borrowed pointer, so the static"McpInputRequired"is assigned after the dispatch copy-out instead of pointing into the freed call arena.Provider-attempt scratch (commit 3). Every model attempt received the turn arena, so the request body, the
std.http.Clientstate, the provider-state replay parse of every prior assistant message, and the JSON DOM of every SSE event stayed until the turn returned. A long turn resends its growing context on every step, so this grew quadratically, and concurrent subagents in the same process multiplied it. Each attempt-loop iteration now owns an arena; the retry paths already calledResult.deinit, which now reclaims; on successcopyStreamResultToTurnArenadeep-copies bothResultvariants into the turn arena.dupeModelCompletioncarriescomptimefield-count assertions onModelCompletion,ProviderBilling,Completed,Failure,FailureDiagnostics, andDeferredUsageReference, so a new slice-bearing field fails the build until the copy covers it.Subagent inspect-wait polls (commit 4). A parent waiting on a child calls
subagent inspectwith a wait condition, andexecuteModelInspectionpolls every 100 ms for up to a minute. Each poll ranmanager.executewith the tool call's arena, loading every locked child's control record and cloning the target's messages and events, and a child's event list grows while it works. Each poll now owns its own arena;encodeResultalready serializes the returned inspection into the caller's allocator.Measurements
Offline reproduction against a fake Codex Responses endpoint (real
fxbinary, no network). "Before" is the build with commits 1 and 2 only.Real Codex sessions on the full patch stack: a 15-step
read_fileturn adds 9 MB of footprint and releases it on return; a parent waiting on three one-off subagents (22 threads alive, two 60 s wait timeouts) peaks at 69 MB and settles at 28 MB, where the pre-fix build passed 3.6 GB one minute into the same phase and 57 GiB before the children finished.Requests are byte-identical; permission, retry, and usage-ledger semantics are untouched.
Test plan
zig build testtypes.zig: field-count tripwires fordupeToolResultMemoryanddupeModelCompletiontool_runtime.zig: an ordinary tool runs with distinct call/result owners and the result is read after the call arena is destroyedorchestrator.zig: bothResultvariants are copied out of an attempt arena that is then destroyed, every surviving field read backfootprintwhile the turn runs