From 4462f0caa43a6d17c8b28455783ba9c9f6314706 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 23 May 2026 23:08:01 -0700 Subject: [PATCH] refactor(terminal-agent): extract internalHandler helper for /internal/* routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the copy-pasted bearer-auth + X-Browse-Gen + req.json().then().catch() boilerplate on /internal/grant and /internal/revoke with a single internalHandler(req, fn) wrapper. Future /internal/* routes added by the v1.44 long-lived-sidebar work (/internal/lease-refresh, /internal/restart) land as one-liners using the same helper. Pure refactor; no behavior change. /internal/healthz stays on the bare checkInternalAuth gate because it's a GET with no JSON body to parse — the helper's body-parse path would 400 it. * browse/src/terminal-agent.ts — new internalHandler; /internal/grant + /internal/revoke routed through it. * browse/test/terminal-agent-internal-handler.test.ts — static-grep tripwire that fails CI if the helper goes away or either of the two refactored routes regresses to the old inline pattern. Co-Authored-By: Claude Opus 4.7 (1M context) --- browse/src/terminal-agent.ts | 58 ++++++++++++++----- .../terminal-agent-internal-handler.test.ts | 51 ++++++++++++++++ 2 files changed, 94 insertions(+), 15 deletions(-) create mode 100644 browse/test/terminal-agent-internal-handler.test.ts diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts index d1cae0fb2..1d54a1a63 100644 --- a/browse/src/terminal-agent.ts +++ b/browse/src/terminal-agent.ts @@ -214,10 +214,7 @@ function disposeSession(session: PtySession): void { * Validate a loopback /internal/* request. Returns null when the request * is allowed; otherwise returns the Response to send back. Centralizes * bearer auth + the v1.44 X-Browse-Gen generation check so adding a new - * /internal/* route is a one-liner. The full internalHandler wrapper - * arrives in Commit 1 alongside the new routes; this is the minimal - * shape needed to gate the existing /internal/grant + /internal/revoke - * without copy-pasting the gen check. + * /internal/* route is a one-liner. */ function checkInternalAuth(req: Request): Response | null { const auth = req.headers.get('authorization'); @@ -231,6 +228,42 @@ function checkInternalAuth(req: Request): Response | null { return null; } +/** + * Wrap a JSON-bodied /internal/* handler with the standard bearer-auth + + * generation-check + json-parse + error-response boilerplate. The handler + * `fn` is called with the parsed body; whatever it returns is JSON-stringified + * into a 200 Response, or the handler can return a Response directly to + * customize status / headers. Throwing from `fn` collapses to a 400 "bad". + * + * Centralizing the dance kills the copy-paste pattern of bearer + gen check + * + req.json().then(...).catch(...) that every /internal/* route needs. + * New routes become a single call to internalHandler. + */ +async function internalHandler( + req: Request, + fn: (body: any) => T | Promise | Response | Promise, +): Promise { + const denied = checkInternalAuth(req); + if (denied) return denied; + let body: any; + try { + body = await req.json(); + } catch { + return new Response('bad', { status: 400 }); + } + try { + const result = await fn(body); + if (result instanceof Response) return result; + if (result === undefined || result === null) return new Response('ok'); + return new Response(JSON.stringify(result), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } catch { + return new Response('bad', { status: 400 }); + } +} + function buildServer() { return Bun.serve({ hostname: '127.0.0.1', @@ -242,30 +275,25 @@ function buildServer() { // /internal/grant — loopback-only handshake from parent server. if (url.pathname === '/internal/grant' && req.method === 'POST') { - const denied = checkInternalAuth(req); - if (denied) return denied; - return req.json().then((body: any) => { + return internalHandler(req, (body) => { if (typeof body?.token === 'string' && body.token.length > 16) { validTokens.add(body.token); } - return new Response('ok'); - }).catch(() => new Response('bad', { status: 400 })); + }); } // /internal/revoke — drop a token (called on WS close or bootstrap reload) if (url.pathname === '/internal/revoke' && req.method === 'POST') { - const denied = checkInternalAuth(req); - if (denied) return denied; - return req.json().then((body: any) => { + return internalHandler(req, (body) => { if (typeof body?.token === 'string') validTokens.delete(body.token); - return new Response('ok'); - }).catch(() => new Response('bad', { status: 400 })); + }); } // /internal/healthz — liveness probe used by the v1.44 watchdog. // Returns this agent's pid + gen + active session count without // touching claude binary lookup (which can fail for non-process - // reasons and isn't a useful liveness signal). + // reasons and isn't a useful liveness signal). GET — no body to parse, + // so it stays on the bare checkInternalAuth gate. if (url.pathname === '/internal/healthz' && req.method === 'GET') { const denied = checkInternalAuth(req); if (denied) return denied; diff --git a/browse/test/terminal-agent-internal-handler.test.ts b/browse/test/terminal-agent-internal-handler.test.ts new file mode 100644 index 000000000..04e7f3597 --- /dev/null +++ b/browse/test/terminal-agent-internal-handler.test.ts @@ -0,0 +1,51 @@ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +// Static-grep tripwire for the v1.44 internalHandler refactor. +// +// /internal/grant and /internal/revoke were copies of the same dance: +// bearer-auth → x-browse-gen check → req.json().then(...).catch(...). +// internalHandler(req, fn) collapses that into a single helper call. +// This test fails CI if the helper goes away or the existing routes +// regress to inline auth + JSON parse boilerplate. Wiring tests +// (token grant/revoke behavior) already live in +// browse/test/terminal-agent-integration.test.ts. + +const AGENT_TS = path.resolve(new URL(import.meta.url).pathname, '..', '..', 'src', 'terminal-agent.ts'); + +describe('terminal-agent internalHandler refactor (v1.44+)', () => { + test('1. internalHandler exists with the documented signature', () => { + const src = fs.readFileSync(AGENT_TS, 'utf-8'); + expect(src).toMatch(/async function internalHandler\s*\(/); + // Body must include the auth gate, body parse, and result coercion. + expect(src).toContain('checkInternalAuth(req)'); + expect(src).toContain('await req.json()'); + expect(src).toContain('instanceof Response'); + }); + + test('2. /internal/grant routes through internalHandler', () => { + const src = fs.readFileSync(AGENT_TS, 'utf-8'); + // Match the route handler block. + const block = sliceBetween(src, "url.pathname === '/internal/grant'", "url.pathname === '/internal/revoke'"); + expect(block).toContain('internalHandler(req'); + // Must NOT have the old inline pattern (would be a regression). + expect(block).not.toContain('req.headers.get(\'authorization\')'); + expect(block).not.toContain('req.json().then('); + }); + + test('3. /internal/revoke routes through internalHandler', () => { + const src = fs.readFileSync(AGENT_TS, 'utf-8'); + const block = sliceBetween(src, "url.pathname === '/internal/revoke'", "url.pathname === '/internal/healthz'"); + expect(block).toContain('internalHandler(req'); + expect(block).not.toContain('req.json().then('); + }); +}); + +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); +}