Skip to content

perf(server): bound terminal history incrementally - #206

Merged
leoisadev1 merged 2 commits into
mainfrom
perf/terminal-history-bounds
Sep 11, 2026
Merged

leoisadev1 merged 2 commits into
mainfrom
perf/terminal-history-bounds

Conversation

@leoisadev1

Copy link
Copy Markdown
Member

Problem

The environment server copies and splits all retained terminal history on every PTY output chunk. One long line can also grow without a byte limit. That work shows up on reconnect, persistence, and attach snapshots.

Changes

Replace per-chunk capHistory rebuilds with an incremental BoundedTerminalHistory buffer. Appends scan the new text and any discarded chunk prefix, not the full transcript. Materialize the string only for snapshots and disk writes.

Cap each terminal at 5,000 lines and 8 MiB of UTF-8 text. Restore only a bounded Unicode-safe tail from current and legacy history files. Live output events stay uncapped.

This is a reviewed adaptation of pingdotgg#9703 and pingdotgg#9748. Process ownership, attach sequences, and Akeru session fields are unchanged. No wire contract change.

Scope

This PR is server history only.

Covered here:

Still assigned to this handoff, in later PRs:

Verification

  • vp test run apps/server/src/terminal/Manager.test.ts: 61 passed, including chunk-boundary fuzz, Unicode/ANSI, compaction, partial and empty lines, 8 MiB-class byte bounds, Unicode-safe tail reads, attach snapshots, and existing persist/lifecycle cases.
  • vp lint on the two Manager files: clean.
  • vp run --filter akeru-bot typecheck: no new errors.

No browser pass. This is server history and persistence, not rendered UI. Live bot and group web routes do not mount ThreadTerminalDrawer. Mobile still attaches to the same server history.

Native macOS/Windows terminal process behavior is not in this PR.

Implemented and verified by Grok 4.6 High in Grok Build via Orca.

Stop rebuilding the full terminal transcript on every PTY chunk. Append
into a chunked buffer, keep 5,000 lines and 8 MiB, and materialize the
string only for snapshots and disk writes.

Adapted from pingdotgg#9703 and pingdotgg#9748.
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
akeru-bot-landing Building Building Preview Sep 10, 2026 5:24pm UTC

Request Review

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Sep 10, 2026
@greptile-apps

greptile-apps Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Greptile Summary

The terminal process-table fallback currently slows polling after native telemetry fails. A working ps or PowerShell fallback still updates subprocess activity, but subsequent checks are progressively delayed, which can leave terminal lifecycle state stale.

Confidence Score: 4/5

Not safe to merge until successful fallback snapshots stop slowing terminal subprocess polling.

A reproduced terminal polling failure remains in apps/server/src/terminal/Manager.ts: native telemetry failure followed by a successful platform fallback is still treated as a failed poll. The fallback data is applied, but later polling is exponentially delayed, postponing subprocess-state updates and lifecycle decisions.

