[nateEc]

Terminal output currently rewrites all retained history on every debounced flush. At the cap, even a 1 KiB update rewrites 8 MiB. This is the disk-write amplification reproduced by maintainers in #20535.

Append only the output since the last successful flush. Compact back to the existing bounded history when the file would exceed twice the byte limit, when pending output was evicted, after clear, or when a split surrogate changes already-encoded text. Immediate lifecycle flushes still write the exact capped history. A failed append invalidates its cursor so the next flush repairs the file from a full snapshot.

The file remains plain text; replay limits, debounce, live output, and client/provider contracts are unchanged. While a terminal is active, its file can temporarily reach 16 MiB with the default 8 MiB replay limit. No new format or migration is required.

Scope and approval: maintainer triage and runtime reproduction establish the existing failure. This PR only changes terminal-history persistence for that defect. That triage is not approval of this particular compaction design: the temporary 2× disk bound and recovery algorithm still require human review, as Macroscope noted. No explicit approval of that design is claimed.

Verification (macOS arm64, Node 26.12.1, isolated temporary state):

• Added a failing regression using the real TerminalManager and filesystem, with a fake PTY emitting deterministic output. At 64 KiB retained history plus sixteen 1 KiB updates, old code wrote 1,114,112 bytes and failed the <131,072-byte assertion; the new code passes and restores the exact expected history.
• Runtime trace: open -> fill -> 16 acknowledged writes -> close -> reopen; retained history is 49,152 a bytes followed by 16,384 b bytes. No sleeps/polling in the test; it awaits filesystem write acknowledgments.
• Covered repeated compaction, line/byte trimming, clears, split Unicode, output exceeding the retained window, and a real filesystem error after a partial append followed by recovery.
• vp test run apps/server/src/terminal/Manager.test.ts: 45 passed.
• vp run check:changed: format, lint, knip, generated GraphQL, affected typechecks, and focused tests passed. Existing unrelated lint warning remains.

Maria's Linux runtime verification on unchanged head 9923e5b measured the actual terminal log after an 8 MiB workload and sixteen 1 KiB updates: 134,217,728 bytes written on main versus 16,453 bytes on this PR for those final flushes. It also verified repeated compaction, abrupt server restart/reconnect, clear, Unicode output and browser reload; all 46 terminal-manager tests passed in that environment. The linked report includes the command transcript and client recordings.

This is a server-only fix shared by web, desktop, mobile and remote clients. The runtime evidence measures terminal-log writes, not whole-app CPU/throughput. Native Electron/mobile, Windows/macOS live workloads, relay/tunnel and real disk-full/power-loss conditions remain unverified. The persistence design still requires human approval.

Fixes #20535.

Model: GPT-5-based Codex. Harness: Codex desktop.

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This server-side fix replaces full terminal-history rewrites with stateful append and compaction behavior, including recovery from partial filesystem writes and temporary growth beyond the configured history limit. The scope is focused and tested, but the persistence algorithm materially changes an existing production path and merits human review.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 53896ae
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5f9ce7d0-a50b-42f9-8dd7-6f4c19cd9512

Commits

Reviewing files that changed from the base of the PR and between 5015dbd and 9923e5b.

Files selected for processing: 2

• apps/server/src/terminal/Manager.test.ts
• apps/server/src/terminal/Manager.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Terminal history persistence now uses cursor-based updates. The worker appends new data when the cursor is valid and the file remains within the configured size bound. It replaces the file with a full history snapshot when the cursor is invalid or the bound would be exceeded.

Changes: Terminal history persistence

Layer / File(s)	Summary
Track bounded history updates apps/server/src/terminal/Manager.ts, apps/server/src/terminal/Manager.test.ts	BoundedTerminalHistory tracks generations and cumulative appended text. Its persistenceUpdate method returns new text when the cursor remains valid, or a full history snapshot otherwise. Tests cover compaction, clearing, split-surrogate recovery, and output beyond the retained window.
Persist history incrementally apps/server/src/terminal/Manager.ts, apps/server/src/terminal/Manager.test.ts	The worker appends valid updates and replaces the file when required. It saves the cursor only after a successful write. Tests check persisted content, write volume, and recovery after a partial append failure.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• pingdotgg/t3code#24261: Changes cancellation handling in the terminal-history sanitizer, which affects the history data persisted and restored by this path.
• pingdotgg/t3code#25193: Changes which terminal output the history parser retains, affecting data handled by this persistence path.
• pingdotgg/t3code#24586: Changes when terminal history persistence runs for temporary threads.

Suggested labels: bugfix

Suggested reviewers: aksharp5, t3dotgg

Merge Risk: Minimal · up to 9923e

