From 9a0e665f727688a0a8f645c2ce2b0c472d6a84cb Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 14 Aug 2026 00:00:59 +0200 Subject: [PATCH 1/4] fix(terminal): preserve scroll intent across keyboard resize, surface history truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #259, closes #258. Both bottom out in the same gap: nothing tracked whether the user was following live output or reading history. #259 — the keyboard path forced the terminal to the bottom unconditionally (onKeyboardShow/onKeyboardHide passed scrollToBottom:true, applied with no check), so opening the keyboard while scrolled up yanked the user down. The settle cycle now captures intent on its FIRST event, before any fit() has reflowed the buffer, and returns to that anchor when the user was reading. A later capture would read an already-moved viewportY, which is why the capture point matters. The param is renamed restoreScroll to match. Separately, flushPendingWrites gated viewport preservation on _hasRecentUserScrollUp(), a 1500ms decay window, so a user who scrolled up and then actually READ for longer lost protection mid-read. Being scrolled up IS the intent however long ago it was expressed, so it now keys off position. The recency window stays as a race guard on the sticky scroll-to-bottom. The full-history repull already held the user's place and is unchanged. #258 — truncation was reported by a grey line written INTO the terminal ("earlier output truncated"), which scrolls away with the output it describes, cannot be acted on, and said the same thing whether the rest was one click away or gone forever. The server set one `truncated` boolean at two sites meaning opposite things, and the client discarded fullSize and source entirely. The route now reports truncationReason ('tail' = intentional partial replay, the rest is retained; 'capped' = the byte ceiling dropped it) plus retainedBytes, and 'capped' is not downgraded by a later tail cut. The client renders a dismissible banner outside terminal output with three honest states: recoverable (offers Load full history), at-ceiling, and exhausted. The Load button forces past the scroll cooldown but NOT past _replayWouldShrinkBuffer, which still refuses a downgrade for repaint-mode panes. The banner is an overlay, not a flex child: FitAddon derives rows/cols from the terminal parent's computed height, so occupying real layout space would SIGWINCH the CLI on every truncation-state change. Verified in a real browser on the 7 skins: banner text and button clear 4.5:1 contrast on all of them, and terminal height is byte-identical with the banner shown. The first cut used --bg-elevated and --accent-muted, which do not exist, so light skins rendered a hardcoded dark bar under dark text; it now uses only tokens every skin redefines. test/terminal-scroll-intent.test.ts lives outside test/mobile/ deliberately — that suite is excluded from test:ci, so a guard placed there is invisible to CI. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/public/app.js | 111 +++++++++++++++-- src/web/public/constants.js | 66 +++++++++++ src/web/public/index.html | 5 + src/web/public/mobile-handlers.js | 64 ++++++++-- src/web/public/styles.css | 77 ++++++++++++ src/web/public/terminal-ui.js | 13 +- src/web/routes/session-routes.ts | 16 +++ test/history-truncation-notice.test.ts | 132 +++++++++++++++++++++ test/mobile/keyboard.test.ts | 4 +- test/routes/session-routes.test.ts | 72 ++++++++++++ test/terminal-scroll-intent.test.ts | 157 +++++++++++++++++++++++++ test/terminal-scroll-routing.test.ts | 4 +- 12 files changed, 699 insertions(+), 22 deletions(-) create mode 100644 test/history-truncation-notice.test.ts create mode 100644 test/terminal-scroll-intent.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index d80aa7305..2c378b9db 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -4509,28 +4509,36 @@ class CodemanApp { * gets a much longer cooldown so a hollow pane stops re-fetching megabytes on * every scroll-up (issue #205, round 2). */ - async _maybeRefetchFullHistory() { + async _maybeRefetchFullHistory({ force = false } = {}) { const sessionId = this.activeSessionId; if (!sessionId || this._fullHistoryRepullInFlight || this._isLoadingBuffer) return; if (this.detachedSessions?.has(sessionId)) return; const now = Date.now(); // Momentum scrolling fires this dozens of times per flick, and a burst of new // output is the normal reason to want a re-pull, so cooldown rather than latch. + // `force` is the user pressing "Load full history" (#258): they asked once, + // explicitly, so the scroll-gesture cooldown does not apply. The downgrade + // guard below still does — a forced pull must not destroy history either. const cooldown = this._fullHistoryRepullUseless?.has(sessionId) ? 60000 : 4000; - if (now - (this._fullHistoryRepullAt.get(sessionId) || 0) < cooldown) return; + if (!force && now - (this._fullHistoryRepullAt.get(sessionId) || 0) < cooldown) return; this._fullHistoryRepullAt.set(sessionId, now); this._fullHistoryRepullInFlight = true; try { const res = await fetch(`/api/sessions/${sessionId}/terminal?full=1`); - const buffer = (await res.json())?.data?.terminalBuffer; + const payload = (await res.json())?.data ?? {}; + const buffer = payload.terminalBuffer; // Bail on a tab switch mid-fetch: writing here would paint another session's // history into the terminal the user is now looking at. if (!buffer || this.activeSessionId !== sessionId) return; if (this._replayWouldShrinkBuffer(buffer)) { (this._fullHistoryRepullUseless ||= new Set()).add(sessionId); this._logScrollRouting?.('repull-refused-downgrade'); + // The browser already holds more than tmux can give back, so there is + // nothing further to offer and the indicator must stop promising it. + this._setHistoryTruncation(sessionId, { ...payload, exhausted: true }); return; } + this._setHistoryTruncation(sessionId, payload); this._fullHistoryRepullUseless?.delete(sessionId); const rowsBefore = this.terminal.buffer.active.length; this._resetTerminalForReplay(); @@ -4551,6 +4559,89 @@ class CodemanApp { } } + /** + * Record how much history a replay actually carried, and refresh the banner. + * + * Called from every path that writes a fetched buffer into xterm. Keyed by + * session because the banner describes the ACTIVE tab and a background fetch + * must not relabel it. + */ + _setHistoryTruncation(sessionId, payload = {}) { + if (!sessionId) return; + (this._historyTruncation ||= new Map()).set(sessionId, { + truncated: !!payload.truncated, + reason: payload.truncationReason ?? null, + source: payload.source ?? null, + fullSize: payload.fullSize ?? 0, + retainedBytes: payload.retainedBytes ?? 0, + // Set once a full-history pull has been refused as a downgrade: the + // browser holds more than the server can return, so there is no more. + exhausted: !!payload.exhausted, + }); + if (sessionId === this.activeSessionId) this._renderHistoryTruncationBanner(); + } + + /** Drop banner state for a session that is going away. */ + _clearHistoryTruncation(sessionId) { + this._historyTruncation?.delete(sessionId); + if (sessionId === this.activeSessionId) this._renderHistoryTruncationBanner(); + } + + /** + * Paint the partial-history banner for the active session. + * + * Three distinct states, because "we tailed for speed" and "the oldest output + * is gone forever" are not the same message and the old single boolean could + * not tell them apart: + * - recoverable → offer to load the rest + * - exhausted → say so plainly, offer nothing + * - at the limit → the full capture ITSELF hit the byte ceiling + */ + _renderHistoryTruncationBanner() { + const bar = document.getElementById('historyTruncationBar'); + if (!bar) return; + const state = this.activeSessionId ? this._historyTruncation?.get(this.activeSessionId) : null; + const notice = computeHistoryTruncationNotice(state || {}); + if (!notice.visible) { + bar.hidden = true; + return; + } + + bar.textContent = ''; + const label = document.createElement('span'); + label.className = 'history-trunc-text'; + label.textContent = notice.message; + bar.appendChild(label); + + if (notice.canLoadMore) { + const btn = document.createElement('button'); + btn.type = 'button'; + btn.className = 'history-trunc-load'; + btn.textContent = 'Load full history'; + btn.onclick = () => { + btn.disabled = true; + btn.textContent = 'Loading…'; + // Forced: the cooldown exists to throttle scroll gestures, not choices. + this._maybeRefetchFullHistory({ force: true }).finally(() => { + this._renderHistoryTruncationBanner(); + }); + }; + bar.appendChild(btn); + } + + const dismiss = document.createElement('button'); + dismiss.type = 'button'; + dismiss.className = 'history-trunc-dismiss'; + dismiss.setAttribute('aria-label', 'Dismiss history notice'); + dismiss.textContent = '×'; + dismiss.onclick = () => { + bar.hidden = true; + }; + bar.appendChild(dismiss); + + bar.hidden = false; + } + _shouldFocusTerminalForTabSwitch() { if (typeof MobileDetection === 'undefined' || !MobileDetection.isTouchDevice()) { return true; @@ -4607,6 +4698,10 @@ class CodemanApp { this._cleanupPreviousSession(sessionId); this.activeSessionId = sessionId; + // Repaint the partial-history banner for the tab being switched TO. The + // replay paths refresh it when their fetch lands; without this the previous + // session's notice stays on screen until then (#258). + this._renderHistoryTruncationBanner(); try { localStorage.setItem('codeman-active-session', sessionId); } catch {} // Narrow SSE filter to the active session — server stops streaming // session:terminal events for other sessions to this client. Cuts @@ -4858,10 +4953,11 @@ class CodemanApp { _crashDiag.log(`REWRITE: ${(data.terminalBuffer.length/1024).toFixed(0)}KB`); this._setTerminalLoadState(sessionId, selectGen, 'replaying'); this._resetTerminalForReplay(); - // Show truncation indicator if buffer was cut - if (data.truncated) { - this.terminal.write('\x1b[90m... (earlier output truncated for performance) ...\x1b[0m\r\n\r\n'); - } + // Truncation is reported OUT OF BAND (#258). This used to write a grey + // "... earlier output truncated ..." line into the + // terminal itself, which scrolls away with the output it describes, + // cannot be actioned, and is indistinguishable from real CLI output. + this._setHistoryTruncation(sessionId, data); // Use chunked write for large buffers to avoid UI jank await this.chunkedTerminalWrite(data.terminalBuffer, TERMINAL_CHUNK_SIZE, bufferLoadOwner); if (this._isStaleSelect(selectGen)) { @@ -5043,6 +5139,7 @@ class CodemanApp { } this.terminalBuffers.delete(sessionId); this.terminalBufferCache.delete(sessionId); + this._clearHistoryTruncation(sessionId); this._xtermSnapshots?.delete(sessionId); try { localStorage.removeItem(`codeman-xs-${sessionId}`); } catch {} diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 1d206fd13..a0609a2e5 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -785,3 +785,69 @@ function escapeHtml(text) { if (typeof text !== 'string') return ''; return text.replace(_htmlEscapePattern, (ch) => _htmlEscapeMap[ch]); } + +/** + * Human-readable byte size for the partial-history banner (#258). + * + * Deliberately coarse: the banner is telling the user roughly how much of a + * transcript they are looking at, not accounting for bytes. Sub-KB amounts read + * as "less than 1 KB" rather than an exact count nobody can act on. + * + * @param {number} bytes + * @returns {string} + */ +function formatHistoryBytes(bytes) { + const n = typeof bytes === 'number' && isFinite(bytes) && bytes > 0 ? bytes : 0; + if (n < 1024) return 'less than 1 KB'; + if (n < 1024 * 1024) return `${Math.round(n / 1024)} KB`; + return `${(n / (1024 * 1024)).toFixed(1)} MB`; +} + +/** + * Decide what the partial-history banner should say (#258). + * + * PURE so the three states can be tested without a DOM. They exist because one + * `truncated` boolean could not distinguish messages the user acts on very + * differently: + * - recoverable: we tailed for speed and the rest is still retained + * - atCeiling: the FULL capture itself hit the byte ceiling + * - exhausted: a full pull was refused as a downgrade, so this is all there is + * + * @param {{truncated?: boolean, reason?: string|null, source?: string|null, + * fullSize?: number, retainedBytes?: number, exhausted?: boolean}} state + * @returns {{visible: boolean, message: string, canLoadMore: boolean}} + */ +function computeHistoryTruncationNotice(state = {}) { + if (!state.truncated) return { visible: false, message: '', canLoadMore: false }; + + const retained = Math.max(0, state.retainedBytes || 0); + const dropped = Math.max(0, (state.fullSize || 0) - retained); + const shown = formatHistoryBytes(retained); + // A full-history capture that was STILL capped is already everything tmux + // holds, so the remainder is out of reach rather than one request away. + const atCeiling = state.source === 'mux-full-history' && state.reason === 'capped'; + + if (state.exhausted) { + return { + visible: true, + message: `Showing all ${shown} of retained history. Earlier output is no longer kept for this session.`, + canLoadMore: false, + }; + } + if (atCeiling) { + return { + visible: true, + message: `Showing the most recent ${shown}. Earlier output exceeds the retained history limit and cannot be recovered.`, + canLoadMore: false, + }; + } + return { + visible: true, + message: `Showing the most recent ${shown} of this session. ${formatHistoryBytes(dropped)} more may still be retained.`, + canLoadMore: true, + }; +} + +if (typeof window !== 'undefined') { + window.CodemanHistoryFormat = { formatHistoryBytes, computeHistoryTruncationNotice }; +} diff --git a/src/web/public/index.html b/src/web/public/index.html index c2e8966c3..2983ac6ef 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -310,6 +310,11 @@
+ +