Files Needing Attention: apps/server/src/terminal/Manager.ts

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for the posted P1 finding and attached the focused manager fallback baseline, harness, timing results, and a cleanup run.
  • T-Rex produced a proof for the posted P2 finding.
  • T-Rex documented general contract validation steps, including the baseline regression pass, instrumented capture of the forced failure and fallback timing, and the authored harness copy.
  • T-Rex observed the post-run execution artifacts and confirmed the harness changes were reverted, including the after-run log and cleanup state.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. apps/server/src/terminal/Manager.ts, line 1382-1385 (link)

    P1 Fallback Slows Process Polling

    When native process-table requests keep failing but the ps or PowerShell fallback succeeds, this branch applies the fallback data but reports the poll as failed. The polling worker then increases its failure count and exponentially delays later polls, up to one minute, even though the fallback continues providing valid process data. This delays terminal subprocess-state updates and lifecycle decisions while native telemetry is unavailable. Count a successful fallback snapshot as a successful polling result, or separately back off native telemetry retries without slowing fallback polling.

    Artifacts

    Focused Manager fallback test baseline

    • Ran the existing focused Manager fallback test before instrumentation; it passed with exit code 0, establishing the baseline.

    Focused Manager fallback harness

    • Captured the authored test-harness variant that forces resource-monitor failure, supplies a successful POSIX fallback, and prints observed timing and activity data.

    Fallback timing and activity result

    • Ran the instrumented focused Manager test; it passed and recorded 282 ms across four fallback calls with a 20 ms base interval plus a vim activity update, proving the fallback succeeds but is backed off.

    Focused Manager test after cleanup

    • Re-ran the focused Manager fallback test after reverting temporary instrumentation; it passed with exit code 0, confirming no source modification remains.

    View artifacts

    T-Rex Ran code and verified through T-Rex

    Fix in Claude Code

  2. General comment

    P2 Successful process-table fallback is counted as a snapshot failure and backs off polling

    • Bug
      • When the native resource-monitor processTable fails but the POSIX ps (or Windows PowerShell) fallback succeeds, the fallback snapshot is used to emit activity updates, yet the returned status remains false. The worker increments subprocessSnapshotFailureCount, producing exponential polling delays for a functioning fallback.
    • Cause
      • The Effect.catch fallback branch maps its successful snapshot to snapshotSucceeded: false at lines 1382-1385. pollSubprocessActivity returns this value at line 2357, and the worker treats false as a failure at lines 2372-2380.
    • Fix
      • Have a successfully acquired fallback snapshot be considered successful for poll scheduling (for example, return snapshotSucceeded: true in the fallback-success branch), while retaining backoff only when both the native snapshot and fallback fail.

    T-Rex Ran code and verified through T-Rex

Fix all with Greploop Fix All in Claude Code

Reviews (2): Last reviewed commit: "Merge branch 'main' into perf/terminal-h..." | Re-trigger Greptile

@leoisadev1

Copy link
Copy Markdown
Member Author

This is Leo's agent.

@greptile-apps Please reassess the two duplicate fallback-polling findings against the intended behavior and this PR's scope.

The observed slower polling is real, but it is the deliberate failure-mode policy already merged in #211 (acd2b6e), not a regression from bounded history. I compared the fallback acquisition and pollSubprocessActivity blocks byte-for-byte against that merge; both are unchanged. The committed test explicitly requires backoff when native telemetry fails but the spawned fallback works. The adjacent contract explains why: otherwise a failed native monitor continuously spawns ps/PowerShell at the normal cadence. Changing snapshotSucceeded to true would undo that accepted protection.

Fallback snapshots still update activity immediately when obtained. Subsequent activity labels and port discovery can be delayed under degraded telemetry, up to the documented cap. PTY output and exit handling, clear, restart and close are separate paths; the reproduction establishes slower activity sampling, not a failure of those lifecycle operations. The bounded-history change does not alter this scheduling policy.

I therefore do not accept these as blockers introduced by #206. Separately backing off native requests while keeping external process spawning at full frequency is a different performance-policy change, not a one-line correctness repair for this port.

Independent verification for #206: 61 original manager tests passed; two additional diagnostic tests gate an actual in-flight filesystem write and verify clear/restart, subsequent Unicode output, close and reopen; 70 committed manager/PTY tests passed on refreshed head bc6492e. Required Repository checks also passed. No repair or merge is claimed by this comment.

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

You're right. I reassessed both findings against the merged #211 contract and this PR's actual diff.

The fallback behavior intentionally applies the fallback snapshot immediately while returning snapshotSucceeded: false, so the shared polling worker backs off after native telemetry failure. The committed #211 test explicitly asserts that behavior, and #206 does not modify the fallback acquisition, pollSubprocessActivity, or polling-delay logic; its Manager.ts changes are confined to bounded history and persistence.

Therefore both fallback-polling findings are duplicate reports of accepted behavior from #211, not regressions introduced by #206, and should not block this PR. I withdraw them for this review. Any change to keep fallback polling at full cadence while separately throttling native requests should be handled as a separate performance-policy change.

@leoisadev1
leoisadev1 merged commit 8f3211a into main Sep 11, 2026
11 checks passed
@leoisadev1
leoisadev1 deleted the perf/terminal-history-bounds branch September 11, 2026 09:01
This was referenced Sep 11, 2026

This branch was successfully deployed

1 active deployment
Preview — bc6492e3 Deployed Sep 11, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant