feat(terminal): tmux pane-buffer primitives and session/render reliability

[aakhter]

Summary

Terminal rendering and buffer-replay foundation for the web UI, extracted as a focused change set (Codex/Gemini-specific redraw handling intentionally left out):

• xterm snapshot/serialize: snapshot + restore xterm state per session on tab switch; persist snapshots to localStorage so terminal content survives tab discard.
• Live tmux pane capture: capture the active pane (active-pane resolution) and combine saved history with the live pane; sync the terminal buffer from the pane.
• Buffer replay: replay tmux snapshots as plain text; position snapshot rows; replay busy shell buffers on tab switch; keep replayed content visible; refresh/reload and repaint stability (fallback anchors, buffer-load ownership).
• Sizing & rendering: keep terminal width stable during scrollback; pin tmux terminal sizing; color/cell-width handling; suppress spurious OSC terminal replies.

Verification

• tsc --noEmit passes
• eslint passes

Notes

This is the shared terminal foundation only; Codex/Gemini run-mode behavior is out of scope and excluded. Extracted from downstream work; includes the related tmux/terminal unit tests.

[Ark0N]

Thanks for extracting this foundation work, @aakhter. I did a deep review (multi-agent + ran the unit suite and loaded the result locally) and unfortunately it isn't landable yet — there are several confirmed blockers, and importantly all of them currently pass CI green, because CI lints only src/**/*.ts (not frontend .js), tsc excludes test/, and the unit suite isn't run in CI. Please npm test -- test/tmux-manager.test.ts and load the page in a browser before re-requesting review. (We're tightening CI to cover these.)

This looks like a partial extraction from a larger "codex CLI mode" branch — several pieces depend on a 'codex'SessionMode and a buildCodexCommand that don't exist on master, which is the root of blocker #3.

Blockers

1. Fatal SyntaxError in session-ui.js — breaks the whole module.finishRename adds const suffix / let fullName but leaves the original const suffix / const fullName in the same block → Identifier 'suffix' has already been declared (verified with node --check). Since session-ui.js is a plain <script> (no bundler), the parse error kills the entire file — inline rename, case settings, detach, and every method it attaches to CodemanApp.prototype become undefined. → delete the leftover duplicate declarations.

2. createSession drops the dedicated -L socket and reverts the cwd hardening (tmux-manager.ts). Changed from ${this.tmux()} new-session … -c ${TMUX_LAUNCH_CWD} to bare tmux new-session … -c "${workingDir}". The session is created on the default tmux server while every other call uses -L <socket> → spawn breaks; it also re-introduces the FUSE/rclone getcwd issue #110 fixed. → restore ${this.tmux()} + -c ${TMUX_LAUNCH_CWD} (and cwd: TMUX_LAUNCH_CWD). The same socket-drop also appears in session.ts's re-attach window-size query (see #4).

3. The snapshot/replay feature is dead code on master, and its tests won't load. The client path is gated on session.mode === 'codex', but SessionMode = 'claude' | 'shell' | 'opencode' — there is no 'codex'. test/tmux-manager.test.ts imports buildCodexCommand (exists nowhere) so the file throws at module load; test/routes/session-routes.test.ts sets mode = 'codex'; a buildEnvExports(…, 'codex') test asserts COLORTERM=truecolor the shipped code doesn't emit. → either include the codex-mode dependency this branch needs, or strip the codex-gating and re-gate the snapshot feature on an existing mode; remove/port the codex-only tests accordingly.

4. session.ts re-attach window-size query drops the -L socket, so it queries the default server, always throws, and falls back to 120×40 — defeating the anti-flicker sizing the surrounding comment promises. → keep queryTmuxWindowSize(muxName, mux.muxSocket).

5. selectSession drops detached-window handling — popped-out tabs now load inline instead of raising their existing window. → restore the _raiseDetached short-circuit.

6. Synchronous pane capture on the /api/sessions/:id/terminal request path for ALL modes.captureActivePaneBuffer runs 3 blocking execSync calls (list-panes / capture-pane / display-message, 5s timeout each) on every terminal fetch / tab switch, un-gated by mode → can stall the Fastify event loop up to ~15s and append a repainted frame onto the (up to 2MB) accumulated buffer. → gate to the relevant mode and move it off the synchronous request path.

Should-fix (medium)

• formatPaneSnapshot paints to cols - 1, dropping the rightmost column of every captured row.
• The localStorage snapshot cache isn't LRU; with >10 live sessions it evicts nothing → shared-quota exhaustion.
• Hardcoded 400ms delay added to every non-shell tab switch.
• Vendored xterm-addon-serialize.min.js: it's genuine-by-content but has no package.json dependency and no regen path. → add @xterm/addon-serialize as a pinned dep (or document the vendoring + a fetch script) so it's reproducible and tracks the xterm v6 internals it depends on.

The sound parts — SGR/grapheme handling in formatPaneSnapshot, the buffer-load owner-token race fix, active-pane resolution, OSC/CSI suppression — are good. Happy to re-review promptly once the blockers are addressed.

[aakhter]

Reworked and force-pushed (now 0569f68, single clean commit). All six blockers addressed; verified locally with the unit suite and a browser load (Playwright), since CI doesn't cover frontend .js, test/, or the unit suite.

Scope decision on the snapshot/replay (blocker #3)

You offered two options — pull in the codex-mode dependency, or re-gate the snapshot feature on an existing mode. I took a third path: removed the codex-specific snapshot/replay from this PR and deferred it to the codex run-mode PR (where the 'codex'SessionMode actually lands).

Reasoning — re-gating on an existing mode would regress it. The client xterm snapshot/replay only helps codex (its TUI redraws drop earlier scrollback, so server byte-replay shows just the latest frame). For claude/shell/opencode the server replay is already correct, so restoring a serialized snapshot on tab-switch would show stale content — and those modes already get instant paint from the existing terminalBufferCache path. So this PR now ships only the mode-agnostic primitives you flagged as sound; the snapshot/replay, the vendored serialize addon, and the live pane-capture return with the codex mode.

Blockers

1. session-ui.js SyntaxError — removed the duplicate suffix/fullName declarations. The rename tests then revealed the same botched merge had also dropped master's finishRename({ commit }) semantics (cancel still fired the PUT; _activeRename never cleared), so I restored those too, keeping this branch's _inlineRenameActive re-render guard. node --check clean; test/inline-rename.test.ts 7/7.
2. createSession socket/cwd — restored ${this.tmux()} dedicated -L socket + -c ${TMUX_LAUNCH_CWD} (and cwd), reinstating the fix: harden tmux launch cwd #110 FUSE/getcwd hardening.
3. snapshot/replay dead code — removed + deferred (above); dropped the codex-only tests (buildCodexCommand, mode='codex', and the COLORTERM=truecolor assertion the shipped code doesn't emit).
4. session.ts re-attach window size — now uses the socket-aware queryTmuxWindowSize(muxName, mux.muxSocket) instead of bare tmux display, so it no longer always falls back to 120×40.
5. selectSession — restored the _raiseDetached short-circuit so popped-out tabs raise their window instead of loading inline.
6. synchronous pane capture — removed captureActivePaneBuffer from the GET /api/sessions/:id/terminal request path (reverts to session.terminalBuffer); no more blocking execSync on tab switch. The capture primitives stay exported + tested for the codex PR to re-add async and bounded.

Should-fixes

The non-LRU snapshot cache and the un-pinned vendored serialize addon are now moot (feature removed). The formatPaneSnapshot rightmost-column (cols-1) and the hardcoded 400 ms non-shell tab-switch delay are still present in the retained primitive / selectSession — happy to fix here or fold into the codex PR, your call.

Verification

test/tmux-manager.test.ts 42 ✓ (was throwing at module load), test/inline-rename.test.ts 7/7 ✓, session-routes + tmux-window-size-query 58 ✓, tsc --noEmit + eslint clean. Browser load via the Playwright mobile/inline-rename suites — fixing the parse error repaired 9 previously-failing tab keyboard/swipe-navigation tests. A full-suite diff vs the prior commit shows net fewer failures and no regressions attributable to this rework (the remaining failures are pre-existing mobile-CSS/visual/perf tests unrelated to these files). Ready for re-review.

[Ark0N]

Thanks for the thorough rework, @aakhter — I re-reviewed 0569f68 and independently re-ran the unit suites, tsc, eslint, and node --check.

✅ All six blockers confirmed fixed

1. session-ui.js SyntaxError + rename semantics — duplicate decls gone, cancel no longer fires the PUT, _activeRename/_inlineRenameActive cleared on both paths. node --check clean.
2. createSession socket/cwd — back through ${this.tmux()} + -c ${TMUX_LAUNCH_CWD}; tmux-manager.test.ts locks the exact tmux -L 'codeman' new-session … -c /tmp string.
3. snapshot/replay dead code — removed cleanly, no orphaned codex/serialize wiring; deferring it to a follow-up makes sense.
4. session.ts window size — socket-aware queryTmuxWindowSize(muxName, mux.muxSocket). ✔
5. selectSession — _raiseDetached short-circuit restored as the first statement. ✔
6. sync pane-capture on GET …/terminal — reverted to session.terminalBuffer; captureActivePaneBuffer has zero call sites. ✔

tsc --noEmit 0, eslint 0, tmux-manager.test.ts 42/42, tmux-capture-color + terminal-layout-css + session-routes 45/45.

🔴 Two new bugs the rework introduced

Both are off the happy path, which is likely why the Playwright load didn't surface them.

1. this._scheduleTerminalRepaint() is called but never defined → TypeError

app.js:655 (WebGL onContextLoss) and app.js:686 (long-task fallback trip) both call this._scheduleTerminalRepaint(), but it's defined nowhere (grep -rn _scheduleTerminalRepaint src/ → only those 2 call sites; it didn't exist pre-PR). The refactor replaced the old inline try { this.terminal.refresh(0, this.terminal.rows - 1); } catch {} with a helper that was never added. When WebGL context is lost / the long-task guard trips, the addon is disposed and then this throws — so the post-fallback repaint is lost and the terminal goes stale/blank on the canvas/DOM renderer. node --check passes (it's a runtime TypeError, not a parse error), which is why CI stayed green.

Fix — add the helper next to _disposeWebGLObserver:

/** * Repaint the full terminal viewport after a renderer swap (WebGL → canvas/DOM). * Scheduled on the next frame so it lands after the addon teardown settles, and * debounced so the context-loss and long-task fallback paths can't double-fire. */_scheduleTerminalRepaint(){if(this._terminalRepaintScheduled)return;this._terminalRepaintScheduled=true;constraf=typeofrequestAnimationFrame==='function' ? requestAnimationFrame : (cb)=>setTimeout(cb,0);raf(()=>{this._terminalRepaintScheduled=false;try{this.terminal?.refresh(0,this.terminal.rows-1);}catch{}});}

(or just revert both call sites to the old inline refresh.)

2. Loading-spinner leaks permanently on switch-away-mid-load

selectSession has six _isStaleSelect(selectGen) early-returns. Four clear the load state first; the two that fire after a chunkedTerminalWrite don't:

 await this.chunkedTerminalWrite(cachedBuffer, TERMINAL_CHUNK_SIZE, bufferLoadOwner);
- if (this._isStaleSelect(selectGen)) return;+ if (this._isStaleSelect(selectGen)) {+ this._clearTerminalLoadState(sessionId, selectGen);+ return;+ }
this.terminal.scrollToBottom();

 await this.chunkedTerminalWrite(data.terminalBuffer, TERMINAL_CHUNK_SIZE, bufferLoadOwner);
- if (this._isStaleSelect(selectGen)) return;+ if (this._isStaleSelect(selectGen)) {+ this._clearTerminalLoadState(sessionId, selectGen);+ return;+ }
// Ensure terminal is scrolled to bottom after buffer load
this.terminal.scrollToBottom();

Switching A→B while A is mid-load resumes A's select, sees it's stale, and returns without clearing — so tab A keeps .tab-loading + the spinner + aria-busy="true" until A is reselected/deleted. _clearTerminalLoadState already no-ops when a newer generation owns the entry, so this is safe.

Two "your call" items

3. The 400ms non-shell tab-switch delay — better to fix it here than defer. It's gated only by mode, so it fires even when dims didn't change (the common same-size tab switch sends no SIGWINCH → no redraw to wait for), taxing the core workflow. Gate it on a real resize: have sendResize report whether dims changed and only wait then —

// terminal-ui.js sendResize(): after computing `dims`constprev=this._lastResizeDims;constchanged=!prev||prev.cols!==dims.cols||prev.rows!==dims.rows;// … set this._lastResizeDims, then `return changed;` on both the WS and HTTP paths// (and `return false` on the early `if (!dims)`).

// app.js selectSession()constdimsChanged=awaitthis.sendResize(sessionId,{forceHttp: true}).catch(()=>false);…if(session?.mode!=='shell'&&dimsChanged){awaitnewPromise((resolve)=>setTimeout(resolve,TUI_REDRAW_SETTLE_MS));// hoist 400 → named const…}

4. formatPaneSnapshotcols-1 — agreed, fold into the codex PR. It's in the capture primitives that have zero production callers on master, and the right fix (paintCols = cols) needs the wide-char/grapheme test expectations re-pinned, which the codex PR's end-to-end capture coverage should own.

One more (low, optional)

resizeWindow() uses execSync on the resize hot path (tmux-manager.ts). It's reachable from the live WS {t:'z'} handler + HTTP /resize; a slow/hung tmux blocks the Fastify event loop. Debounced ~1/gesture so low-severity, but trivial to make non-blocking since Session.resize already fire-and-forgets the result:

exec(`${this.tmux()} resize-window -t ${shellescape(muxName)} -x ${cols} -y ${rows}`,{timeout: EXEC_TIMEOUT_MS},(err)=>{if(err)console.error('[TmuxManager] Failed to resize tmux window:',err);});returntrue;

Net: 1 and 2 are the hard blockers; 3 is a worthwhile cleanup to land here, and the resizeWindow change is optional.

[Ark0N]

Pushed the re-review fixes directly to the branch as b75181b (maintainer edits enabled): items 1, 2, 3, and the optional resizeWindow change. #4 stays folded into the codex PR as discussed.

Re-verified on the merge result — tsc --noEmit 0, eslint 0, node --check on the touched modules clean, tmux-manager.test.ts 42/42, and CI is green on b75181b. Approving. Thanks for the solid rework, @aakhter.