Terminal attach budget: bound replays, fast reload, pong fix (WI-121) - #676
Draft
thedancingdeveloper wants to merge 8 commits into
Draft
Terminal attach budget: bound replays, fast reload, pong fix (WI-121)#676thedancingdeveloper wants to merge 8 commits into
thedancingdeveloper wants to merge 8 commits into
Conversation
Add the integration-test scaffolding the terminal-attach-budget work (WI-121) builds on, driving real bytes through the PTY reader, the 1024-slot broadcast channel and the outbound coalesce path rather than the HTTP fill helper: - boot_with(scrollback_bytes): choose the ring size per test; the suite default stays 64 KiB. - load_session_command(count): a bash loop emitting coloured, sequence-numbered lines (a monotonic SEQ spine for gap/duplicate detection) with periodic cursor-home frames, then idling so the ring is stable for a reattach. - read_snapshot now returns (reset, len, first_bytes); assert_ground_state_ aligned pins the invariant that a snapshot never begins on a UTF-8 continuation byte or in a chopped escape fragment. Tests: - large_stale_warm_reattach_today_floods_the_whole_ring (64 KiB ring) and four_mib_ring_stale_reattach_is_ground_state_aligned_and_bounded (4 MiB ring, ~6.5 MiB pushed): pin today's aged-out-cursor flood (full untrimmed ring, reset:true) with a comment marking the F1 (WI-125) size inversion; the ground-state alignment assertion holds across that change. - retained_cursor_deltas_are_byte_exact_under_load: two warm reattaches from retained cursors Ca < Cb resolve to byte-exact, gap-free, duplicate-free deltas (the later delta equals the earlier past the Cb-Ca boundary). - lagging_subscriber_recovers_in_band_bounded_and_without_duplicates: a client that stops reading while a load session floods past the broadcast capacity is recovered in-band (send_resync), bounded by the ring, spine strictly increasing (no duplicate/reorder across the resync boundary). Each test runs in ~3.5 s, well under the 10 s guard. Full integration suite green (103 passed). WI-122. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
The stale warm reattach flood: when a client's resume cursor had aged out of the scrollback ring, snapshot_for_attach returned the FULL untrimmed ring with reset:true — up to 4 MiB parsed into an xterm that keeps 5000 lines, 4x worse than a cold attach, and above a screen the client already had stale. An oversized in-window delta had the same problem. Engine (pty.rs): snapshot_tail_bytes is now the replay budget on every path. A warm reattach whose cursor is retained and whose delta fits the budget is still returned byte-exact with reset:false; a delta that aged out OR exceeds the budget returns a ground-state-aligned tail of at most the budget with reset:true. The in-band resync path (tail_bytes=None) is unchanged. Web (api.ts): openAttach always sends snapshot_tail_bytes: REPLAY_TAIL_MAX_BYTES alongside resume_from, so warm reattaches are bounded too. Terminal.tsx already resets and clears its cache on any reset:true; the snapshot-start position arithmetic is extracted into snapshotStartPosition() so the re-anchor after a reset-following-a-resume is unit-tested (the client discards its stale cursor, re-anchors to scrollback_pos - scrollback_bytes, and resumes gap-free). Tests: - H1 (a)/(b) "before" flood assertions flipped to bounded (len <= budget, reset:true, ground-state aligned). - warm_attach_is_not_narrowed_by_a_tail_hint inverted to warm_attach_over_budget_is_a_bounded_reset; new warm_attach_within_budget_stays_a_byte_exact_delta keeps the reset:false byte-exact case. - web: snapshotStartPosition.test.ts; attachAuthFrame.test.ts updated for the always-sent budget. Verification: cargo test -p vogt-engine-server --test integration 104 passed; fmt + clippy clean; web typecheck clean; pnpm vitest 906 passed; scripts/check_docs.py clean. docs/ENGINE.md §5 documents the budget contract. WI-125. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
On reload every retained terminal tab replayed its cached tail (up to 1 MiB) immediately, through the single global replay FIFO, round-robin with the active pane. With N retained tabs the pane the user is actually looking at got 1/N of the parser — the reload-is-slow half of the terminal budget bug. A parked pane (a retained tab that is not active, or the unfocused half of a split) now keeps its prepared cache tail in memory and replays it lazily on first activation in resumeSocket(), before connect(), instead of scheduling it into the shared FIFO on load. The active pane still replays immediately, so its restore is no longer time-sliced against tabs the user cannot see. The cache position is adopted at load time so a warm reattach resumes correctly even if the deferred replay is later cancelled by a re-park. The decision is a pure helper, shouldDeferCacheReplay(parked, hasCachedBytes), unit-tested here; the six-cached-tabs end-to-end assertion lands with H3's mocked Playwright spec (WI-124). Verification: pnpm typecheck clean; pnpm vitest 909 passed. WI-127. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
The liveness probe could force a spurious reconnect on a bursty session. The inbound task answered a ping with scrollback_position() (total_written) and handed the pong to the outbound task on a separate channel; select! could send that pong before the broadcast chunks it referenced, so the client briefly saw serverPos > localPos, printed [disconnected], and reattached. Engine (ws.rs): the ping id is forwarded to the outbound task, which answers with sent_pos — the offset actually written to this socket — after flushing anything already queued (new flush_available helper). The pong is therefore ordered after those chunks on the wire and its pos can never exceed what the client has received. ENGINE.md documents the pong pos semantics. Web (terminalWatchdog.ts): a single behind pong is a suspect, not a verdict — it arms a prompt confirm probe rather than recycling. Only a second behind pong within WATCHDOG_TIMEOUT_MS, with no output in between, is treated as a real stall; any output clears the suspicion. (terminalWatchdog.test.ts already existed, contrary to the plan note; extended with the two-strike cases.) Tests: - engine pong_never_reports_the_server_ahead_of_this_socket: under load, ping mid-stream and assert pong.pos <= snapshot_end + live bytes received on the socket. - web: five two-strike watchdog cases (suspect, confirm-recycle, output clears, caught-up clears, stale-suspect drop). Verification: cargo test -p vogt-engine-server --test integration 105 passed; fmt + clippy clean; web typecheck clean; pnpm vitest 913 passed; scripts/check_docs.py clean. WI-126. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
The ground-state trimming that both the server ring and the client cache rely on was only ever tested against synthetic `line\n` data. Add a corpus of real PTY captures and a fidelity suite that exercises it against genuine escape sequences, SGR colour, cursor moves, alt-screen switches and UTF-8. - web/tests/fixtures/transcripts/: claude-code-tui (curses alt-screen redraws: cursor addressing, SGR, OSC title, box/emoji UTF-8 — a seamless, newline-free stream), shell-plain (coloured recursive ls + multibyte chars), cargo-build (real `cargo build -v` colour output with \r). Gzipped; a README documents the intended production source (GET /api/history/:id/download from a dev stack) and how these local stand-ins were captured. - scripts/capture_transcript.py: a pty.fork capture helper. - scripts/sanitise_transcript.py: rewrites home paths/tokens/JWTs/emails to inert placeholders and ASSERTS no secret pattern survives before a fixture is committed. - src/__tests__/transcriptFidelity.test.ts: over each corpus at 4 KiB cut offsets, groundStateReplayStart / prepareReplayTail / sliceForReplay pick a start that is never inside an escape or on a UTF-8 continuation byte (just past a line feed, or start-of-stream), and the kept tail is byte-for-byte the source suffix. The alt-screen corpus documents the no-newline-seam case where raw-byte trimming can only fall back — the gap F5's serialized-state restore closes. The xterm serialize()-compare fidelity and the parse-rate baseline (both need @xterm/addon-serialize) fold into F5 (WI-129), where that addon becomes a real dependency, rather than adding a test-only copy here. Verification: pnpm vitest 928 passed (15 new); ruff check + format clean on the scripts; scripts/check_docs.py clean. WI-123. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
…tore
On reload a pane re-parsed its cached raw scrollback tail into xterm — the
per-pane parse cost F1 already bounds, but still a full re-parse. F5 persists
the pane's xterm screen + a capped scrollback via @xterm/addon-serialize and
restores it in a single term.write, no raw re-parse, then reattaches with the
F1-bounded resume_from delta.
Cache format (terminalCache.ts): an entry now holds EITHER a serialized screen
(a live pane, the fast path) OR raw bytes. Raw is still written by the headless
pre-warm path (terminalPrewarm has no xterm to serialize) and as a live-pane
fallback when serialization is unavailable, and it still reads a pre-F5 entry —
so the format change needs no DB version bump or migration (a deviation from
the plan's "bump + drop v1", forced by the pre-warm path the plan did not
account for; keeping raw as a first-class shape is strictly more compatible).
The ArrayBuffer check is now realm-safe.
Restore (Terminal.tsx): restoreCache() writes a serialized entry in one go, or
re-parses a ground-state raw tail through the shared queue, and it composes with
F3 — a parked tab defers the whole entry and restores on first activation. The
serialized scrollback is capped at 2000 lines (SERIALIZE_MAX_SCROLLBACK), well
under the 5000-line buffer, to bound the cache entry and the restore write.
Tests:
- serializeRestore.test.ts: over the real corpus (H2), serialize→restore→
reserialize is a fixed point (screen + scrollback preserved) through the same
core VT parser (@xterm/headless); the cap holds. This also lands H2's deferred
serialize()-compare fidelity.
- terminalCacheRoundtrip.test.ts (fake-indexeddb): serialized and raw entries
round-trip, raw is ground-state trimmed, a malformed entry reads as cold.
- terminalPrewarmAttach updated for the { data } payload shape.
Verification: pnpm typecheck clean; pnpm vitest 938 passed (10 new); pnpm build
bundles addon-serialize. WI-129.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
…now" Answer the D6 design question (WI-130) in a new docs/ROADMAP.md: should the engine run a headless terminal emulator per session so a cold attach ships screen state instead of byte history? - Crate comparison: vt100 (best fit — minimal grid, smallest surface), alacritty_terminal and termwiz (heavier, renderer/feature surface a snapshot server does not need). - Grounded per-session cost at 8 sessions: ~1.7–3.4 MB grid each (~14–27 MB total) ON TOP of the raw ring, plus continuous VT parse CPU on the PTY hot path — a cost the raw-ring design pays only per attach. No live number taken (no stack reachable); H1's load session is the measurement path if revisited. - Wire shape (snapshot-start kind:"screen" + server escape stream), the server-VT-vs-xterm fidelity risk and how the H2 harness proves it, and the modest mobile win (F5 already gives a bounded one-write restore). Recommendation: not now. F1–F5 (+F4) address both operator symptoms by bounding and client-side serialization without doubling engine memory or adding hot-path CPU. Revisit only if, once shipped and measured, mobile cold attach is still bottlenecked on the tail, the client cache proves untenable, or a new server-authoritative-screen requirement appears. No new work items filed. WI-130. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
This was referenced Sep 9, 2026
thedancingdeveloper
changed the title
Add web/tests/browser/terminalReplay.spec.ts, the acceptance harness the terminal-attach-budget fixes are measured against (WI-124). Mocked project (desktop/phone, Vite dev server) — a new routeWebSocket harness streams a 1 MiB corpus (built from the H2 transcript) into a REAL terminal as a cold snapshot, then asserts the per-pane `vogt-terminal-replay.snapshot` performance measure exists and is under the CI budget (2500 ms), and that the `[vogt] terminal replay` telemetry the live spec reads was logged. This is the first WebSocket-mocked terminal test in the suite; it is stable and box- independent (the measure's existence proves the bounded snapshot rendered; the budget has generous headroom over the ~5-6 MB/s parse rate). Live project (PLAYWRIGHT_LIVE_BASE_URL only) — drives a real load session via POST /api/sessions and asserts the two operator symptoms directly: switching away past the ring and back replays <= budget with no reset and no `[disconnected]` (F1/F4/F2), and a reload keeps the active pane under budget (F3/F5). Gated to the `live` project, so mocked runs skip it. e2e wiring: the playwright container already runs `playwright test --project=live` over the whole testDir, so this spec is picked up automatically; the live-step comment is updated to name it. Not added to demo gating. Verified: the mocked test passes and is stable across repeated runs; the live tests skip in the mocked projects. The live half runs against a real stack in e2e.yml (advisory/continue-on-error there) — an agent session cannot reach one, so its green/red-before-F1 confirmation is owed to a stack run. WI-124. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb
thedancingdeveloper
force-pushed
the
feat/terminal-attach-budget
branch
from
September 9, 2026 21:27
bc6d335 to
e9a640b
Compare
thedancingdeveloper
changed the title
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
1 participant
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 the two operator symptoms on
vogt.sprooty.com0.6.2 (WI-121): switching sessions re-streams MBs of scrollback, and a fresh open / reload is slow. One branch, eight commits, reviewable per-commit.Scope note — F4 (dormant panes) deferred
The plan had 10 items; this PR lands 8. F4 (dormant panes, WI-128) is deferred: it's a socket-lifecycle change whose multi-pane behavior proved environment-timing-sensitive (it made the
demo-agentscreenshot render differently on CI vs local — a flaky-gate risk) and the plan gated its acceptance on live review. The remaining items already fix both symptoms; F4 is a further optimization to land after live verification. D6 (WI-130) is a design-spike doc.The eight commits (each = one item, review in order)
4705423488a320402b523e7320e0sent_pos(nottotal_written) + watchdog two-strike → no spurious[disconnected]b293632968d8da249cb6ddocs/ROADMAP.md): engine-side VT → "not now"e9a640bNet: F1 bounds the flood, F3 + F5 make reload fast, F2 stops the false disconnect.
Gates — all green
cargo fmt/clippy --workspace --all-targets/test --workspacepnpm typecheck·pnpm vitest(938 passed) ·pnpm builduvruffcheck+format ·mypy --strict·pytest(1296 passed, 91.55%) · product-versionscripts/check_docs.py;demo.spec.tsscreenshots green (original baselines, no drift)origin/main(380950b)After merge
🤖 Generated with Claude Code
https://claude.ai/code/session_01UtEFLJAhLiq4NfZCN2vuBb