[AksharP5]

Problem

Large terminal clipboard writes repeatedly scan the accumulated pending control string before publishing each PTY output chunk. Emitting a 256 KiB OSC 52 clipboard sequence in 4 KiB writes reproduces the extra work on Linux.

Change

Resume string-terminator searches at the new data, retaining the previous character for a split ESC/backslash terminator. Preserve the existing strip rules and exact saved history. The regression bounds character reads and checks large OSC 52/C1 DCS strings, split termination, later controls, persistence, and reopen.

Scope and approval

Fixes #23817
Fixes https://git.cafe/pingdotgg/t3code/issues/23817

Triaged bug via the automated issue flow. Reuses the existing fix PR and merges current main. No product setting, contract, or client behavior changes.

Verification

• Deterministic regression fails on main: 1,368,175 character reads exceed the 262,242 bound. With the fix: 123,066 reads.
• vp test run src/terminal/Manager.test.ts in apps/server: 43 passed after merging main.
• Server typecheck passed. Scoped lint passed with one existing warning at unchanged Manager.test.ts line 731. Diff whitespace check passed.
• Real dev server and node-pty, driven through the authenticated browser terminal RPC: a 349,536-byte OSC 52 sequence produced 24,417,215 parser character visits before and about 349,700 after. Both persisted histories retain the exact sequence. These counts do not measure UI latency; PTY chunk boundaries vary.
• Runtime transcript. Server-only change; fixture matrix is empty and no render paths change. Browser/native rendering captures do not apply.

Implemented and verified with GPT-6.1 Sol and Claude Opus 5.5 through Codex and Claude Agent SDK in T3 Code; original implementation by GPT-6.1 Sol and GPT-6 Astra through Codex.

[macroscopeapp]

Approvability

Verdict: Approved at 9d2e612

Macroscope's review found this PR approvable — This localized optimization reduces repeated scanning of large chunked terminal control sequences and includes regression coverage for persistence across reopen. It does not change product defaults, APIs, workflows, or static-analysis configuration.

^{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: fbc6a08
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5c1b2e4d-474a-4afd-bed2-6b51df12cbff

📥 Commits

Reviewing files that changed from the base of the PR and between 9d2e612 and 12cf2c9.

📒 Files selected for processing: 1

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

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

📝 Walkthrough

The terminal history scanner now limits terminator searches for pending string control sequences. A regression test checks large chunked OSC 52 and DCS sequences, persisted history, and reopened history.

Changes: Terminal history sanitization

Layer / File(s)	Summary
Bounded terminator scanning and history validation apps/server/src/terminal/Manager.ts, apps/server/src/terminal/Manager.test.ts	The scanner starts terminator searches at the later of the sequence body start and the pending-prefix scan bound for ESC- and C1-prefixed sequences. The test checks large chunked OSC 52 and DCS output, persisted history, and reopened history.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to 12cf2

The terminal history change is ready to merge after normal checks; no actionable risk remains identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "perf(server): speed up large terminal clipboard sequences" states the main change: the server sanitizer now resumes terminator searches at new data, so large OSC 52 clipboard sequences are …
Linked Issues check		Issue #23817 requires that sanitizeTerminalHistoryChunk scan only new bytes for a string terminator. It also requires that split terminators and exact saved history stay correct. The whole-PR summar…
Out of Scope Changes check		The whole-PR summary lists two changed files: apps/server/src/terminal/Manager.ts (+10/-2) and apps/server/src/terminal/Manager.test.ts (+56/-0). The production change limits terminator searches i…
Description check		The pull request description includes all required sections. It clearly states the problem, change, linked issue and approval context, focused verification results, limitations, and agent attribution.

Full details: Title check

Explanation

The title "perf(server): speed up large terminal clipboard sequences" states the main change: the server sanitizer now resumes terminator searches at new data, so large OSC 52 clipboard sequences are processed faster. It uses a conventional commit prefix and is concise and specific. The title does not mention the split-terminator handling, but that is a detail.

Full details: Linked Issues check

Explanation

Issue #23817 requires that sanitizeTerminalHistoryChunk scan only new bytes for a string terminator. It also requires that split terminators and exact saved history stay correct. The whole-PR summary (merge base to reviewed head) states that apps/server/src/terminal/Manager.ts now limits terminator searches to the sequence body or the pending prefix's final character. This covers ESC-prefixed and C1-prefixed sequences. The summary states that the regression test in apps/server/src/terminal/Manager.test.ts covers large chunked OSC 52 and DCS strings, split termination, stripped query strings, preserved sequences, persistence, and reopen. The PR description reports a deterministic failure on main (1,368,175 character reads against a 262,242 bound) and 123,066 reads with the fix. It also reports 43 passing manager tests. Limit: the sanitizer diff and the reviewed-head file could not be read directly. The file read was truncated before the sanitizer, and the diff read failed because changed blobs were missing. This verdict therefore rests on the whole-PR summary and the reported test results, not on direct inspection of the current code. The previous assessment at 9d2e612 also passed. Changes between that head and 12cf2c9 could not be inspected, so their effect on the verdict is not established.

Full details: Out of Scope Changes check

Explanation

The whole-PR summary lists two changed files: apps/server/src/terminal/Manager.ts (+10/-2) and apps/server/src/terminal/Manager.test.ts (+56/-0). The production change limits terminator searches in sanitizeTerminalHistoryChunk, which is the behavior requested by #23817. The test change adds a regression for that behavior. The PR description states that no product settings, contracts, or client behavior change. The summary reports no public declaration changes. Limit: the diff could not be read directly, so the absence of unrelated hunks rests on the summary and the PR description. The previous assessment also found no out-of-scope changes.

✨ 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.}