mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
fix(terminal): compare capture geometry only on a visible-frame response
Only a visible-frame capture positions its rows absolutely, so only that frame can be damaged by a terminal of the wrong size. A `full=1` body is linear scrollback closed by a relative cursor move, which is relative precisely so the browser's row count need not match the pane's, and a `history` body is the byte stream, which carries no row alignment to protect. The geometry comparison ran on all three, so it fired most often on the one response it cannot help: `_fullHistoryLoaded` is empty on the first select of every non-shell session per page, and a session whose pane a desktop tab holds too tall to ever fit then paid a second whole-scrollback capture, reset and replay on every page load and every first tab switch. `framePositionsRowsAbsolutely` gates both the captured-geometry comparison and `sizeMovedUnderLoad`. A size that moved under a byte-stream or scrollback replay is healed by xterm's own reflow plus the SIGWINCH the trailing `sendResize` already sends. A pane WIDER than the terminal damages the same frame a second way, so `captureCols` is now compared rather than only logged. `formatPaneSnapshot` paints each row out to the pane's own width, so a narrower browser wraps every painted row, and the wrap on the last one scrolls the whole frame up by a row. The terminal response no longer falls back to `session.ptyCols`/`ptyRows` when the capture reported no geometry. The cursor query is what produces the absolute addressing in the first place, so a capture that lost it returned a raw frame that was never positioned, and a byte-history response was never positioned either. Naming the session's own PTY size there described a frame that does not exist and invited a repair for damage that is not present. `_ptyCols` is also written only by `resize()` while the PTY is spawned at the size queried from tmux, so it can be wrong on its own terms. Both fields are now absent instead, and the `Session` getters added for that fallback go with it. Two browser cases cover the new behaviour and each fails without its fix: a `mux-full-history` response with both dimensions mismatched asserts one fetch (two without the gate), and a `mux-visible` response wider than the terminal but short enough to fit asserts two (one without the width comparison). Corrects a claim in the comment above `capturedGeometry` in tmux-manager.ts. Both replay paths do not address rows absolutely; the full-history one ends in a relative move, which is the whole reason the gate is right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3edf9aae2f
commit
5cfb98fb8b
@@ -3737,20 +3737,6 @@ export class Session extends EventEmitter {
|
||||
private _ptyCols = 120;
|
||||
private _ptyRows = 40;
|
||||
|
||||
/**
|
||||
* The geometry the pane is currently drawing at. A caller that captures the
|
||||
* pane needs this to report the size the frame was built for, and `resize`
|
||||
* can decline a small viewport's request while a desktop claim is live, so
|
||||
* the last size asked for is not always the size in force.
|
||||
*/
|
||||
get ptyCols(): number {
|
||||
return this._ptyCols;
|
||||
}
|
||||
|
||||
get ptyRows(): number {
|
||||
return this._ptyRows;
|
||||
}
|
||||
|
||||
/**
|
||||
* Live WebSocket connections that have announced a desktop viewport for this
|
||||
* session. While at least one is registered, small-viewport (mobile/tablet)
|
||||
|
||||
+7
-4
@@ -3482,10 +3482,13 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
{ encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS }
|
||||
)
|
||||
);
|
||||
// Report the size the pane was really drawing at. Both replay paths below
|
||||
// address rows absolutely, so a consumer whose terminal is shorter than
|
||||
// this piles every overflow row onto its last line and loses the rows it
|
||||
// overwrote. Only the caller can see both sizes, so hand it this one.
|
||||
// Report the size the pane was really drawing at. The visible-frame path
|
||||
// below addresses every row absolutely, so a consumer whose terminal is
|
||||
// shorter than this piles the overflow rows onto its last line and loses
|
||||
// the rows it overwrote. The full-history path instead ends in a RELATIVE
|
||||
// cursor move, which costs it nothing when the two sizes disagree, so the
|
||||
// geometry is reported there for diagnosis rather than for repair. Only
|
||||
// the caller can see both sizes, so hand it this one.
|
||||
if (opts && geometry) opts.capturedGeometry = { cols: geometry.cols, rows: geometry.rows };
|
||||
|
||||
if (fullHistory) {
|
||||
|
||||
+29
-4
@@ -6528,14 +6528,30 @@ class CodemanApp {
|
||||
// size that survived the load rather than the one the capture was taken
|
||||
// at. The two differ whenever the terminal was still settling.
|
||||
const dimsAfterLoad = this.getTerminalDimensions?.();
|
||||
// Only a visible-frame capture positions its rows absolutely, and only
|
||||
// that frame can be damaged by a terminal of the wrong size. A `full=1`
|
||||
// body is linear scrollback closed by a RELATIVE cursor move
|
||||
// (`formatCursorRestore`), which is relative precisely so the browser's
|
||||
// row count need not match the pane's, and a `history` body is the byte
|
||||
// stream, which carries no row alignment to protect. Replaying either at
|
||||
// a different size repairs nothing, and the full-history replay costs a
|
||||
// second whole-scrollback capture to learn that. Since the first select
|
||||
// of every non-shell session per page takes the full-history path, an
|
||||
// ungated comparison fires most often on the one response it cannot help.
|
||||
const framePositionsRowsAbsolutely = data.source === 'mux-visible';
|
||||
const sizeMovedUnderLoad =
|
||||
framePositionsRowsAbsolutely &&
|
||||
!!dimsAtCapture &&
|
||||
!!dimsAfterLoad &&
|
||||
(dimsAfterLoad.cols !== dimsAtCapture.cols || dimsAfterLoad.rows !== dimsAtCapture.rows);
|
||||
// A capture positions every row absolutely, so a pane taller than this
|
||||
// terminal writes its overflow rows onto the last line and loses the rows
|
||||
// it overwrote. That happens when the capture wins a race against the
|
||||
// resize meant to precede it, which is what the retry below repairs.
|
||||
// it overwrote. A pane WIDER than this terminal damages the same frame a
|
||||
// second way: `formatPaneSnapshot` paints each row out to the pane's own
|
||||
// width, so a narrower browser wraps every painted row, and the wrap on
|
||||
// the last one scrolls the whole frame up by a row. Both happen when the
|
||||
// capture wins a race against the resize meant to precede it, which is
|
||||
// what the retry below repairs.
|
||||
//
|
||||
// It also happens when `Session.resize` DECLINED the resize, which it does
|
||||
// for a small viewport while a desktop viewport's size claim is live. The
|
||||
@@ -6545,8 +6561,17 @@ class CodemanApp {
|
||||
// changing who owns the pane size, which is a policy question this does
|
||||
// not touch. What the flag does buy there is that the client can SEE the
|
||||
// mismatch at all, which it previously could not.
|
||||
//
|
||||
// An ABSENT field is not a fit. It means the capture reported no geometry
|
||||
// at all, so nothing was positioned and there is nothing to repair.
|
||||
const capturedTallerThanTerminal =
|
||||
Number.isFinite(data.captureRows) && data.captureRows > (this.terminal?.rows || 0);
|
||||
framePositionsRowsAbsolutely &&
|
||||
Number.isFinite(data.captureRows) &&
|
||||
data.captureRows > (this.terminal?.rows || 0);
|
||||
const capturedWiderThanTerminal =
|
||||
framePositionsRowsAbsolutely &&
|
||||
Number.isFinite(data.captureCols) &&
|
||||
data.captureCols > (this.terminal?.cols || 0);
|
||||
|
||||
// Defer secondary panel updates so they don't block the main thread
|
||||
// after terminal content is already visible.
|
||||
@@ -6654,7 +6679,7 @@ class CodemanApp {
|
||||
// `resizeRetry` caps this at one attempt, so two competing fits cannot
|
||||
// trade replays forever.
|
||||
if (
|
||||
(sizeMovedUnderLoad || capturedTallerThanTerminal) &&
|
||||
(sizeMovedUnderLoad || capturedTallerThanTerminal || capturedWiderThanTerminal) &&
|
||||
!options?.resizeRetry &&
|
||||
!this._isStaleSelect(selectGen)
|
||||
) {
|
||||
|
||||
@@ -2792,10 +2792,16 @@ export function registerSessionRoutes(
|
||||
// positions every row absolutely, so a client whose terminal has fewer
|
||||
// rows than this overwrites its last line with the overflow and loses
|
||||
// the rows underneath. The client compares these against its own size.
|
||||
// Falls back to the session's own geometry when the capture reported
|
||||
// none (cursor query failed, or the buffer came from byte history).
|
||||
captureCols: captureOpts.capturedGeometry?.cols ?? session.ptyCols,
|
||||
captureRows: captureOpts.capturedGeometry?.rows ?? session.ptyRows,
|
||||
//
|
||||
// BOTH FIELDS ARE ABSENT when the capture reported no geometry, and that
|
||||
// is the honest answer rather than a gap to paper over: the cursor query
|
||||
// is what produces the absolute addressing in the first place, so a
|
||||
// capture that lost it returned a raw frame with no row positioning in
|
||||
// it, and a byte-history response was never positioned at all. Reporting
|
||||
// the session's own PTY size here would name a geometry no frame was
|
||||
// built for and invite the client to repair damage that does not exist.
|
||||
captureCols: captureOpts.capturedGeometry?.cols,
|
||||
captureRows: captureOpts.capturedGeometry?.rows,
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user