mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
fix(terminal): address re-review findings on the pane-buffer rework
Follow-up to the PR #112 re-review (all six prior blockers were already resolved; these are new issues the rework introduced): - app.js: define the missing `_scheduleTerminalRepaint()` helper. It was called from both WebGL-fallback paths (onContextLoss + long-task trip) but defined nowhere, so each fallback threw `TypeError` and lost the post-fallback repaint, leaving a stale/blank terminal. Implemented as an rAF-debounced full refresh (matches the old inline `terminal.refresh`). - app.js: clear terminal load-state on the two post-write stale-select early-returns (cached-buffer + rewrite branches), matching the other four checks. Switching away from a mid-loading tab no longer leaks a permanent `.tab-loading` spinner / `aria-busy=true`. - terminal-ui.js + app.js: gate the post-resize TUI-redraw settle on an actual dimension change. `sendResize` now returns whether dims changed; a same-size tab switch sends no SIGWINCH, so the wait is skipped instead of charging a flat tax on every non-shell switch. Literal hoisted to `TUI_REDRAW_SETTLE_MS`. - tmux-manager.ts: `resizeWindow()` uses a non-blocking `exec` instead of `execSync` so the interactive WS/HTTP resize path can't stall the Fastify event loop on a slow/hung tmux. Sole caller already fire-and- forgets the result; test updated to assert the async dispatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+13
-10
@@ -2137,16 +2137,19 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
execSync(`${this.tmux()} resize-window -t ${shellescape(muxName)} -x ${cols} -y ${rows}`, {
|
||||
timeout: EXEC_TIMEOUT_MS,
|
||||
stdio: 'ignore',
|
||||
});
|
||||
return true;
|
||||
} catch (err) {
|
||||
console.error('[TmuxManager] Failed to resize tmux window:', err);
|
||||
return false;
|
||||
}
|
||||
// Fire-and-forget: this runs on the interactive resize path (WS {t:'z'} and
|
||||
// HTTP /resize), so use a non-blocking exec — a slow/hung tmux must not stall
|
||||
// the Fastify event loop while other sessions' input/SSE are served. The sole
|
||||
// caller (Session.resize) ignores the result, and under `window-size manual`
|
||||
// the subsequent ptyProcess.resize is subordinate to this authoritative size.
|
||||
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);
|
||||
}
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
isAvailable(): boolean {
|
||||
|
||||
+31
-8
@@ -702,6 +702,22 @@ class CodemanApp {
|
||||
this._webglLongTaskObserver = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* No-ops safely if the terminal isn't ready.
|
||||
*/
|
||||
_scheduleTerminalRepaint() {
|
||||
if (this._terminalRepaintScheduled) return;
|
||||
this._terminalRepaintScheduled = true;
|
||||
const raf = typeof requestAnimationFrame === 'function' ? requestAnimationFrame : (cb) => setTimeout(cb, 0);
|
||||
raf(() => {
|
||||
this._terminalRepaintScheduled = false;
|
||||
try { this.terminal?.refresh(0, this.terminal.rows - 1); } catch {}
|
||||
});
|
||||
}
|
||||
|
||||
_disableWebGLSticky(reason) {
|
||||
try {
|
||||
localStorage.setItem('codeman-webgl-disabled', JSON.stringify({ reason, at: Date.now() }));
|
||||
@@ -3008,7 +3024,7 @@ class CodemanApp {
|
||||
// a small region with empty rows below the status bar.
|
||||
// sendResize is a no-op on the server when dims haven't changed, so
|
||||
// calling it every tab switch is cheap.
|
||||
await this.sendResize(sessionId, { forceHttp: true }).catch(() => {});
|
||||
const dimsChanged = await this.sendResize(sessionId, { forceHttp: true }).catch(() => false);
|
||||
if (this._isStaleSelect(selectGen)) {
|
||||
this._clearTerminalLoadState(sessionId, selectGen);
|
||||
return;
|
||||
@@ -3027,7 +3043,10 @@ class CodemanApp {
|
||||
this._setTerminalLoadState(sessionId, selectGen, 'replaying');
|
||||
this._resetTerminalForReplay();
|
||||
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();
|
||||
_crashDiag.log('CACHE_DONE');
|
||||
} else if (sessionIsBusy) {
|
||||
@@ -3037,11 +3056,12 @@ class CodemanApp {
|
||||
}
|
||||
|
||||
// Give TUI sessions a short chance to redraw after resize before the
|
||||
// fresh buffer fetch captures the live mux pane. Shell sessions and
|
||||
// snapshot restores do not need this delay, so terminal content can
|
||||
// appear immediately when switching between shells.
|
||||
if (session?.mode !== 'shell') {
|
||||
await new Promise((resolve) => setTimeout(resolve, 400));
|
||||
// fresh buffer fetch. Only needed when the resize actually changed
|
||||
// dimensions (a real SIGWINCH → Ink redraw); a same-size tab switch sent
|
||||
// no resize, so waiting would just add latency. Shell sessions never need
|
||||
// it, so terminal content can appear immediately when switching shells.
|
||||
if (session?.mode !== 'shell' && dimsChanged) {
|
||||
await new Promise((resolve) => setTimeout(resolve, TUI_REDRAW_SETTLE_MS));
|
||||
if (this._isStaleSelect(selectGen)) {
|
||||
this._clearTerminalLoadState(sessionId, selectGen);
|
||||
return;
|
||||
@@ -3075,7 +3095,10 @@ class CodemanApp {
|
||||
}
|
||||
// Use chunked write for large buffers to avoid UI jank
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -57,6 +57,7 @@ const TERMINAL_CHUNK_SIZE = 32 * 1024; // 32KB chunks for terminal buffer l
|
||||
const TERMINAL_TAIL_SIZE = 1024 * 1024; // 1MB tail for initial load (more scrollback on tab switch)
|
||||
const SYNC_WAIT_TIMEOUT_MS = 50; // Wait timeout for terminal sync
|
||||
const STATS_POLLING_INTERVAL_MS = 2000; // System stats polling
|
||||
const TUI_REDRAW_SETTLE_MS = 400; // Grace for a TUI to redraw after a real resize, before fetching its buffer
|
||||
|
||||
// Z-index base values for layered floating windows
|
||||
const ZINDEX_SUBAGENT_BASE = 1000;
|
||||
|
||||
@@ -1906,7 +1906,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
// terminal size matches what we report to the server PTY.
|
||||
if (this.fitAddon) this.fitAddon.fit();
|
||||
const dims = this.getTerminalDimensions();
|
||||
if (!dims) return;
|
||||
if (!dims) return false;
|
||||
// Did the dimensions actually change since the last resize we sent? Callers
|
||||
// use this to skip work (e.g. the post-resize TUI-redraw settle) when no
|
||||
// real SIGWINCH was triggered — switching tabs at the same browser size is
|
||||
// a no-op on the server and needs no redraw grace.
|
||||
const prev = this._lastResizeDims;
|
||||
const changed = !prev || prev.cols !== dims.cols || prev.rows !== dims.rows;
|
||||
// Update _lastResizeDims so the throttledResize handler won't redundantly
|
||||
// clear the terminal for the same dimensions (which would blank the screen
|
||||
// without a subsequent Ink redraw to repaint it).
|
||||
@@ -1915,7 +1921,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
if (!options.forceHttp && this._wsReady && this._wsSessionId === sessionId) {
|
||||
try {
|
||||
this._ws.send(JSON.stringify({ t: 'z', c: dims.cols, r: dims.rows }));
|
||||
return;
|
||||
return changed;
|
||||
} catch {
|
||||
// Fall through to HTTP POST
|
||||
}
|
||||
@@ -1925,6 +1931,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(dims),
|
||||
});
|
||||
return changed;
|
||||
},
|
||||
|
||||
/**
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { TmuxManager, formatPaneSnapshot, parsePaneList, resolveActivePaneTarget } from '../src/tmux-manager.js';
|
||||
import { execSync } from 'node:child_process';
|
||||
import { execSync, exec } from 'node:child_process';
|
||||
|
||||
// ============================================================================
|
||||
// Unit Tests (mocked)
|
||||
@@ -64,6 +64,7 @@ vi.mock('node:fs/promises', async () => {
|
||||
describe('TmuxManager (unit)', () => {
|
||||
let manager: TmuxManager;
|
||||
const mockedExecSync = vi.mocked(execSync);
|
||||
const mockedExec = vi.mocked(exec);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -131,9 +132,11 @@ describe('TmuxManager (unit)', () => {
|
||||
it('resizes the tmux window when Codeman accepts a desktop resize', () => {
|
||||
expect(manager.resizeWindow('codeman-abc12345', 140, 42)).toBe(true);
|
||||
|
||||
expect(mockedExecSync).toHaveBeenCalledWith(
|
||||
// Non-blocking exec (not execSync) on the interactive resize hot path.
|
||||
expect(mockedExec).toHaveBeenCalledWith(
|
||||
"tmux -L 'codeman' resize-window -t 'codeman-abc12345' -x 140 -y 42",
|
||||
expect.objectContaining({ stdio: 'ignore' })
|
||||
expect.objectContaining({ timeout: expect.any(Number) }),
|
||||
expect.any(Function)
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user