The persistence change preserves bounded history in the inspected paths. No issue requiring resolution before merge was identified.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description includes the required Problem, Change, Scope and approval, and Verification content. However, the Scope section links only to issue triage and explicitly states that the cited discussi…	Add a link to an explicit maintainer approval comment for the append and compaction design, including its temporary 2× disk bound and recovery behavior. If the change qualifies as an obvious focused bug fix without prior approval, explain w…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title follows conventional commit form and names the main change: terminal output is now persisted by appending instead of rewriting the full retained history. It matches the Manager.ts persistenc…
Linked Issues check		[#20535] The implementation reduces terminal-history write amplification. persistenceUpdate appends only the new retained output when the cursor is valid and the file stays within the temporary 2× b…
Out of Scope Changes check		The changes remain within [#20535]. Manager.ts changes terminal-history cursor tracking, append persistence, compaction, and recovery. Manager.test.ts adds persistence regression coverage. The sum…

Full details: Title check

Explanation

The title follows conventional commit form and names the main change: terminal output is now persisted by appending instead of rewriting the full retained history. It matches the Manager.ts persistence changes and the linked issue #20535. The title is 66 characters, within a typical length, and it is specific to the server-side fix.

Full details: Linked Issues check

Explanation

[#20535] The implementation reduces terminal-history write amplification. persistenceUpdate appends only the new retained output when the cursor is valid and the file stays within the temporary 2× byte bound. It requests a full snapshot after compaction conditions, clear, split-surrogate changes, eviction, or cursor invalidation. Immediate flushes write the capped history. The existing history limits and debounce remain unchanged. Manager.test.ts covers append persistence, compaction, clear, split Unicode, oversized output, and recovery after a partial append failure. The reported focused tests passed.

Full details: Out of Scope Changes check

Explanation

The changes remain within [#20535]. Manager.ts changes terminal-history cursor tracking, append persistence, compaction, and recovery. Manager.test.ts adds persistence regression coverage. The summaries show no changes to UI behavior, client or provider contracts, replay limits, debounce behavior, or unrelated server functionality.

Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification content. However, the Scope section links only to issue triage and explicitly states that the cited discussion does not approve this compaction design. The template requires explicit maintainer approval of the direction and scope unless the change qualifies for a documented exemption.

Resolution

Add a link to an explicit maintainer approval comment for the append and compaction design, including its temporary 2× disk bound and recovery behavior. If the change qualifies as an obvious focused bug fix without prior approval, explain why that exemption applies instead.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

kept @nateEc's implementation and commits, merged current main (53896aea76), removed three non-null assertions from the persistence result, and strengthened the existing regression to restore an un-compacted 80 kib log through a fresh manager with a 64 kib replay cap. no new test files or unrelated changes. the full diff remains two terminal manager files.

verification at 9923e5b2e6fea248828bc08554acb0628eb01b50:

• reproduced on current main: the original regression fails with 1,114,112 bytes written against a 131,072-byte ceiling.
• blacksmith: all 46 manager tests pass, including partial-append failure recovery, clears, unicode, compaction, and cold replay. vp run check:changed passes format, lint, both knip checks, graphql, affected typechecks, and focused tests. one existing lint warning remains.
• github checks settled successfully, with expected skips. coderabbit reports no actionable findings; macroscope correctness passes. two independent read-only reviews approve this exact head.
• unverified: live pty/client/provider interaction, desktop/mobile, relay/tunnel, crash/power-loss durability, and whole-app performance. this is server-only persistence work, with no ui change or visual evidence.

review decision: active logs can reach 2× the replay cap, 16 mib per terminal at the default 8 mib cap. another append beyond that bound rewrites the capped snapshot; lifecycle flushes also compact. cold reads cap the tail. compaction retains the existing non-atomic truncate/write behavior, and failed append recovery requires a later flush. macroscope still requests human approval of this persistence design. that approval is not claimed here.

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 9923e5b2e6fea248828bc08554acb0628eb01b50 on a disposable linux vm, after reproducing on github/main at 9bc4138923. ran one dev server at a time with isolated state, repo playwright, and /usr/bin/google-chrome. created the project and threads through the app against a real git repo. real codex gpt-6.1-sol turns completed on both revisions.

• typed an 8 mib terminal workload followed by sixteen 1 kib updates through the web terminal drawer. syscall traces of the actual terminal log showed 134,217,728 bytes written on main versus 16,453 bytes on this head for the final sixteen flushes. main used truncate/write; the pr used append. the final update includes the completion marker and shell prompt. this measures log writes, with no cpu or whole-app throughput claim.
• streamed another 16 mib through the same terminal. compaction occurred twice; the largest completed file was 16,270,004 bytes, below the 16 mib bound, and the final snapshot was 8 mib.
• abruptly stopped and restarted the server with an 8,405,330-byte append log. the reconnected client restored an exact 524,160-byte suffix plus the existing input-reset sequence, preserving the completion marker within the existing 512 kib cold replay window.
• authenticated terminal.clear returned success and persisted zero bytes. subsequent unicode output survived a browser reload without bringing old history back. the rocket's utf-8 bytes persisted correctly; this vm's terminal font shows a missing glyph.
• reran the existing terminal-manager suite through blacksmith: 46 tests passed. github checks on this head have no failures or pending runs, with expected skips and a neutral design-approval check. no source changes, commits, or pushes were needed.

command transcript

main, after the terminal workload:

pr, after the same workload:

terminal output and drawer reopen, recorded in the real client:

Not verified

native electron, ios/android, macos/windows, relay/tunnel, other providers, real disk-full/permission-error/power-loss conditions, and whole-app performance. partial-append failure recovery was covered by the existing focused tests, not a live disk failure. the persistence design still needs the human approval already requested in the discussion; this verification does not claim that approval.