diff --git a/browse/test/sidepanel-patient-autoconnect.test.ts b/browse/test/sidepanel-patient-autoconnect.test.ts new file mode 100644 index 000000000..faf38499b --- /dev/null +++ b/browse/test/sidepanel-patient-autoconnect.test.ts @@ -0,0 +1,70 @@ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +// v1.44 patient autoConnect — static-grep invariants for the polling loop. +// +// Pre-v1.44 the sidebar gave up at 15s with "Browse server not ready. +// Reload sidebar to retry." Cold-start the browse server takes ~3-8s on a +// healthy laptop, longer on Conductor workspaces / slow CI, so the user +// frequently saw the failure message even when nothing was wrong. The +// fix: poll forever with ascending status messages and only abort on +// explicit unrecoverable signals (401 auth invalid). + +const CLIENT_JS = path.resolve( + new URL(import.meta.url).pathname, + '..', + '..', + '..', + 'extension', + 'sidepanel-terminal.js', +); + +describe('sidepanel tryAutoConnect patience (v1.44+)', () => { + test('1. no 15s give-up message', () => { + const src = fs.readFileSync(CLIENT_JS, 'utf-8'); + // The v0.x give-up string must NOT reappear — it's the message users + // saw on every cold start and the whole point of v1.44 was to delete it. + expect(src).not.toContain('Browse server not ready. Reload sidebar to retry.'); + }); + + test('2. ascending status messages at 15s / 60s / 5min', () => { + const src = fs.readFileSync(CLIENT_JS, 'utf-8'); + expect(src).toContain('Waiting for browse server...'); + expect(src).toContain('Still waiting'); + expect(src).toContain('still not responding after 5 min'); + }); + + test('3. sticky abort flag prevents loop spam on 401', () => { + const src = fs.readFileSync(CLIENT_JS, 'utf-8'); + expect(src).toContain('autoConnectAborted'); + // The mint failure branch must short-circuit on 401 specifically. + expect(src).toMatch(/minted\.error.*startsWith\('401'\)/); + // tryAutoConnect tick must respect the flag. + expect(src).toMatch(/if \(autoConnectAborted\) return/); + }); + + test('4. forceRestart re-arms the loop by clearing the abort flag', () => { + const src = fs.readFileSync(CLIENT_JS, 'utf-8'); + // forceRestart is the user's "try again" escape hatch — must reset + // the sticky flag or 401-once means stuck-forever. + const block = sliceBetween(src, 'function forceRestart', 'function repaintIfLive'); + expect(block).toContain('autoConnectAborted = false'); + }); + + test('5. poll interval is 2s, not the legacy 200ms tight loop', () => { + const src = fs.readFileSync(CLIENT_JS, 'utf-8'); + // 200ms ticks burned CPU and made the give-up window land too fast. + // 2s is the v1.44 cadence — verify the tight-loop literal is gone. + expect(src).toContain('setTimeout(tick, 2000)'); + expect(src).not.toContain('setTimeout(tick, 200)'); + }); +}); + +function sliceBetween(source: string, start: string, end: string): string { + const i = source.indexOf(start); + if (i === -1) throw new Error(`marker not found: ${start}`); + const j = source.indexOf(end, i + start.length); + if (j === -1) throw new Error(`end marker not found: ${end}`); + return source.slice(i, j); +} diff --git a/extension/sidepanel-terminal.js b/extension/sidepanel-terminal.js index 5e5835d5e..88c3dbb10 100644 --- a/extension/sidepanel-terminal.js +++ b/extension/sidepanel-terminal.js @@ -48,6 +48,15 @@ let term = null; let fitAddon = null; let ws = null; + /** + * Sticky abort flag for tryAutoConnect's polling loop. Set when we + * receive an unrecoverable signal (auth invalid → 401, claude not + * found, fatal server error) so the loop doesn't keep retrying and + * spamming the user with the same failure message every 2s. Cleared + * by forceRestart() so the user can re-enter the polling loop after + * fixing whatever was wrong. + */ + let autoConnectAborted = false; /** * 25s client-side WS keepalive interval (v1.44+). Belt-and-suspenders with * the server-side ping in terminal-agent.ts: server pings cover most @@ -325,6 +334,16 @@ const minted = await mintSession(); if (minted.error) { + // 401 = stale auth token; no amount of retrying will fix it. Sticky + // abort the polling loop so we don't spam the same error every 2s + // until the user clicks Restart (which clears the flag). + if (typeof minted.error === 'string' && minted.error.startsWith('401')) { + autoConnectAborted = true; + setState(STATE.IDLE, { + message: 'Auth invalid — reload the sidebar or restart your gstack session.', + }); + return; + } setState(STATE.IDLE, { message: `Cannot start: ${minted.error}` }); return; } @@ -464,6 +483,9 @@ term = null; fitAddon = null; } + // User explicitly asked for a fresh start; re-arm the auto-connect + // polling loop in case a prior auth failure stuck the abort flag. + autoConnectAborted = false; setState(STATE.IDLE, { message: 'Starting Claude Code...' }); tryAutoConnect(); } @@ -542,23 +564,47 @@ * as /health succeeds), then fires connect() automatically. The user * doesn't have to press a key — Terminal is the default tab and "tap to * start" was a needless paper cut on every reload. + * + * v1.44 patience overhaul: no more 15s give-up. The user already opened + * the sidebar; giving up tells them "you did something wrong" when the + * truth is the daemon is slow to boot (or restarting via the upstream + * supervisor). We poll forever at 2s intervals with ascending status + * messages so the user knows we're still trying, and ONLY abort on + * explicit signals: state transition out of IDLE (connect succeeded + * or user navigated), or an unrecoverable auth/network signal. */ function tryAutoConnect() { if (state !== STATE.IDLE) return; - let waited = 0; + if (autoConnectAborted) return; + const startedAt = Date.now(); const tick = () => { // If the user navigated away (Chat tab) or already connected, drop out. if (state !== STATE.IDLE) return; + // If a prior attempt hit an unrecoverable error (401, etc.), stop + // polling. The user clears the flag by clicking Restart. + if (autoConnectAborted) return; if (getServerPort() && getAuthToken()) { connect(); return; } - waited += 200; - if (waited > 15000) { - setState(STATE.IDLE, { message: 'Browse server not ready. Reload sidebar to retry.' }); - return; + // Ascending status messages — the user wants to know the sidebar is + // still trying. Each threshold is the moment the message would + // mislead if left silent: at 15s "should have started by now," at + // 60s "the server might be in trouble," at 5min "stop waiting and + // check on it manually." + const elapsed = Date.now() - startedAt; + if (elapsed > 300_000) { + setState(STATE.IDLE, { + message: 'Browse server still not responding after 5 min. Try `$B status` in a terminal.', + }); + } else if (elapsed > 60_000) { + setState(STATE.IDLE, { + message: 'Still waiting — browse server may be slow to start.', + }); + } else if (elapsed > 15_000) { + setState(STATE.IDLE, { message: 'Waiting for browse server...' }); } - setTimeout(tick, 200); + setTimeout(tick, 2000); }; tick(); }