mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
fix(terminal): place cursor correctly after restart for persistent TUIs
SerializeAddon emits a relative cursor-move tail (\x1b[NA\x1b[MD)
computed from the last content cell it serialized to the terminal's real
cursor. That math drifts when a TUI drew rows below the cursor (border,
status lines) and then parked the cursor back on an input prompt above
— on replay the cursor lands one row below the prompt.
Append an authoritative absolute CSI row;col H after SerializeAddon
output at both capture sites (renderer captureBuffers and daemon
HeadlessEmulator.getSnapshot) using the terminal's live
buffer.active.cursorY/cursorX. The absolute positioner is the last
cursor-move seen on replay, overriding SerializeAddon's drifty relative
tail.
Also remove two cooperating bugs in restoreScrollbackBuffers and
document the contract:
1. Alt-screen trim (slice(0, lastIndexOf('\x1b[?1049h'))) discarded
not only alt-buffer content but the cursor-position tail, mode
bits, and scroll region — fueling the "cursor lands below input
box" bug for Claude Code, vim, lazygit.
2. Unconditional trailing \r\n append for PROMPT_EOL_MARK pushed the
cursor one extra row down on top of the alt-trim drift.
The reattach path in pty-connection.ts is authoritative when it has
data; restoreScrollbackBuffers is the pre-paint and must replay the
serialized buffer verbatim + POST_REPLAY_MODE_RESET only.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
Orca
@@ -0,0 +1,259 @@
|
||||
# Design: Fix persistent-terminal cursor landing below TUI after restart
|
||||
|
||||
**Branch:** `Jinwoo-H/cursor-bug`
|
||||
**Author:** Jinwoo Hong
|
||||
**Status:** Draft
|
||||
|
||||
## Problem
|
||||
|
||||
When Orca restarts with an alt-screen TUI running in a persistent terminal (most commonly Claude Code, but the same class also covers vim, lazygit, htop, btop, k9s, etc.), the terminal repaints correctly but the xterm visible cursor lands on a blank row **below** the TUI's rendered UI — outside the Claude input box, beneath the "bypass permissions on" status line. Typing still delivers to the TUI because keyboard input flows to the PTY (which is unaffected), so the bug is purely a visual desync of the rendered cursor cell.
|
||||
|
||||
Screenshot: cursor is one or two rows below where Claude's input prompt expects it; first keystroke appears in the correct place and the cursor snaps back.
|
||||
|
||||
## Architecture: two restore paths
|
||||
|
||||
There are two code paths that can write into the xterm instance after a restart. They run in sequence, and most restart cases go through both:
|
||||
|
||||
```
|
||||
┌────────────────────────────────────┐
|
||||
│ renderer: hydrate from disk │
|
||||
│ savedBuffers (SerializeAddon out) │
|
||||
└────────────────┬───────────────────┘
|
||||
│
|
||||
Path A (local pre-paint)
|
||||
│
|
||||
▼
|
||||
┌──────────────────────────────────────────────┐
|
||||
│ layout-serialization.ts │
|
||||
│ restoreScrollbackBuffers(...) │
|
||||
│ replayIntoTerminal(buf) │
|
||||
│ replayIntoTerminal(POST_REPLAY_MODE_...) │
|
||||
└──────────────────────────┬───────────────────┘
|
||||
│
|
||||
│ xterm now shows last-known frame
|
||||
│ (this is the "pre-paint")
|
||||
│
|
||||
Path B (daemon / SSH authoritative)
|
||||
│
|
||||
▼
|
||||
┌──────────────────────────────────────────────┐
|
||||
│ pty-connection.ts │
|
||||
│ handleReattachResult(connectResult) │
|
||||
│ if coldRestore → clear + write + reset │
|
||||
│ if snapshot → clear + write + reset │
|
||||
│ if replay → write │
|
||||
│ else → (no write; rely on A) │
|
||||
└──────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
Path A runs first from disk-serialized state and is meant to give the user something to look at with no latency. Path B runs when the reattach async resolves and is authoritative when it has data. The bug is that Path A was mutating cursor state in a way that survived into the one case where Path B does not overwrite it (the `else` branch above) — and, even when Path B does overwrite, there was a window where the wrong cursor was visible.
|
||||
|
||||
## Root cause
|
||||
|
||||
Two cooperating bugs in `src/renderer/src/components/terminal-pane/layout-serialization.ts#restoreScrollbackBuffers` (lines 236–284), combined with a restoration pipeline that pre-paints before the authoritative live snapshot arrives.
|
||||
|
||||
### Bug 1 — alt-screen trim amputates the cursor-positioning tail
|
||||
|
||||
`SerializeAddon.serialize()` (xterm addon source) emits, in order:
|
||||
1. Normal-buffer rows, ending with a relative-cursor-move tail positioning xterm's cursor at the normal-buffer's captured `(cursorX, cursorY)`.
|
||||
2. If alt buffer was active at capture: `\x1b[?1049h\x1b[H`, then alt-buffer rows, then a relative-cursor-move tail for the alt-buffer's captured `(cursorX, cursorY)`.
|
||||
3. Mode bits from `_serializeModes` (`?1h / ?66h / ?2004h / ?1004h / ?25l` etc.).
|
||||
4. Scroll region (DECSTBM) if non-default.
|
||||
|
||||
Per `@xterm/addon-serialize/src/SerializeAddon.ts:593-618` and `_serializeString:441-466`, the cursor-move tail is emitted INSIDE `_serializeString` PER BUFFER (so at the end of normal-buffer content AND at the end of alt-buffer content), and mode bits + scroll region are concatenated AFTER both buffer serializations in `serialize()`.
|
||||
|
||||
The alt-screen trim slices at `lastIndexOf('\x1b[?1049h')` — discarding in one cut: the alt-buffer content, the alt-buffer's cursor tail, the mode bits, and the scroll region.
|
||||
|
||||
`restoreScrollbackBuffers` has this heuristic for TUIs that were mid-run at shutdown:
|
||||
|
||||
```ts
|
||||
// layout-serialization.ts:260-264
|
||||
const lastOn = buf.lastIndexOf('\x1b[?1049h')
|
||||
const lastOff = buf.lastIndexOf('\x1b[?1049l')
|
||||
if (lastOn > lastOff) {
|
||||
buf = buf.slice(0, lastOn) // discards alt-screen section
|
||||
}
|
||||
```
|
||||
|
||||
The stated intent is "if the buffer ends in alt-screen mode, exit alt-screen so the user sees a usable terminal." But the slice happens *before* the reattach path writes anything, and it throws away:
|
||||
|
||||
- The alt-buffer contents (Claude's rendered UI)
|
||||
- The trailing mode bits
|
||||
- **The cursor-positioning escapes**
|
||||
|
||||
After the trim, `buf` now ends wherever the main-screen scrollback ended — typically the blank row below the previous prompt. That is literally the "one row below" the user sees in the screenshot.
|
||||
|
||||
### Bug 2 — unconditional `\r\n` pushes the cursor one more line down
|
||||
|
||||
```ts
|
||||
// layout-serialization.ts:272-274
|
||||
replayIntoTerminal(pane, replayingPanesRef, buf)
|
||||
replayIntoTerminal(pane, replayingPanesRef, '\r\n') // "PROMPT_EOL_MARK protection"
|
||||
```
|
||||
|
||||
The comment cites zsh's `PROMPT_EOL_MARK` (`%` indicator when a line has no trailing newline). That concern only applies when a **fresh shell is about to print a new prompt** — i.e. true cold spawn. In every reattach path in `pty-connection.ts` (`coldRestore`, `snapshot`, `replay`), the branch already issues `\x1b[2J\x1b[3J\x1b[H` *before* writing its authoritative content, which erases any PROMPT_EOL concern. So the `\r\n` protects against a case that no live code path hits, and in the bug scenario it adds a second row of cursor drift on top of Bug 1.
|
||||
|
||||
### Why the downstream snapshot write doesn't fix it
|
||||
|
||||
In `pty-connection.ts:583-592` the `snapshot` branch (daemon live-session reattach) does:
|
||||
|
||||
```ts
|
||||
replayIntoTerminal(pane, …, '\x1b[2J\x1b[3J\x1b[H') // clear
|
||||
replayIntoTerminal(pane, …, connectResult.snapshot) // authoritative state
|
||||
replayIntoTerminal(pane, …, POST_REPLAY_FOCUS_REPORTING_RESET) // \e[?25h\e[?1004l — no cursor move
|
||||
…
|
||||
window.api.pty.signal(ptyId, 'SIGWINCH') // prompt TUI to repaint
|
||||
```
|
||||
|
||||
This *would* re-place the cursor if `connectResult.snapshot` contained explicit cursor-positioning bytes or if the TUI's SIGWINCH-triggered repaint emitted an absolute `CSI H`. The daemon snapshot is captured from the headless emulator (`src/main/daemon/headless-emulator.ts`) which uses the same SerializeAddon, so it *does* emit the same relative-move tail — but by the time this path writes, xterm's cursor is already sitting at the wrong row thanks to Bugs 1+2, and SerializeAddon's `_serializeString` computes moves **relative to its own last-written cell**, not relative to xterm's current cursor. After the `\x1b[2J\x1b[3J\x1b[H` clear the cursor is at (0,0); SerializeAddon's snapshot writes content and then moves to its captured `(cursorX, cursorY)` — which should land correctly.
|
||||
|
||||
That means the snapshot path *does* correct the cursor if it runs. But there are cases where it doesn't run or is racy:
|
||||
|
||||
1. **No daemon snapshot available** — the connect returns neither `snapshot` nor `coldRestore` nor `replay` (fresh reattach, daemon scrollback not yet populated). In this case Bugs 1+2 are the only cursor writes, and they land below the UI. SIGWINCH triggers Claude's repaint, which paints content into the (now-wrong) alt screen state, but Claude's repaint does not emit an absolute cursor move — it typically restores cursor via DECRC or leaves it wherever its last render put it.
|
||||
2. **The alt-screen trim in Bug 1 discarded `\x1b[?1049h`** — so when the snapshot path later writes its alt-screen content, xterm is still in normal-screen mode because we never re-entered alt. The daemon snapshot *does* include a leading `\x1b[?1049h\x1b[H` (SerializeAddon adds it when the alt buffer is serialized), so this is self-healing. Verified by reading the addon source.
|
||||
|
||||
The dominant failure mode is (1): persistent terminal with a live process, but the daemon snapshot path doesn't run (because the daemon already gave the renderer all it had via pty attach) or the replay path writes something that ends with the cursor relative-moved to a location that intersects a still-stale xterm cursor position. Either way, the bug originates in `restoreScrollbackBuffers`; cleaning that up removes the source of the drift.
|
||||
|
||||
## Fix
|
||||
|
||||
**Make `restoreScrollbackBuffers` idempotent with the reattach path and stop clobbering cursor state.**
|
||||
|
||||
Two principles:
|
||||
|
||||
1. The serialized xterm buffer from disk is a **pre-paint** for perceived latency. It should match what the reattach path will overwrite, not fight it.
|
||||
2. Whenever a live daemon session is about to supply an authoritative snapshot, skip the local pre-paint entirely. The daemon snapshot is always at least as fresh and uses the same SerializeAddon format.
|
||||
|
||||
### Changes
|
||||
|
||||
#### A. `layout-serialization.ts#restoreScrollbackBuffers` — remove unsafe manipulations
|
||||
|
||||
- **Remove the alt-screen trim** (lines 258–264). Writing the full SerializeAddon output, including `\x1b[?1049h` and the alt-screen body, is correct: when xterm replays the snapshot the TUI's rendered frame is visible. If the TUI process is truly dead (process exited between capture and restore), the downstream coldRestore branch in `pty-connection.ts:557-582` will clear and replace anyway — the trim never added safety, only corruption.
|
||||
- **Remove the `\r\n` nudge** (lines 272–274). PROMPT_EOL_MARK only matters for cold-spawn of a fresh interactive shell; no current reattach path in `pty-connection.ts` relies on this. For cold-spawn, the new prompt is printed *after* this function returns, and zsh's `PROMPT_EOL_MARK` will correctly detect the prior buffer's trailing state.
|
||||
- **Keep** `POST_REPLAY_MODE_RESET` — it's the mode-sanitization for crashed TUIs and is still needed.
|
||||
- **Replace the existing block comment at `layout-serialization.ts:229-235`** (which describes the old alt-trim behavior being removed) with a new contract comment above `restoreScrollbackBuffers` that codifies the invariant a future maintainer needs to know. The new comment REPLACES the old one; it does not sit alongside it — leaving both would leave the file self-contradicting. The new comment must explicitly forbid BOTH stripping (Bug 1) AND appending (Bug 2) to the SerializeAddon output. Suggested wording:
|
||||
|
||||
> The serialized buffer from disk is a PRE-PAINT. It is the SerializeAddon output verbatim, including mode bits, the `\x1b[?1049h` alt-screen marker (when captured in alt mode), and the trailing relative-cursor-move tail that positions the cursor at capture time. Write it through `replayIntoTerminal` exactly as-is, followed only by `POST_REPLAY_MODE_RESET`.
|
||||
>
|
||||
> Do not strip escape sequences from this buffer (the alt-screen trim that used to live here discarded the cursor tail — Bug 1). Do not append synthetic bytes like `\r\n` (the PROMPT_EOL_MARK nudge that used to live here pushed the cursor an extra row down — Bug 2).
|
||||
>
|
||||
> The reattach path in `pty-connection.ts` is authoritative when it has data and will overwrite this pre-paint; when it doesn't, the verbatim replay is what the user sees.
|
||||
|
||||
This is the comment that would have prevented the original bug, and it's the one piece of new text that must land with this change regardless of everything else.
|
||||
|
||||
Net diff shape:
|
||||
|
||||
```ts
|
||||
export function restoreScrollbackBuffers(manager, savedBuffers, restoredPaneByLeafId, replayingPanesRef) {
|
||||
if (!savedBuffers) return
|
||||
for (const [oldLeafId, buffer] of Object.entries(savedBuffers)) {
|
||||
const newPaneId = restoredPaneByLeafId.get(oldLeafId)
|
||||
if (newPaneId == null || !buffer) continue
|
||||
const pane = manager.getPanes().find(p => p.id === newPaneId)
|
||||
if (!pane) continue
|
||||
try {
|
||||
replayIntoTerminal(pane, replayingPanesRef, buffer)
|
||||
replayIntoTerminal(pane, replayingPanesRef, POST_REPLAY_MODE_RESET)
|
||||
} catch { /* ignore per-pane */ }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
#### B. `pty-connection.ts` — redundant pre-paint suppression
|
||||
|
||||
When the reattach yields `snapshot` (daemon live session) we already do `\x1b[2J\x1b[3J\x1b[H` + authoritative write at lines 587–592. That clears the xterm screen, so the pre-paint from `restoreScrollbackBuffers` was going to be wiped regardless. No change needed here beyond what A delivers — once A stops corrupting cursor state, the snapshot path is already cursor-correct on its own.
|
||||
|
||||
For `coldRestore` (lines 557–582) we currently do clear + write + mode reset. The xterm pre-paint from A is overwritten by the clear, so behavior is preserved.
|
||||
|
||||
For **no-reattach-result** (fresh spawn, no daemon state), the pre-paint from A is now the only content visible until the shell emits a prompt. That's the right fallback: user sees their last terminal view (correctly, with cursor in place) while the new shell starts. No change needed.
|
||||
|
||||
#### C. Test: reproducing the cursor drift
|
||||
|
||||
Add to `src/renderer/src/components/terminal-pane/layout-serialization.test.ts` (create if absent):
|
||||
|
||||
- Given a buffer captured while alt-screen is active with cursor at (col=15, row=3):
|
||||
- **Before fix:** after `restoreScrollbackBuffers`, xterm's `buffer.active.cursorX/Y` differs from (15, 3) by the alt-trim + `\r\n` delta.
|
||||
- **After fix:** `cursorX === 15 && cursorY === 3` exactly.
|
||||
- Given a normal-screen buffer with cursor at end-of-line:
|
||||
- After fix, no spurious newline appended; cursor position matches capture-time cursor.
|
||||
|
||||
Use `@xterm/headless` for the test terminal (already used in `src/main/daemon/headless-emulator.ts`) so we can assert on `buffer.active.cursorY/X` without DOM. The existing `layout-serialization.test.ts` is pure-logic (mocks `HTMLElement`, never uses xterm) — these new tests must drive a real terminal end-to-end so the bug (which lives in xterm's parsed cursor state) is observable. Scaffolding, following the working pattern in `src/main/daemon/headless-emulator.ts:49-57`:
|
||||
|
||||
- Instantiate two `@xterm/headless` `Terminal` instances — one for capture, one for restore — and load `SerializeAddon` on each.
|
||||
- Build a minimal `ManagedPane`-shaped stub: `{ id: 1, terminal }` where `terminal` is the real headless `Terminal`.
|
||||
- Build a fake `PaneManager` exposing only `getPanes(): ManagedPane[]` returning `[stub]`.
|
||||
- `replayingPanesRef` is a plain `{ current: new Map<number, number>() }`.
|
||||
- Assertions are on `terminal.buffer.active.cursorX/Y` and `terminal.buffer.active.type === 'alternate'` where applicable — NOT on what bytes went through `replayIntoTerminal`. The whole point is to verify xterm's internal cursor state after parsing, because that's where the bug lives.
|
||||
|
||||
## Behavior matrix
|
||||
|
||||
The renderer's restart flow runs Path A (`restoreScrollbackBuffers`) unconditionally, then Path B (`handleReattachResult`) based on what the daemon/SSH return. There are four cases; the table below shows the observable cursor state before and after this fix.
|
||||
|
||||
| # | Case | Path B branch | Pre-fix cursor | Post-fix cursor |
|
||||
|---|------|---------------|----------------|-----------------|
|
||||
| 1 | Daemon live reattach with snapshot (happy path) | `snapshot` → clear + write + mode reset | Correct eventually, but wrong for ~1 frame of pre-paint (alt-trim + `\r\n` drift briefly visible) | Correct immediately; pre-paint matches what snapshot will re-assert |
|
||||
| 2 | Daemon cold restore (process exited while Orca was closed) | `coldRestore` → clear + write + mode reset | Wrong pre-paint, then clear+write overwrites — cursor ends correct | Correct; no behavior change for this case, pre-paint just matches sooner |
|
||||
| 3 | Reattach returns no snapshot / coldRestore / replay (fresh attach, daemon scrollback empty) | `else` (no write) | **Bug lands here.** Path A's alt-trim + `\r\n` is the only cursor write; xterm cursor sits one or two rows below TUI rendering | Correct; Path A now faithfully replays SerializeAddon's relative-move tail |
|
||||
| 4 | Reattach errors out (daemon unavailable, connect fails) | Error path, no authoritative write | Bug also lands here, same as case 3 | Correct pre-paint survives until user intervention |
|
||||
|
||||
Cases 3 and 4 are where the principal failure mode lived; case 1 was the cosmetic flash the user perceived as "cursor appears wrong then moves."
|
||||
|
||||
## Risk analysis
|
||||
|
||||
| Risk | Likelihood | Mitigation | Contingent mitigation |
|
||||
|------|-----------|------------|-----------------------|
|
||||
| Removing alt-screen trim leaves restored terminal stuck in alt mode when TUI has died | Low — the serialized buffer ends with SerializeAddon's mode bits, which may include `?1049h`. If the TUI died mid-render, on cold restart the reattach branch clears with `\x1b[2J\x1b[3J\x1b[H` which exits alt mode. | If we see stuck-in-alt symptoms in QA, add `\x1b[?1049l` to `POST_REPLAY_MODE_RESET` for cold-restore only (via a second constant, as is already done for `POST_REPLAY_FOCUS_REPORTING_RESET` vs `POST_REPLAY_MODE_RESET`). | — |
|
||||
| Removing `\r\n` breaks zsh PROMPT_EOL_MARK on fresh cold spawn | Very low — zsh runs after `restoreScrollbackBuffers` returns and is responsible for emitting its own leading newline when needed. The `\r\n` was a speculative cross-shell safety net that isn't load-bearing. | Manually verify cold-restart with zsh and non-newline-terminated scrollback. The new PROMPT_EOL cold-spawn unit test (see Verification → Automated) is the gate: if it fails in practice, we flip to the contingent mitigation. | Restore the `\r\n` **conditionally**: only when the buffer's final cell is not `\n` **and** alt-screen was not active at capture. Implementation: inspect last non-escape byte of `buf` (trim trailing ESC-sequences) and check for the presence of an unpaired `\x1b[?1049h`. Emit `\r\n` only if both conditions hold. This preserves the PROMPT_EOL protection for bare-shell cold-spawn without corrupting alt-screen cursor state. |
|
||||
| Bash (and other non-zsh shells) cold-spawn with non-newline-terminated prior output will now render the new prompt adjacent to the last character instead of on a fresh line | Low — only visible in the narrow case where the user's last output before shutdown did not end in `\n` **and** the shell is bash/sh/fish (not zsh). zsh's `PROMPT_EOL_MARK` handles this correctly by printing a `%` on its own line; bash has no equivalent and will simply print `user@host:~$ ` immediately after the trailing character. | Documented behavior, not a regression in terminal semantics. The prior `\r\n` was masking this by always inserting a line break; removing it exposes the actual shell behavior, which matches what the user would see if they had quit the shell cleanly and restarted it in a fresh terminal. | If we get user reports, we can gate the `\r\n` on detecting a non-zsh shell (via `process.env.SHELL` at spawn), but this is unlikely to be worth the complexity. |
|
||||
| SerializeAddon's relative cursor moves overshoot when the replay lands in a terminal with different rows than capture | Medium — FitAddon runs after restore via `queueResizeAll(isActive)` in `use-terminal-pane-lifecycle.ts:789`. | This is orthogonal to the fix: the relative moves are computed during serialization when rows were known, and xterm clamps CUP to viewport. We rely on the same behavior the daemon snapshot path already depends on. | — |
|
||||
| Alt-screen content previously hidden (via the trim) will now be visible on restore for one frame before reattach clears | Low — it *should* be visible; that's what the user was looking at. The "flash of prior TUI before Claude repaints" is a feature, not a bug. | None. | — |
|
||||
|
||||
## Verification plan
|
||||
|
||||
### Automated
|
||||
|
||||
- Unit test per C above (cursor-drift reproduction with alt-screen buffer).
|
||||
- **PROMPT_EOL cold-spawn guard test.** In the same `layout-serialization.test.ts`:
|
||||
- Capture a buffer from a `@xterm/headless` terminal whose final written cell is **not** `\n` (e.g. write `"echo hi"` with no trailing newline, then `serialize()`).
|
||||
- Call `restoreScrollbackBuffers` into a fresh `@xterm/headless` terminal.
|
||||
- Assert that the restored terminal's last non-empty row ends with `"echo hi"` and **no spurious newline has been appended** — i.e. `buffer.active.cursorY` equals the row containing `"echo hi"`, not the row below it. Equivalently: assert the terminal did not emit a row-break between the serialized content and the cursor.
|
||||
- Belt-and-suspenders: also assert the row at `cursorY + 1` is empty (e.g. `terminal.buffer.active.getLine(cursorY + 1)?.translateToString().trim() === ''`). Closes an edge case where a future conditional `\r\n` could otherwise slip through the primary assertion.
|
||||
- This is the concrete gate for the "removing `\r\n` is safe" claim. If it fails in any shell/terminal-size permutation we care about, we fall back to the contingent mitigation in the Risk table (conditional `\r\n` based on final-cell + alt-screen inspection).
|
||||
- Existing `src/renderer/src/store/slices/terminals-hydration.test.ts` coverage for hydration remains green.
|
||||
- Existing `src/renderer/src/components/terminal-pane/pty-connection.test.ts` assertions on `POST_REPLAY_MODE_RESET` and `POST_REPLAY_FOCUS_REPORTING_RESET` sequencing remain green (we're not touching pty-connection).
|
||||
|
||||
### Manual
|
||||
|
||||
1. Open Orca, start Claude Code in a persistent terminal, let it sit at the input box.
|
||||
2. Quit Orca fully.
|
||||
3. Reopen Orca and switch to that tab **without typing first**.
|
||||
4. Observe: cursor should blink **inside** the Claude input box (to the right of the `>` or leading whitespace), not one or two rows below. The "bypass permissions on (shift+tab to cycle)" status line should be directly above the cursor, and no phantom blank row between them.
|
||||
5. Repeat with vim mid-edit, lazygit with a dialog open, and htop. In all cases the cursor should match where it was at capture.
|
||||
6. Cold-spawn case: open a new persistent terminal, type `echo hi` (no trailing newline, leave cursor mid-line). Quit, restart. The terminal's scrollback should show `echo hi` with the cursor after the `i`, and the new prompt should appear on the following line without a `%` indicator from zsh PROMPT_EOL_MARK.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Daemon snapshot capture cadence (`src/main/daemon/headless-emulator.ts`)
|
||||
- SSH replay buffer semantics (separate path, unaffected)
|
||||
- `queueResizeAll` / FitAddon interaction (separate known-good path; cursor is preserved across fit because xterm's CUP state is maintained by buffer coordinates, not screen coordinates)
|
||||
|
||||
## Follow-ups (separate PRs)
|
||||
|
||||
These came up during Phase 0.5 review. They are deliberately excluded from this change because they are scope-creep on a bug fix, not because they are low-value.
|
||||
|
||||
- **Alternative B: skip Path A pre-paint entirely when a daemon reattach is expected.** The cleanest long-term shape is to let the daemon snapshot be the only writer when it's known to be coming, so we never pay for a pre-paint that's about to be clobbered by a clear+write. Not included here because it's a perceived-latency change (we're trading a frame of stale-but-close UI for a frame of blank terminal) and needs measurement — ideally a before/after on time-to-first-correct-paint under cold daemon attach. Tracked separately.
|
||||
|
||||
- **Observability: log the "Path B made no authoritative write" case.** Currently `handleReattachResult` in `pty-connection.ts:593-606` has an `else` branch where none of `coldRestore` / `snapshot` / `replay` was provided and the code silently falls through, relying on whatever Path A left behind. That's exactly the case where the original bug was load-bearing and invisible. Add a single diagnostic log line at that branch:
|
||||
|
||||
```ts
|
||||
// pty-connection.ts, in handleReattachResult's fall-through else
|
||||
log.debug('[pty-connection] reattach produced no coldRestore/snapshot/replay; relying on pre-paint from restoreScrollbackBuffers', { ptyId })
|
||||
```
|
||||
|
||||
Cheap to add, makes the "case 3" row of the behavior matrix visible in logs next time it misbehaves. Author's call whether to land in this PR or defer — leaning defer to keep the diff surgical, but calling it out here so it doesn't get lost.
|
||||
|
||||
## References
|
||||
|
||||
- `src/renderer/src/components/terminal-pane/layout-serialization.ts:236-284` — `restoreScrollbackBuffers`
|
||||
- `src/renderer/src/components/terminal-pane/pty-connection.ts:557-618` — reattach branches + SIGWINCH
|
||||
- `src/renderer/src/components/terminal-pane/TerminalPane.tsx:740-808` — shutdown capture (unchanged)
|
||||
- `@xterm/addon-serialize@0.15.0-beta.198` — `SerializeAddon._serializeString` for cursor-tail semantics
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { Terminal } from '@xterm/headless'
|
||||
import { HeadlessEmulator } from './headless-emulator'
|
||||
|
||||
describe('HeadlessEmulator', () => {
|
||||
@@ -180,10 +181,182 @@ describe('HeadlessEmulator', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('absolute cursor tail', () => {
|
||||
// Why: SerializeAddon emits a relative cursor-move tail that drifts when
|
||||
// the TUI drew rows out of visual order. getSnapshot() appends an
|
||||
// absolute `CSI row;col H` after the SerializeAddon output so the
|
||||
// restored cursor lands exactly where it was at capture time regardless
|
||||
// of what relative math the serializer used.
|
||||
it('appends absolute cursor position after the serialized buffer', async () => {
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('hello\r\nworld')
|
||||
// Move cursor to row 10, col 5 (1-indexed CSI)
|
||||
await emulator.write('\x1b[10;5H')
|
||||
|
||||
const snapshot = emulator.getSnapshot()
|
||||
// Last 6 chars must be the absolute-cursor CSI
|
||||
expect(snapshot.snapshotAnsi.endsWith('\x1b[10;5H')).toBe(true)
|
||||
})
|
||||
|
||||
it('restores cursor after a normal-buffer TUI that drew rows below the cursor', async () => {
|
||||
// Reproduces the shape of the real-world Claude Code bug: input prompt
|
||||
// drawn above, border + status drawn below, cursor parked back on the
|
||||
// input prompt. SerializeAddon's relative tail would land one row low;
|
||||
// the absolute tail corrects it.
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('\x1b[H')
|
||||
await emulator.write('\r\n\r\n\r\n\r\n\r\n\r\n') // rows 0-5 blank
|
||||
await emulator.write('> ') // row 6 cursor col 2
|
||||
await emulator.write('\x1b[8;1H──────') // row 7
|
||||
await emulator.write('\x1b[9;1Hstatus') // row 8
|
||||
await emulator.write('\x1b[10;1Hhint') // row 9
|
||||
await emulator.write('\x1b[7;3H') // park cursor on prompt
|
||||
|
||||
const srcSnapshot = emulator.getSnapshot()
|
||||
|
||||
const replayTerm = new Terminal({ cols: 80, rows: 24, allowProposedApi: true })
|
||||
await new Promise<void>((resolve) =>
|
||||
replayTerm.write(srcSnapshot.rehydrateSequences + srcSnapshot.snapshotAnsi, () => resolve())
|
||||
)
|
||||
|
||||
expect(replayTerm.buffer.active.cursorX).toBe(2)
|
||||
expect(replayTerm.buffer.active.cursorY).toBe(6)
|
||||
replayTerm.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
describe('dispose', () => {
|
||||
it('can be disposed without error', () => {
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
expect(() => emulator.dispose()).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Snapshot restore round-trip — alt-screen cursor alignment
|
||||
//
|
||||
// Regression guard for the "cursor lands one row below the TUI input box
|
||||
// after restart" bug. The reattach path in pty-connection.ts writes
|
||||
// `rehydrateSequences + snapshotAnsi` into a fresh xterm. For alt-screen
|
||||
// sessions, `rehydrateSequences` must not pre-enter the alt buffer with
|
||||
// `\x1b[?1049h` because SerializeAddon's own output already emits
|
||||
// `\x1b[?1049h\x1b[H` between the normal and alt buffers. Pre-entering
|
||||
// causes normal-buffer content to be drawn into the alt buffer, pushing
|
||||
// the alt cursor down by the height of the normal buffer's trailing rows.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('alt-screen snapshot restore round-trip', () => {
|
||||
async function replaySnapshotIntoFreshTerminal(rehydrate: string, snapshotAnsi: string) {
|
||||
const replayTerm = new Terminal({ cols: 80, rows: 24, allowProposedApi: true })
|
||||
await new Promise<void>((resolve) =>
|
||||
replayTerm.write(rehydrate + snapshotAnsi, () => resolve())
|
||||
)
|
||||
return replayTerm
|
||||
}
|
||||
|
||||
it('places the cursor at the same (row, col) as the source alt-screen terminal', async () => {
|
||||
// Simulate a shell session that ran `$ claude`, entered alt-screen, drew
|
||||
// the Claude UI header + an input box, and left the cursor inside the
|
||||
// input box at column 2, row 5.
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('$ claude\r\n')
|
||||
await emulator.write('\x1b[?1049h\x1b[H') // enter alt screen, home
|
||||
await emulator.write('Claude Code v2.1.126\r\n')
|
||||
await emulator.write('Opus 4.7 · API Usage Billing\r\n')
|
||||
await emulator.write('~/orca/workspaces/orca/Chimaera\r\n')
|
||||
await emulator.write('\r\n') // blank
|
||||
await emulator.write('> ') // input prompt
|
||||
// Cursor is now at row 4, col 2 in the alt buffer.
|
||||
|
||||
const srcSnapshot = emulator.getSnapshot()
|
||||
expect(srcSnapshot.modes.alternateScreen).toBe(true)
|
||||
|
||||
// Match the reattach path in daemon-pty-adapter.ts:199 exactly:
|
||||
// `snapshotPayload = rehydrateSequences + snapshotAnsi`.
|
||||
const replayTerm = await replaySnapshotIntoFreshTerminal(
|
||||
srcSnapshot.rehydrateSequences,
|
||||
srcSnapshot.snapshotAnsi
|
||||
)
|
||||
|
||||
expect(replayTerm.buffer.active.type).toBe('alternate')
|
||||
expect(replayTerm.buffer.active.cursorY).toBe(emulator.getSnapshot().rows - 24 + 4)
|
||||
// The crucial assertion: cursor col/row match the source.
|
||||
expect({
|
||||
x: replayTerm.buffer.active.cursorX,
|
||||
y: replayTerm.buffer.active.cursorY
|
||||
}).toEqual({ x: 2, y: 4 })
|
||||
replayTerm.dispose()
|
||||
})
|
||||
|
||||
it('places cursor correctly when the normal buffer has scrollback beyond terminal rows', async () => {
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
// Fill normal buffer with enough scrollback that it scrolls past one screen.
|
||||
for (let i = 0; i < 40; i++) {
|
||||
await emulator.write(`line ${i}\r\n`)
|
||||
}
|
||||
await emulator.write('$ claude\r\n')
|
||||
await emulator.write('\x1b[?1049h\x1b[H')
|
||||
await emulator.write('Claude Code v2.1.126\r\n')
|
||||
await emulator.write('Opus 4.7 · API Usage Billing\r\n')
|
||||
await emulator.write('~/orca/workspaces/orca/Chimaera\r\n')
|
||||
await emulator.write('\r\n')
|
||||
await emulator.write('> ')
|
||||
const srcSnapshot = emulator.getSnapshot()
|
||||
expect(srcSnapshot.modes.alternateScreen).toBe(true)
|
||||
|
||||
const replayTerm = new Terminal({ cols: 80, rows: 24, allowProposedApi: true })
|
||||
await new Promise<void>((resolve) =>
|
||||
replayTerm.write(srcSnapshot.rehydrateSequences + srcSnapshot.snapshotAnsi, () => resolve())
|
||||
)
|
||||
|
||||
expect(replayTerm.buffer.active.type).toBe('alternate')
|
||||
expect({
|
||||
x: replayTerm.buffer.active.cursorX,
|
||||
y: replayTerm.buffer.active.cursorY
|
||||
}).toEqual({ x: 2, y: 4 })
|
||||
replayTerm.dispose()
|
||||
})
|
||||
|
||||
it('reproduces the renderer write sequence: local scrollback replay, then daemon snapshot reattach', async () => {
|
||||
// Source: same alt-screen Claude session as above.
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('$ claude\r\n')
|
||||
await emulator.write('\x1b[?1049h\x1b[H')
|
||||
await emulator.write('Claude Code v2.1.126\r\n')
|
||||
await emulator.write('Opus 4.7 · API Usage Billing\r\n')
|
||||
await emulator.write('~/orca/workspaces/orca/Chimaera\r\n')
|
||||
await emulator.write('\r\n')
|
||||
await emulator.write('> ')
|
||||
const srcSnapshot = emulator.getSnapshot()
|
||||
|
||||
// Step 1: restoreScrollbackBuffers replays the LOCAL saved xterm buffer
|
||||
// (same data as the daemon's serialize, since both sides come from the
|
||||
// same bytes), followed by POST_REPLAY_MODE_RESET.
|
||||
// Local serialized buffer = `snapshotAnsi` (what the renderer's
|
||||
// SerializeAddon would have produced when Orca saved layout state).
|
||||
const POST_REPLAY_MODE_RESET =
|
||||
'\x1b[?25h\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1004l\x1b[?1006l\x1b[?2004l'
|
||||
// Step 2: reattach path — pty-connection.ts:587 — clear then write
|
||||
// `rehydrateSequences + snapshotAnsi`, then POST_REPLAY_FOCUS_REPORTING_RESET.
|
||||
const POST_REPLAY_FOCUS_REPORTING_RESET = '\x1b[?25h\x1b[?1004l'
|
||||
const snapshotPayload = srcSnapshot.rehydrateSequences + srcSnapshot.snapshotAnsi
|
||||
|
||||
const replayTerm = new Terminal({ cols: 80, rows: 24, allowProposedApi: true })
|
||||
await new Promise<void>((resolve) =>
|
||||
replayTerm.write(srcSnapshot.snapshotAnsi + POST_REPLAY_MODE_RESET, () => resolve())
|
||||
)
|
||||
await new Promise<void>((resolve) =>
|
||||
replayTerm.write(
|
||||
`\x1b[2J\x1b[3J\x1b[H${snapshotPayload}${POST_REPLAY_FOCUS_REPORTING_RESET}`,
|
||||
() => resolve()
|
||||
)
|
||||
)
|
||||
|
||||
expect(replayTerm.buffer.active.type).toBe('alternate')
|
||||
expect({
|
||||
x: replayTerm.buffer.active.cursorX,
|
||||
y: replayTerm.buffer.active.cursorY
|
||||
}).toEqual({ x: 2, y: 4 })
|
||||
replayTerm.dispose()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -90,7 +90,7 @@ export class HeadlessEmulator {
|
||||
getSnapshot(): TerminalSnapshot {
|
||||
const modes = this.getModes()
|
||||
return {
|
||||
snapshotAnsi: this.serializer.serialize(),
|
||||
snapshotAnsi: this.serializer.serialize() + this.buildAbsoluteCursorTail(),
|
||||
scrollbackAnsi: '',
|
||||
rehydrateSequences: this.buildRehydrateSequences(modes),
|
||||
cwd: this.cwd,
|
||||
@@ -147,6 +147,24 @@ export class HeadlessEmulator {
|
||||
}
|
||||
}
|
||||
|
||||
// Why: SerializeAddon appends a *relative* cursor-move tail (e.g. `\x1b[4A\x1b[104D`)
|
||||
// computed from the last content cell it emitted to `_buffer.baseY + _buffer.cursorY`.
|
||||
// That math lands the cursor on the correct cell in simple cases but drifts
|
||||
// when rows are emitted out of visual order (e.g. a TUI drew an input
|
||||
// prompt, then drew a border below it, then moved the cursor back up to
|
||||
// the prompt — the tail then targets a row below the prompt). A trailing
|
||||
// absolute `CSI row;col H` after the serialized buffer acts as an
|
||||
// authoritative override: whatever SerializeAddon's relative math lands
|
||||
// on, the absolute positioner places the cursor exactly where the source
|
||||
// terminal had it at capture time. 1-indexed per DECSET/CUP; the buffer
|
||||
// API is 0-indexed.
|
||||
private buildAbsoluteCursorTail(): string {
|
||||
const buffer = this.terminal.buffer.active
|
||||
const row = buffer.cursorY + 1
|
||||
const col = buffer.cursorX + 1
|
||||
return `\x1b[${row};${col}H`
|
||||
}
|
||||
|
||||
private buildRehydrateSequences(modes: TerminalModes): string {
|
||||
const seqs: string[] = []
|
||||
if (modes.bracketedPaste) {
|
||||
|
||||
@@ -771,7 +771,21 @@ export default function TerminalPane({
|
||||
serialized = best
|
||||
}
|
||||
if (serialized.length > 0) {
|
||||
buffers[leafId] = serialized
|
||||
// Why absolute cursor tail: SerializeAddon emits a relative
|
||||
// cursor-move tail (`\x1b[NA\x1b[MD`) computed from the last
|
||||
// content cell it serialized to the terminal's real cursor.
|
||||
// That math drifts when a TUI drew rows out of visual order
|
||||
// (input prompt, then border below, then cursor moved back up
|
||||
// to prompt) — replay lands the cursor on the border row
|
||||
// instead of the prompt. An absolute `CSI row;col H`
|
||||
// appended after the SerializeAddon tail overrides whatever
|
||||
// relative move it produced and places the cursor exactly
|
||||
// where xterm had it at capture time. 1-indexed per CUP;
|
||||
// buffer API is 0-indexed.
|
||||
const activeBuffer = pane.terminal.buffer.active
|
||||
const cursorRow = activeBuffer.cursorY + 1
|
||||
const cursorCol = activeBuffer.cursorX + 1
|
||||
buffers[leafId] = `${serialized}\x1b[${cursorRow};${cursorCol}H`
|
||||
}
|
||||
} catch {
|
||||
// Serialization failure for one pane should not block others.
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
/* oxlint-disable max-lines -- Why: cohesive test suite covering paneLeafId,
|
||||
buildFontFamily, serializePaneTree, serializeTerminalLayout,
|
||||
collectLeafIdsInReplayCreationOrder, and restoreScrollbackBuffers for the
|
||||
same layout-serialization module. Splitting by function would scatter
|
||||
related fixtures and the xterm round-trip helpers. */
|
||||
import { describe, expect, it, beforeAll } from 'vitest'
|
||||
import { Terminal } from '@xterm/headless'
|
||||
import { SerializeAddon } from '@xterm/addon-serialize'
|
||||
import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager'
|
||||
import type { TerminalPaneLayoutNode } from '../../../../shared/types'
|
||||
import { type ReplayingPanesRef } from './replay-guard'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Provide a minimal HTMLElement so `instanceof HTMLElement` passes in Node env
|
||||
@@ -39,7 +48,8 @@ import {
|
||||
serializeTerminalLayout,
|
||||
EMPTY_LAYOUT,
|
||||
collectLeafIdsInOrder,
|
||||
collectLeafIdsInReplayCreationOrder
|
||||
collectLeafIdsInReplayCreationOrder,
|
||||
restoreScrollbackBuffers
|
||||
} from './layout-serialization'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -300,3 +310,179 @@ describe('collectLeafIdsInReplayCreationOrder', () => {
|
||||
expect(collectLeafIdsInReplayCreationOrder(layout)).toEqual(['A', 'B', 'C'])
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// restoreScrollbackBuffers — cursor position preservation
|
||||
//
|
||||
// These tests lock in the cursor-placement contract documented on the
|
||||
// function: the serialized buffer is replayed verbatim, followed only by
|
||||
// POST_REPLAY_MODE_RESET. Two prior heuristics caused the "cursor drifts
|
||||
// below the TUI input box" bug after app restart:
|
||||
// 1. Trimming from the last `\x1b[?1049h` to "exit alt-screen" also
|
||||
// discarded SerializeAddon's trailing cursor-position tail.
|
||||
// 2. Appending `\r\n` for "PROMPT_EOL_MARK protection" pushed the cursor
|
||||
// one extra row down.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
type CapturedTerminal = {
|
||||
terminal: Terminal
|
||||
serializer: SerializeAddon
|
||||
write: (data: string) => Promise<void>
|
||||
}
|
||||
|
||||
function createCapturedTerminal(): CapturedTerminal {
|
||||
const terminal = new Terminal({ cols: 80, rows: 24, allowProposedApi: true })
|
||||
const serializer = new SerializeAddon()
|
||||
terminal.loadAddon(serializer)
|
||||
const write = (data: string): Promise<void> =>
|
||||
new Promise<void>((resolve) => terminal.write(data, () => resolve()))
|
||||
return { terminal, serializer, write }
|
||||
}
|
||||
|
||||
function makeFakePaneManager(panes: ManagedPane[]): PaneManager {
|
||||
// Why the `as unknown as`: restoreScrollbackBuffers only calls
|
||||
// `manager.getPanes()`; building a real PaneManager would require a DOM.
|
||||
return { getPanes: () => panes } as unknown as PaneManager
|
||||
}
|
||||
|
||||
function waitForWriteDrain(terminal: Terminal): Promise<void> {
|
||||
return new Promise<void>((resolve) => terminal.write('', () => resolve()))
|
||||
}
|
||||
|
||||
describe('restoreScrollbackBuffers', () => {
|
||||
it('places cursor where SerializeAddon captured it for alt-screen TUIs (prior bug: cursor landed one row below)', async () => {
|
||||
// Capture: simulate a Claude Code-like TUI that switched to the alt
|
||||
// buffer, drew content, and left its cursor at column 15, row 3 (the
|
||||
// "input box" row, not the last row of output).
|
||||
const capture = createCapturedTerminal()
|
||||
await capture.write('\x1b[?1049h') // enter alt screen
|
||||
await capture.write('\x1b[H') // home
|
||||
await capture.write('╭─ Claude\r\n')
|
||||
await capture.write('│ previous output line 1\r\n')
|
||||
await capture.write('│ previous output line 2\r\n')
|
||||
await capture.write('╰─\r\n')
|
||||
// Move the cursor to row 3 (0-indexed), col 15 — this represents the
|
||||
// TUI's input position, which is ABOVE the last written row.
|
||||
await capture.write('\x1b[4;16H') // CSI is 1-indexed: row 4 col 16 => (15,3)
|
||||
const serialized = capture.serializer.serialize()
|
||||
expect(serialized).toContain('\x1b[?1049h')
|
||||
|
||||
// Restore into a fresh terminal via restoreScrollbackBuffers.
|
||||
const restore = createCapturedTerminal()
|
||||
const pane = { id: 1, terminal: restore.terminal } as unknown as ManagedPane
|
||||
const manager = makeFakePaneManager([pane])
|
||||
const replayingPanesRef: ReplayingPanesRef = { current: new Map() }
|
||||
|
||||
restoreScrollbackBuffers(
|
||||
manager,
|
||||
{ 'pane:1': serialized },
|
||||
new Map([['pane:1', 1]]),
|
||||
replayingPanesRef
|
||||
)
|
||||
await waitForWriteDrain(restore.terminal)
|
||||
|
||||
// The alt buffer must be active and the cursor must match capture time.
|
||||
expect(restore.terminal.buffer.active.type).toBe('alternate')
|
||||
expect(restore.terminal.buffer.active.cursorX).toBe(15)
|
||||
expect(restore.terminal.buffer.active.cursorY).toBe(3)
|
||||
})
|
||||
|
||||
it('places cursor correctly for normal-buffer TUIs that leave cursor above last-drawn row (prior bug: SerializeAddon relative cursor tail drifted the cursor one row below the input prompt)', async () => {
|
||||
// This is the REAL-WORLD scenario that escaped the alt-screen test above.
|
||||
// A TUI (Claude Code, in the reproducer) draws its UI directly on the
|
||||
// normal buffer with scrollback history present. SerializeAddon emits a
|
||||
// trailing relative-cursor-move tail like `\x1b[NA\x1b[MD` that counts
|
||||
// rows up from the last content cell it emitted to the terminal's real
|
||||
// cursor. When the last-drawn row is BELOW the cursor's logical
|
||||
// position (the bottom border and status lines live below the input
|
||||
// prompt), the relative math lands the cursor on the border row after
|
||||
// restore.
|
||||
//
|
||||
// The fix: `captureBuffers` appends an authoritative absolute
|
||||
// `CSI row;col H` after SerializeAddon's output using the terminal's
|
||||
// live cursor position at save time. We simulate that here by appending
|
||||
// the same absolute positioner before calling restoreScrollbackBuffers.
|
||||
// The absolute positioner is the last cursor-move seen by the restore
|
||||
// emulator, so it overrides whatever relative math SerializeAddon used.
|
||||
const capture = createCapturedTerminal()
|
||||
// Produce enough scrollback that the serializer takes the non-fixup
|
||||
// branch (`_buffer.length - _firstRow > terminal.rows`), which is where
|
||||
// the relative cursor tail drift manifests in the real bug.
|
||||
for (let i = 0; i < 40; i++) {
|
||||
await capture.write(`scrollback line ${i}\r\n`)
|
||||
}
|
||||
// Now the cursor is at the bottom of the visible region. Draw the TUI:
|
||||
// input prompt, then border/status lines BELOW it, then park cursor
|
||||
// back on the prompt without redrawing the rows between.
|
||||
const promptRow = capture.terminal.buffer.active.cursorY // 0-indexed
|
||||
await capture.write('> ')
|
||||
// Move down and draw filler rows, which become the "last drawn cells"
|
||||
// from SerializeAddon's perspective.
|
||||
await capture.write(`\x1b[${promptRow + 3};1H──────────────────────`)
|
||||
await capture.write(`\x1b[${promptRow + 4};1Hstatus line`)
|
||||
// Park cursor back on the input prompt (1-indexed CUP).
|
||||
await capture.write(`\x1b[${promptRow + 1};3H`)
|
||||
|
||||
// Capture-time cursor is on the input prompt.
|
||||
expect(capture.terminal.buffer.active.cursorX).toBe(2)
|
||||
expect(capture.terminal.buffer.active.cursorY).toBe(promptRow)
|
||||
|
||||
// Simulate what `captureBuffers` in TerminalPane.tsx now does: take
|
||||
// SerializeAddon's output and append an authoritative absolute CUP
|
||||
// using the captured terminal's live cursor position.
|
||||
const rawSerialized = capture.serializer.serialize()
|
||||
const cursorRow = capture.terminal.buffer.active.cursorY + 1
|
||||
const cursorCol = capture.terminal.buffer.active.cursorX + 1
|
||||
const serialized = `${rawSerialized}\x1b[${cursorRow};${cursorCol}H`
|
||||
|
||||
const restore = createCapturedTerminal()
|
||||
const pane = { id: 1, terminal: restore.terminal } as unknown as ManagedPane
|
||||
const manager = makeFakePaneManager([pane])
|
||||
const replayingPanesRef: ReplayingPanesRef = { current: new Map() }
|
||||
|
||||
restoreScrollbackBuffers(
|
||||
manager,
|
||||
{ 'pane:1': serialized },
|
||||
new Map([['pane:1', 1]]),
|
||||
replayingPanesRef
|
||||
)
|
||||
await waitForWriteDrain(restore.terminal)
|
||||
|
||||
// Cursor must land on the input prompt row, not the border below.
|
||||
expect(restore.terminal.buffer.active.cursorX).toBe(2)
|
||||
expect(restore.terminal.buffer.active.cursorY).toBe(promptRow)
|
||||
})
|
||||
|
||||
it('does not append a spurious newline after non-newline-terminated content (prior bug: \\r\\n pushed cursor down one extra row)', async () => {
|
||||
// Capture: output that ends mid-row, without a trailing newline — the
|
||||
// scenario PROMPT_EOL_MARK would normally handle at the shell layer.
|
||||
const capture = createCapturedTerminal()
|
||||
await capture.write('line one\r\n')
|
||||
await capture.write('partial line without newline') // no \r\n at end
|
||||
const serialized = capture.serializer.serialize()
|
||||
|
||||
// Cursor after capture sits on row 1 (0-indexed), column 28.
|
||||
expect(capture.terminal.buffer.active.cursorY).toBe(1)
|
||||
|
||||
const restore = createCapturedTerminal()
|
||||
const pane = { id: 1, terminal: restore.terminal } as unknown as ManagedPane
|
||||
const manager = makeFakePaneManager([pane])
|
||||
const replayingPanesRef: ReplayingPanesRef = { current: new Map() }
|
||||
|
||||
restoreScrollbackBuffers(
|
||||
manager,
|
||||
{ 'pane:1': serialized },
|
||||
new Map([['pane:1', 1]]),
|
||||
replayingPanesRef
|
||||
)
|
||||
await waitForWriteDrain(restore.terminal)
|
||||
|
||||
// Cursor stays on the partial-content row, not one row below.
|
||||
expect(restore.terminal.buffer.active.cursorY).toBe(1)
|
||||
// And the row below must be empty — a stray \r\n append would drop the
|
||||
// cursor onto a next row that, by transitivity, we'd also see content on
|
||||
// if the row was somehow shifted.
|
||||
const rowBelow = restore.terminal.buffer.active.getLine(2)?.translateToString(true) ?? ''
|
||||
expect(rowBelow.trim()).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -228,10 +228,37 @@ function collectLeafIds(
|
||||
|
||||
/**
|
||||
* Write saved scrollback buffers into the restored panes so the user sees
|
||||
* their previous terminal output after an app restart. If a buffer was
|
||||
* captured while the alternate screen was active (e.g. an agent TUI was
|
||||
* running at shutdown), we exit alt-screen first so the user sees a usable
|
||||
* normal-mode terminal.
|
||||
* their previous terminal output after an app restart.
|
||||
*
|
||||
* CONTRACT — do not break this:
|
||||
*
|
||||
* The serialized buffer from disk is a PRE-PAINT. It is the `SerializeAddon`
|
||||
* output verbatim, including mode bits, the `\x1b[?1049h` alt-screen marker
|
||||
* (when captured in alt mode), and the trailing relative-cursor-move tail
|
||||
* that positions xterm's cursor at capture time. Write it through
|
||||
* `replayIntoTerminal` exactly as-is, followed only by
|
||||
* `POST_REPLAY_MODE_RESET`.
|
||||
*
|
||||
* Do not strip escape sequences from this buffer. A previous version sliced
|
||||
* off everything from the last `\x1b[?1049h` onward to "exit alt-screen" —
|
||||
* that also discarded the cursor-position tail and caused the "cursor lands
|
||||
* one row below the TUI input box after restart" bug for alt-screen TUIs
|
||||
* like Claude Code, vim, lazygit.
|
||||
*
|
||||
* Do not append synthetic bytes like `\r\n` for "PROMPT_EOL_MARK protection".
|
||||
* A previous version did this unconditionally and pushed the cursor an extra
|
||||
* row down on top of the alt-trim drift. zsh's PROMPT_EOL_MARK is a
|
||||
* shell-level concern that fires when zsh prints its prompt; this function
|
||||
* runs before any prompt is printed and can only harm cursor state by
|
||||
* injecting bytes.
|
||||
*
|
||||
* The reattach path in `pty-connection.ts` (`handleReattachResult`) is
|
||||
* authoritative when it has data — `coldRestore`, `snapshot`, and `replay`
|
||||
* branches each do `\x1b[2J\x1b[3J\x1b[H` + write and will overwrite this
|
||||
* pre-paint. When none of those branches fire (fresh attach with no daemon
|
||||
* scrollback), the verbatim replay of the serialized buffer is what the
|
||||
* user sees, and its trailing cursor tail is what places the cursor
|
||||
* correctly.
|
||||
*/
|
||||
export function restoreScrollbackBuffers(
|
||||
manager: PaneManager,
|
||||
@@ -242,8 +269,6 @@ export function restoreScrollbackBuffers(
|
||||
if (!savedBuffers) {
|
||||
return
|
||||
}
|
||||
const ALT_SCREEN_ON = '\x1b[?1049h'
|
||||
const ALT_SCREEN_OFF = '\x1b[?1049l'
|
||||
for (const [oldLeafId, buffer] of Object.entries(savedBuffers)) {
|
||||
const newPaneId = restoredPaneByLeafId.get(oldLeafId)
|
||||
if (newPaneId == null || !buffer) {
|
||||
@@ -254,29 +279,17 @@ export function restoreScrollbackBuffers(
|
||||
continue
|
||||
}
|
||||
try {
|
||||
let buf = buffer
|
||||
// If buffer ends in alt-screen mode (agent TUI was running at
|
||||
// shutdown), exit alt-screen so the user sees a usable terminal.
|
||||
const lastOn = buf.lastIndexOf(ALT_SCREEN_ON)
|
||||
const lastOff = buf.lastIndexOf(ALT_SCREEN_OFF)
|
||||
if (lastOn > lastOff) {
|
||||
buf = buf.slice(0, lastOn)
|
||||
}
|
||||
if (buf.length > 0) {
|
||||
// Why replayIntoTerminal: the serialized buffer can contain query
|
||||
// sequences that leaked in via the pendingWritesRef flush before
|
||||
// serialization (see TerminalPane capture hook). Writing those
|
||||
// through xterm would trigger auto-replies that land in the new
|
||||
// shell's stdin. See replay-guard.ts.
|
||||
replayIntoTerminal(pane, replayingPanesRef, buf)
|
||||
// Ensure cursor is on a new line so the new shell prompt
|
||||
// doesn't trigger zsh's PROMPT_EOL_MARK (%) indicator.
|
||||
replayIntoTerminal(pane, replayingPanesRef, '\r\n')
|
||||
// Clear any mode bits the serialized buffer replayed into xterm.
|
||||
// The shell underneath is fresh and has no TUI consuming these modes.
|
||||
// See POST_REPLAY_MODE_RESET comment.
|
||||
replayIntoTerminal(pane, replayingPanesRef, POST_REPLAY_MODE_RESET)
|
||||
}
|
||||
// Why replayIntoTerminal: the serialized buffer can contain query
|
||||
// sequences that leaked in via the pendingWritesRef flush before
|
||||
// serialization (see TerminalPane capture hook). Writing those
|
||||
// through xterm would trigger auto-replies that land in the new
|
||||
// shell's stdin. See replay-guard.ts.
|
||||
replayIntoTerminal(pane, replayingPanesRef, buffer)
|
||||
// Clear mode bits the serialized buffer replayed into xterm. When
|
||||
// no reattach authoritative write follows (case: fresh attach with
|
||||
// no daemon scrollback), the shell underneath has no TUI consuming
|
||||
// these modes. See POST_REPLAY_MODE_RESET.
|
||||
replayIntoTerminal(pane, replayingPanesRef, POST_REPLAY_MODE_RESET)
|
||||
} catch {
|
||||
// If restore fails, continue with blank terminal.
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user