Repository navigation
Long OpenCode v2 streams no longer exhaust the daemon's memory - #6262
Conversation
[Medium impact] The PR appears safe to merge.
|
There was a problem hiding this comment.
Needs from you: answers
Product delta: none beyond the fix
Protocol delta: none
- Merge this now on the live evidence that the merge base kept 36 MB after one 50,863-character OpenCode reasoning part while the PR head had grown 1.1 MB when its run was stopped mid-turn, yes or no? (yes)
- Durability: none; the change is internal to
V2Timelineand commits no interface, type, or schema. - Specificity: no branches; one helper replaces
sliceat the two places the timeline cuts a streamed suffix. - Risk: ships beside everything else, since the copy keeps every UTF-16 code unit and live text, reconcile, history reload, and child-session text all matched what OpenCode stored.
- Serves: anyone streaming long reasoning on OpenCode 2.x, where retained heap grows with the square of the part length until the daemon worker runs out of memory and every agent on the host is interrupted, with no workaround short of a local patch, for a 9-line change.
- Recommendation: merge.
What it does. V2Timeline emits a detached UTF-16 copy of each streamed text or reasoning suffix instead of a slice of the accumulated message, so chunks kept in the daemon's timeline no longer hold every intermediate copy of the message.
Review
- Fixes what was reported: yes, on the reasoning path. The diagnose bot reproduced the reporter's measurement on
main(123 MiB retained for one 128 KB part, quadrupling when the length doubles, on both the delta and snapshot paths) and isolated it to the emitted suffixes. QA reproduced it live: on the merge base, one real 50,863-character reasoning part left the worker heap 36 MB larger after GC. The regression test measures retained heap throughV2TimelineintoTimelineProjection, which is the reported behavior, and goes from 128 MB to under the 4 MB bound. - Right fix: yes.
V2Timelineproduces the chunks the store keeps, so the copy belongs where the chunk is made, and both producing paths (delta()andtextPart()) are covered. I checked that theutf16leround trip preserves lone surrogates, split pairs, ZWJ sequences, flags, and CJK. A possible reshaping for later is to emitevent.deltadirectly on the live path when nothing was emitted ahead of the stream. I did not hold the merge on it. - Anything else: no. No protocol, schema, setting, or dependency change, and nothing visible to users. The child-process memory test follows the existing
session/owned-subscriptionspattern. - Worth shipping: yes. The change is small and contained, and the bug takes down the shared daemon worker.
QA checklist (isolated daemon from this branch, OpenCode 2.0.20, real sessions)
- Reported scenario live: not completed. On the merge base, a reasoning model (gpt-oss-20b, thinking high) streamed a 50,863-character reasoning part and worker heap after GC rose 323.5 MB → 350.6 MB mid-turn → 359.5 MB at completion (+36.0 MB). On the PR head, the same prompt grew heap 322.5 MB → 323.6 MB (+1.1 MB) before the run was stopped at its time budget while still reasoning, so its reasoning length is unknown. An earlier round used text replies instead (26,000–48,000 characters), which left +0.3 to +0.5 MB on both base and head, so the base leak did not show on final text replies. I did not trace what releases text rows and not reasoning rows.
- Regression test red/green: pass. On the merge base's
timeline.ts, both memory tests failed at 128,828,936 B (delta) and 128,311,384 B (snapshot) against a 4,096,000 B bound. On the head, all 4 tests passed. - Streamed emoji, flags, accented, and CJK text: pass. 4,537 characters were identical between Paseo's timeline and OpenCode's export, with no replacement characters.
- Text and tool calls alternating (reconcile mid-turn): pass. Each of the three text parts appeared once and in order.
- History after a daemon restart: pass. The reloaded text was identical to the live text (4,895 and 8,563 characters).
- Child session from the subagent tool: pass. 529 characters were identical to OpenCode's export of the child session.
Outside the checklist
npm run typecheckandnpm run lintwere clean.- The model ignored "reply in chat" once and wrote a file. This is model behavior, not counted.
- Some models were unavailable for the reasoning run. This is setup, not counted.
There are no review threads. The automated review reported no findings.
Decision: needs-maintainer. The fix is right and every other item passed, but checklist item 1 did not finish on the PR head. The merge base's live 36 MB was compared with a head run stopped mid-turn at +1.1 MB, not with a completed head turn of comparable reasoning length. One completed head run of the same reasoning prompt would settle it, or your answer above.
Reviewed commit: ae81149
|
Lifted |
…sage Each streamed text or reasoning chunk was cut from the accumulated message with slice, which can share the source string's storage. The timeline store keeps every chunk, so a 128 KB reasoning part retained about 128 MB: every chunk held the full message as it stood when the chunk arrived. Long OpenCode v2 reasoning streams exhausted the daemon worker's heap. The timeline now emits an independent copy of each suffix, so retained memory follows the length of the text.
ae81149 to
602b463
Compare
There was a problem hiding this comment.
Merged.
Product delta: none beyond the fix. A long OpenCode v2 reply or reasoning stream no longer keeps memory in the daemon that grows with the square of its length.
Protocol delta: none
What it does: V2Timeline emits each streamed text and reasoning chunk as an independent string instead of a slice of the accumulated message, so chunks kept in the timeline store no longer each hold the whole message.
Review
- Fixes what was reported: yes. The diagnose run reproduced the reporter's measurement on main (reproduction 4/5, diagnosis 5/5). After rebasing, I ran the regression test against main's
timeline.ts: both memory tests fail at 128.8 MB and 128.3 MB against a 4 MB bound. On the head, all 5 tests pass. The test measures retained heap after streaming a part throughV2TimelineintoTimelineProjection, which is the reported behavior. - Right fix: yes.
V2Timelineis where the suffix is cut, so the change sits at the layer that creates the retaining string. The UTF-16 copy keeps surrogate pairs split across chunks intact, which the new test and live QA item 3 both check. A wider reshaping, emittingevent.deltadirectly on the live path, would also be correct. It is not needed for this fix. - Anything else: no. It changes no protocol, schema, setting, dependency, design doc, or UI. The test spawns a local Node child on a fixture in the repository, the same pattern
session/owned-subscriptionsalready uses. - Worth shipping: yes. An out-of-memory crash takes down the shared daemon worker and interrupts every agent on the host. The fix is 9 lines in one file.
Rebase
The branch was 161 commits behind main. v2/timeline.test.ts conflicted because main added an image-output test in #6330, and I kept both sides' tests. timeline.ts applied cleanly, and typecheck is green.
QA checklist (isolated daemon and web app built from this branch, real OpenCode v2 sessions)
- Reported scenario live, head vs base: pass. I used the same reasoning prompt on the same model with thinking set to high, and measured worker heap after a forced GC before and after each completed turn.
- Head: 98,460-character reasoning part, +1.5 MB.
- Base (main's
timeline.ts): 100,741 characters, +6.0 MB. - The earlier QA run on this PR measured +36 MB on base for a 50,863-character part with another model, and its head run did not finish. This run's head turn completed, which settles the blocker from the earlier review.
- I had set the base bar at tens of MB. Base retention depends on how finely the model's stream is chunked, so this run's lower base figure says nothing against the fix. The head result is the one this item checks.
- Regression test red on base, green on head: pass. Red: 2 failed, 128.8 MB and 128.3 MB against the 4 MB bound. Green: 5 passed.
- Emoji, CJK, and ASCII reply intact in the app: pass. The rendered text matched OpenCode's stored text exactly: 4,162 UTF-16 code units, 60 lines, no replacement characters.
- Text, then a tool call, then text in one turn: pass. Each piece appears once, in order, with the read tool call between them.
- History after daemon restart: pass. The reloaded message and reasoning matched the live captures exactly.
- Reasoning block in the app: pass. The expanded Thinking text equals the concatenation of OpenCode's 26 stored reasoning parts, 1,302 code units.
Outside the checklist
- On base, a 36,636-character text reply retained +1.1 MB. This is consistent with item 1 and not counted.
- Some model endpoints were unavailable during setup. That is setup, not counted.
Review threads
None. Greptile reported no findings.
Reviewed commit: 602b463
Linked issue
Closes #5882
Type of change
Reasoning
A long OpenCode v2 reply or reasoning stream no longer grows the daemon's memory with the square of its length. Before this change, a single 128 KB reasoning part kept about 128 MB alive in the daemon, and long reasoning from OpenCode v2 agents could run the daemon worker out of memory, which interrupted every agent on that host.
The OpenCode v2 timeline turns each streamed chunk into a timeline item by cutting the new suffix from the accumulated message with
slice. V8 can share the source string's storage for a slice, so each 64-byte chunk kept the whole message, as it stood when the chunk arrived, alive. The daemon's timeline store keeps every chunk of the turn, so retained memory summed over all those message lengths. The timeline now emits an independent copy of each suffix. The copy uses UTF-16 code units so a surrogate pair split across two chunks still arrives intact.The fix is in
V2Timelinebecause it is the only provider path that cuts a suffix from its accumulated text on every streamed chunk. The otherslicesuffixes inopencode-agent.tsandcodex-app-server-agent.tsrun once per completed part, so they hold at most one copy of the text.Reported with a standalone reproduction by @Shunkleburger.
Goals
Non-goals
TimelineProjectionmerges chunks.QA
Before, on
mainat 3f1a043, using the reporter's provider-free measurement against the source: stream one reasoning part throughV2TimelineintoTimelineProjection, run GC, then measure the heap still in use.mainFeeding the projection fresh 64-byte strings directly retained 0.6 MiB for 128 KB, and keeping only
V2Timeline's emitted chunks in an array retained 122 MiB. So the retention comes from the emitted suffixes, not from the projection's merging.The new test
a long %s-streamed part holds memory in proportion to its textruns that measurement in a child Node process with--expose-gc(the patternsession/owned-subscriptionsalready uses). Its bound is 32× the text size. It fails onmainwith about 128 MB retained against a 4 MB bound on both paths, and passes with this change.I did not run an OpenCode 2.x binary through a live daemon. The code path from a
session.reasoning.deltaevent throughV2Timeline.delta()toAgentManager'stimelineStore.appendis the one the test exercises.npx vitest run src/server/agent/providers/opencode/: 15 files, 165 tests pass.npm run typecheck --workspace=@getpaseo/serverpasses.oxfmtandoxlintare clean on the changed files.Checklist
npm run typecheckpassesnpm run lintpassesnpm run formatpasses