mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(terminal): remember a geometry replay that did not converge
`resizeRetry` caps the recursion inside one select and says nothing about the next one, so a pane this browser cannot size reported the same mismatch on every select and bought the same failed repair each time: two fetches per tab switch for the life of the page, measured as a running count of 2, 4, 6 across three selects. That is the case this branch describes as happening every time rather than occasionally, a phone whose resize `Session.resize` declines while a desktop claim is live, and it is not the only one — any pane Codeman cannot size lands there, including one a second tmux client is also holding. Each wasted pass costs another `capture-pane`, which is `execSync` and blocks the server's event loop, plus a reset and chunked rewrite, a discarded snapshot and cache entry, and a dropped and reopened WebSocket. `_geometryRetryUseless` mirrors the existing `_fullHistoryRepullUseless`: a retry pass whose frame still does not fit adds the session, geometry that fits removes it, and the replay gate consults it. The proof has to come from a retry pass rather than a first one, because the retry ran at the size that stuck and the pane ignored it. Clearing on a fitting frame is what stops a pane that becomes sizeable again, once the desktop tab closes or its claim goes idle, from staying permanently unrepaired. The race case never reaches the latch, since it converges on its first attempt. The new browser case walks all of that: three selects reading 2, 3, 4 instead of 2, 4, 6, then a fitting frame, then a mismatch diagnosed afresh. Without the gate it fails on the second switch with `expected 4 to be 3`. Rebased onto master, which has moved to 1.30.0 and taken #436. The one conflict was `config/test-suites.ts`, where both branches appended a glob to `BROWSER_TEST_GLOBS`; both are kept. Everything else merged clean, #436's own changes to the same buffer-load path included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
383f834704
commit
95dc6fe944
@@ -549,6 +549,12 @@ class CodemanApp {
|
||||
// repaint-mode CLI pane, where tmux keeps no history of its own). The pull is
|
||||
// refused for those and retried far more slowly — see _maybeRefetchFullHistory.
|
||||
this._fullHistoryRepullUseless = new Set();
|
||||
// Sessions where the geometry replay has already been tried and did NOT
|
||||
// converge, so the pane is one this browser cannot size. Mirrors the Set
|
||||
// above: `resizeRetry` caps the recursion inside one select, and this is
|
||||
// what stops a fresh select from paying for the same answer again — see
|
||||
// the geometry gate in selectSession.
|
||||
this._geometryRetryUseless = new Set();
|
||||
this.terminalLoadStates = new Map(); // Map<sessionId, { generation, phase }>
|
||||
this.respawnStatus = {};
|
||||
this.respawnTimers = {}; // Track timed respawn timers
|
||||
@@ -6718,6 +6724,33 @@ class CodemanApp {
|
||||
this._clearTerminalLoadState(sessionId, selectGen);
|
||||
_crashDiag.log(`SELECT_DONE: ${selectDoneMs.toFixed(0)}ms`);
|
||||
console.log(`[CRASH-DIAG] selectSession DONE: ${sessionId.slice(0,8)} in ${selectDoneMs.toFixed(0)}ms`);
|
||||
// Remember whether the replay was worth it, because `resizeRetry` only
|
||||
// caps the recursion INSIDE one select and says nothing about the next
|
||||
// one. A pane this browser cannot size — one whose resize `Session.resize`
|
||||
// declines while a desktop claim is live, or one a second tmux client is
|
||||
// also holding — reports the same mismatch on every select, so without a
|
||||
// memo the diagnosis is paid for again on every tab switch, forever: two
|
||||
// fetches per select rather than one. Each extra pass costs a second
|
||||
// `capture-pane`, which is `execSync` and blocks the server's event loop,
|
||||
// plus a reset and chunked rewrite, a discarded snapshot and cache entry,
|
||||
// and a dropped and reopened WebSocket.
|
||||
//
|
||||
// A retry pass that STILL does not fit is the proof, since the retry ran
|
||||
// at the size that stuck and the pane ignored it. Geometry that fits
|
||||
// clears the memo, so a pane that becomes sizeable again (the desktop tab
|
||||
// closes, the claim goes idle) is repaired on the next select. The race
|
||||
// case is untouched: it converges on its first attempt, so it never
|
||||
// reaches the branch that latches.
|
||||
const capturedGeometryFits =
|
||||
framePositionsRowsAbsolutely &&
|
||||
Number.isFinite(data.captureRows) &&
|
||||
!capturedTallerThanTerminal &&
|
||||
!capturedWiderThanTerminal;
|
||||
if (capturedGeometryFits) {
|
||||
this._geometryRetryUseless?.delete(sessionId);
|
||||
} else if (options?.resizeRetry && (capturedTallerThanTerminal || capturedWiderThanTerminal)) {
|
||||
(this._geometryRetryUseless ||= new Set()).add(sessionId);
|
||||
}
|
||||
// What is on screen was drawn for a geometry this terminal does not have.
|
||||
// Replaying once against the size that stuck is the only thing that
|
||||
// repairs it: SIGWINCH reaches the CLI only on a real size change, and
|
||||
@@ -6727,6 +6760,7 @@ class CodemanApp {
|
||||
if (
|
||||
(sizeMovedUnderLoad || capturedTallerThanTerminal || capturedWiderThanTerminal) &&
|
||||
!captureMatchesRequestedSize &&
|
||||
!this._geometryRetryUseless?.has(sessionId) &&
|
||||
!options?.resizeRetry &&
|
||||
!this._isStaleSelect(selectGen)
|
||||
) {
|
||||
|
||||
@@ -98,6 +98,41 @@ async function stubTerminal(
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* As `stubTerminal`, but reading its geometry from a holder the test can change
|
||||
* between selects. That is what lets one case watch a pane stop fitting and
|
||||
* start fitting again, which a stub fixed at construction cannot show.
|
||||
*/
|
||||
async function stubTerminalDynamic(
|
||||
page: Page,
|
||||
counter: { n: number; urls: string[] },
|
||||
state: { captureRows: number; captureCols: number }
|
||||
) {
|
||||
await page.route('**/api/sessions/*/terminal*', async (route) => {
|
||||
const url = route.request().url();
|
||||
counter.n += 1;
|
||||
counter.urls.push(url);
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({
|
||||
success: true,
|
||||
data: {
|
||||
terminalBuffer: paneSnapshot(state.captureRows),
|
||||
status: 'idle',
|
||||
fullSize: 1024,
|
||||
retainedBytes: 1024,
|
||||
truncated: false,
|
||||
truncationReason: null,
|
||||
source: url.includes('full=1') ? 'mux-full-history' : 'mux-visible',
|
||||
captureCols: state.captureCols,
|
||||
captureRows: state.captureRows,
|
||||
},
|
||||
}),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Answer every fetch with the geometry the client itself is asking for, read
|
||||
* live from the page. That is the clamp signature: `getTerminalDimensions()`
|
||||
@@ -341,6 +376,55 @@ describe('a capture bigger than the terminal', () => {
|
||||
await context.close();
|
||||
}, 60_000);
|
||||
|
||||
it('replays once per session, not once per tab switch, when it cannot converge', async () => {
|
||||
// `resizeRetry` caps the recursion inside ONE select and says nothing about
|
||||
// the next one, so a pane this browser cannot size reported the same
|
||||
// mismatch on every select and bought the same failed repair every time:
|
||||
// two fetches per tab switch for the life of the page. That is the case the
|
||||
// description calls "every time rather than occasionally", a phone whose
|
||||
// resize is declined while a desktop claim is live, and it is not the only
|
||||
// one — any pane Codeman cannot size lands there, a second tmux client
|
||||
// attached to it included. Each wasted pass costs another `capture-pane`,
|
||||
// which is `execSync` on the server's event loop, plus a reset and rewrite,
|
||||
// a discarded snapshot, and a dropped and reopened WebSocket.
|
||||
context = await browser.newContext({ viewport: { width: 1280, height: 800 } });
|
||||
page = await context.newPage();
|
||||
const sessionId = await openSession(page);
|
||||
|
||||
const fetches = { n: 0, urls: [] as string[] };
|
||||
const pane = { captureRows: 200, captureCols: 200 };
|
||||
await stubTerminalDynamic(page, fetches, pane);
|
||||
await consumeFullHistory(page, sessionId, fetches);
|
||||
|
||||
// First tab switch: one load, one replay, and the replay does not fit
|
||||
// either, which is the proof that this pane ignores the size it is given.
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
expect(fetches.n).toBe(2);
|
||||
|
||||
// Every switch after it pays once. Unlatched this reads 4 then 6.
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
expect(fetches.n).toBe(3);
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
expect(fetches.n).toBe(4);
|
||||
|
||||
// The memo has to lift when the pane becomes sizeable again, or closing the
|
||||
// desktop tab that was holding it would leave this session permanently
|
||||
// unrepaired. A frame that fits clears it...
|
||||
pane.captureRows = 5;
|
||||
pane.captureCols = 40;
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
expect(fetches.n).toBe(5);
|
||||
|
||||
// ...so the next genuine mismatch is diagnosed again.
|
||||
pane.captureRows = 200;
|
||||
pane.captureCols = 200;
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
expect(fetches.n).toBe(7);
|
||||
|
||||
await closeSession(page, sessionId);
|
||||
await context.close();
|
||||
}, 60_000);
|
||||
|
||||
it('hands over text typed but not yet submitted before it replays', async () => {
|
||||
// On a touch device the characters the user has typed live ONLY in the
|
||||
// local-echo overlay until Enter; they have never reached the PTY. The
|
||||
|
||||
Reference in New Issue
Block a user