mirror of https://github.com/garrytan/gstack.git
refactor(terminal-agent): extract internalHandler<T> helper for /internal/* routes
Replaces the copy-pasted bearer-auth + X-Browse-Gen + req.json().then().catch()
boilerplate on /internal/grant and /internal/revoke with a single
internalHandler<T>(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<T>; /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) <noreply@anthropic.com>
This commit is contained in:
parent
2a55953387
commit
4462f0caa4
|
|
@ -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<T> 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<T>(
|
||||
req: Request,
|
||||
fn: (body: any) => T | Promise<T> | Response | Promise<Response>,
|
||||
): Promise<Response> {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -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<T>(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<T> exists with the documented signature', () => {
|
||||
const src = fs.readFileSync(AGENT_TS, 'utf-8');
|
||||
expect(src).toMatch(/async function internalHandler<T>\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);
|
||||
}
|
||||
Loading…
Reference in New Issue