From 9433790bf06982079158829cd2485b6600cdfa67 Mon Sep 17 00:00:00 2001 From: Yashwant Kotipalli Date: Sun, 3 May 2026 17:27:00 -0700 Subject: [PATCH] fix(browse): declare lastConsoleFlushed to restore console-log persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flushBuffers() references a `lastConsoleFlushed` cursor at server.ts:337 and assigns it at :344, but the `let lastConsoleFlushed = 0;` declaration is missing — only the network and dialog siblings are declared at lines 327-328. Result: every 1-second flushBuffers tick (line 376) throws `ReferenceError: lastConsoleFlushed is not defined`, gets swallowed by the catch at line 369 ("[browse] Buffer flush failed: ..."), and the console branch's append never runs. browse-console.log is never written in any production deployment since this regressed. Discovered by stress-testing the daemon with 15 concurrent CLIs against cold state — the race surfaced the buffer-flush error spam in one spawned daemon's stderr. Verified by running the daemon against a real file:// page with console.log events: in-memory `browse console` returns the entries, but `.gstack/browse-console.log` is never created on disk. Regression introduced by 1a100a2a "fix: eliminate duplicate command sets in chain, improve flush perf and type safety" — the flush refactor switched from `Bun.write` to `fs.appendFileSync` and added the `lastConsoleFlushed` cursor pattern alongside its network/dialog siblings, but missed the matching `let` declaration. Tests don't currently exercise flushBuffers, so the regression shipped silently. Fix: - Declare `let lastConsoleFlushed = 0;` next to `lastNetworkFlushed` and `lastDialogFlushed` (browse/src/server.ts:327) - Add a source-level guard test (browse/test/server-flush-trackers.test.ts) that fails any future refactor that adds a fourth `last*Flushed` cursor without the matching declaration. Same pattern as terminal-agent.test.ts and dual-listener.test.ts — read source as text, assert invariant, no daemon required. Test plan: - [x] New regression test fails on current main, passes with the fix - [x] `bun run build` clean - [x] Manual smoke: spawn daemon -> goto file:// page with console.log -> wait 4s -> .gstack/browse-console.log now exists with the expected entries (163 bytes vs zero before) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- browse/src/server.ts | 1 + browse/test/server-flush-trackers.test.ts | 73 +++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 browse/test/server-flush-trackers.test.ts diff --git a/browse/src/server.ts b/browse/src/server.ts index 042616e75b..e63a87e949 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -324,6 +324,7 @@ const DIALOG_LOG_PATH = config.dialogLog; // terminal-agent.ts; chat queue + per-tab agent multiplexing are no // longer needed. +let lastConsoleFlushed = 0; let lastNetworkFlushed = 0; let lastDialogFlushed = 0; let flushInProgress = false; diff --git a/browse/test/server-flush-trackers.test.ts b/browse/test/server-flush-trackers.test.ts new file mode 100644 index 0000000000..306729af4e --- /dev/null +++ b/browse/test/server-flush-trackers.test.ts @@ -0,0 +1,73 @@ +/** + * Regression: flushBuffers state-tracker declaration audit. + * + * `flushBuffers()` (server.ts) maintains per-buffer cursors so it only + * appends *new* entries to each on-disk log on every interval tick: + * + * const newConsoleCount = consoleBuffer.totalAdded - lastConsoleFlushed; + * const newNetworkCount = networkBuffer.totalAdded - lastNetworkFlushed; + * const newDialogCount = dialogBuffer.totalAdded - lastDialogFlushed; + * + * The trackers must be declared with `let X = 0;` at module scope so the + * subtraction returns a real number on the first tick. If a tracker is + * referenced inside flushBuffers but never declared at module scope, the + * interval throws `ReferenceError: X is not defined` every second — the + * throw is swallowed by the catch at the bottom of flushBuffers (logged + * as `[browse] Buffer flush failed: is not defined`), the + * corresponding on-disk log file is *never written*, and the regression + * is silent in production. + * + * This source-level guard catches that exact class of regression — a + * future flush-perf refactor that adds a fourth buffer cursor (or a + * future contributor that copy-pastes the `last*Flushed` pattern without + * the matching declaration) will fail this test before it ships. + * + * Pattern matches `terminal-agent.test.ts` and `dual-listener.test.ts`: + * read source as text, assert an invariant, no daemon required. + */ + +import { describe, test, expect } from 'bun:test'; +import { readFileSync } from 'fs'; +import * as path from 'path'; + +const SERVER_TS = readFileSync( + path.resolve(import.meta.dir, '../src/server.ts'), + 'utf-8', +); + +describe('server.ts — flushBuffers tracker declarations', () => { + test('every `last*Flushed` tracker referenced inside flushBuffers is declared at module scope', () => { + // Locate the flushBuffers function body. The function is `async function + // flushBuffers() { ... }` — match through the closing brace at the start + // of a line (one-level-deep function in the file). + const fnMatch = SERVER_TS.match( + /async function flushBuffers\([^)]*\)[^{]*\{([\s\S]*?)\n\}/, + ); + expect(fnMatch, 'flushBuffers function not found in server.ts').not.toBeNull(); + const body = fnMatch![1]!; + + // Pull every identifier matching the `lastXxxFlushed` cursor pattern. + const trackerMatches = [...body.matchAll(/\blast([A-Z]\w+)Flushed\b/g)]; + const trackers = Array.from(new Set(trackerMatches.map((m) => `last${m[1]}Flushed`))); + + expect( + trackers.length, + 'flushBuffers should reference at least one last*Flushed tracker', + ).toBeGreaterThan(0); + + for (const tracker of trackers) { + // Module-level `let X = 0;` declaration (not inside a function body). + // Anchored start-of-line to avoid matching nested re-declarations or + // string literals. + const declared = new RegExp( + `(?:^|\\n)let\\s+${tracker}\\s*=\\s*0\\s*;`, + ).test(SERVER_TS); + expect( + declared, + `\`${tracker}\` is referenced inside flushBuffers but never declared at module scope ` + + `with \`let ${tracker} = 0;\` — the interval will throw ReferenceError every tick ` + + `and the corresponding on-disk log will never be written`, + ).toBe(true); + } + }); +});