fix(#647): read the pending queue once per indicator update

The try/catch around _pendingBytes() could not keep _updateConnectionIndicator()
from throwing, because _computeConnectionDescriptor() read the queue again,
unguarded, right after; its test only passed because it stubbed the compute
out. The update now reads the queue once and hands the totals to the compute
(which still reads a fresh queue when called on its own), so the hot input
path walks the queue once per keystroke and ACK instead of twice. The stubbed
test is gone: the COD-133 combinatorial test already pins never-throw.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-18 08:08:46 +02:00
1 parent d8cafc6d0d
commit 67ca0954db
2 files changed
+10 -29

No files matched your search

+10 -12
View File
@@ -4287,13 +4287,14 @@ class CodemanApp {
}
}
// Pure render of the header connection indicator: reads only `this.*` state,
// Pure render of the header connection indicator: reads only `this.*` state (and
// the pending totals it is handed, which default to a fresh `_pendingBytes()`),
// touches NO DOM. Returns the exact { display, dotClass, text, title } tuple the
// writer applies. When hidden (display:'none') the other three are normalized to
// '' so the cache compare in _updateConnectionIndicator() is well-defined.
// Every branch/string here must stay byte-identical to what's rendered today.
_computeConnectionDescriptor() {
const { bytes: totalBytes, count } = this._pendingBytes();
_computeConnectionDescriptor(pending = this._pendingBytes()) {
const { bytes: totalBytes, count } = pending;
const hasQueue = count > 0;
// Only surface a backlog once it's more than a few bytes. A single keystroke
// (1B) ACKs in milliseconds, so without this the label flickered "sending 1B"
@@ -4425,14 +4426,11 @@ class CodemanApp {
// the pill is re-rendered when the hold elapses even if no keystroke or ACK
// arrives to trigger it.
// ⚠️ This runs before _drainSession() on every keystroke, so it must never throw: a throw here
// once skipped immediate delivery and made typing lag (the ReferenceError regression above).
let pendingBytes = 0;
try {
pendingBytes = this._pendingBytes().bytes;
} catch {
/* no queue to read: treat as drained */
}
if (pendingBytes > CONNECTION_BACKLOG_HINT_BYTES) {
// once skipped immediate delivery and made typing lag (COD-133; the combinatorial never-throws
// test in test/connection-indicator.test.ts pins it). The queue is walked ONCE per call and the
// same totals are handed to the compute below, since this is the hot input path.
const pending = this._pendingBytes();
if (pending.bytes > CONNECTION_BACKLOG_HINT_BYTES) {
if (this._backlogOverSince == null) this._backlogOverSince = Date.now();
if (!this._backlogHoldTimer && Date.now() - this._backlogOverSince < CONNECTION_BACKLOG_HOLD_MS) {
const wait = CONNECTION_BACKLOG_HOLD_MS - (Date.now() - this._backlogOverSince) + 25;
@@ -4452,7 +4450,7 @@ class CodemanApp {
// Called on EVERY keystroke (_reliableSend) and EVERY ACK (_ackDelivery).
// During fast typing the rendered tuple is usually identical, so skip the DOM
// writes when nothing changed (COD-136) — the compute above is DOM-free.
const next = this._computeConnectionDescriptor();
const next = this._computeConnectionDescriptor(pending);
const prev = this._lastIndicatorDescriptor;
// The tile's value word is written in the UI language, so a language
// switch counts as a change too.
-17
View File
@@ -509,20 +509,3 @@ describe('_updateConnectionIndicator — COD-136 unchanged-skip', () => {
});
});
describe('connection indicator — the backlog tracking never throws on the input path', () => {
it('still renders when the queue cannot be read', () => {
const { app, els } = makeApp({ activeSessionId: 's1', _wsState: 'connected' });
(app as unknown as { _pendingBytes: () => never })._pendingBytes = () => {
throw new Error('no queue');
};
// The compute reads the queue too, so stub it as the real hot path would see a healthy one.
(app as unknown as { _computeConnectionDescriptor: () => unknown })._computeConnectionDescriptor = () => ({
display: 'flex',
dotClass: 'connection-dot connected',
text: 'WS',
title: 'Terminal connected over WebSocket',
});
expect(() => app._updateConnectionIndicator()).not.toThrow();
expect(els.connectionText.textContent).toBe('WS');
});
});