From 67ca0954dbb01d0c80e80326bfb321853a45aca6 Mon Sep 17 00:00:00 2001
From: Codeman maintainer <noreply@anthropic.com>
Date: Sun, 18 Oct 2026 08:08:46 +0200
Subject: [PATCH] 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>
---
 src/web/public/app.js             | 22 ++++++++++------------
 test/connection-indicator.test.ts | 17 -----------------
 2 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/src/web/public/app.js b/src/web/public/app.js
index f15906b2..14a751b0 100644
--- a/src/web/public/app.js
+++ b/src/web/public/app.js
@@ -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.
diff --git a/test/connection-indicator.test.ts b/test/connection-indicator.test.ts
index 09acc8ec..929c1efe 100644
--- a/test/connection-indicator.test.ts
+++ b/test/connection-indicator.test.ts
@@ -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');
-  });
-});