Make the browse tunnel auto-start share the start lock (GST-2117) (#4212)

* Make the BROWSE_TUNNEL=1 auto-start share the browse tunnel start lock (GST-2117)

#4210 made ctx.tunnel.start single-flight, but the BROWSE_TUNNEL=1
auto-start in start() called startTunnel() directly, so a /tunnel/start
that overlapped it still opened a second ngrok session closeTunnel could
not reach. The lock now lives in startTunnel itself, which both callers
use. The unreleased changelog fragment names both paths.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Split the browse tunnel changelog sentence at its colon (GST-2117)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Restore the landed tunnel changelog fragment and add this fix's own (GST-2117)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

browse/src/server.ts

@@ -401,8 +401,14 @@ type StartTunnelResult =
  *
  * On success, sets the module tunnel state (tunnelListener / tunnelUrl /
  * tunnelServer / tunnelActive) and records the tunnel in the state file.
+ * Concurrent calls from either caller share the one in-flight start.
  */
-async function startTunnel(opts: {
+async function startTunnel(opts: Parameters<typeof openTunnel>[0]): Promise<StartTunnelResult> {
+  tunnelStarting ??= openTunnel(opts).finally(() => { tunnelStarting = null; });
+  return tunnelStarting;
+}
+
+async function openTunnel(opts: {
   fetchHandler: (req: Request, server: any) => Promise<Response>;
   authtoken: string;
   consent: string;
@@ -1788,11 +1794,11 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
       state: () => ({ active: tunnelActive, url: tunnelUrl, hasListener: tunnelServer !== null }),
       close: closeTunnel,
       resolveAuthtoken: resolveNgrokAuthtoken,
-      start: (authtoken) => (tunnelStarting ??= startTunnel({
+      start: (authtoken) => startTunnel({
         fetchHandler: makeFetchHandler('tunnel'),
         authtoken,
         consent: 'pair_agent=on (isPairAgentEnabled gate at /tunnel/start)',
-      }).finally(() => { tunnelStarting = null; })),
+      }),
     },
     commands: {
       handle: handleCommand,

browse/test/dual-listener.test.ts

@@ -332,7 +332,7 @@ describe('Tunnel listener lifecycle', () => {
     // The route calls ctx.tunnel.start (server-auth.test.ts pins that call);
     // the factory wires it to the shared startTunnel() helper with the
     // factory-scoped tunnel-surface handler.
-    const startBlock = sliceBetween(SERVER_SRC, 'start: (authtoken) => (tunnelStarting ??= startTunnel({', 'commands: {');
+    const startBlock = sliceBetween(SERVER_SRC, 'start: (authtoken) => startTunnel({', 'commands: {');
     expect(startBlock).toContain("makeFetchHandler('tunnel')");
     // The helper owns the ephemeral bind and points ngrok at the TUNNEL
     // port — never the local daemon port.

browse/test/tunnel-start-state.test.ts

@@ -1,6 +1,7 @@
 /**
  * POST /tunnel/start state under a mocked ngrok: two concurrent starts share
- * one session, and a start that fails after ngrok opens leaves the daemon
+ * one session, a route start that overlaps the BROWSE_TUNNEL=1 auto-start
+ * shares it too, and a start that fails after ngrok opens leaves the daemon
  * idle-stoppable. Each probe runs in a child `bun test` so the ngrok module
  * mock and the module-level tunnel state cannot leak into other shard files.
  */
@@ -27,8 +28,9 @@ mock.module(${JSON.stringify(Bun.resolveSync('@ngrok/ngrok', SRC))}, () => ({
     return { url: () => 'https://' + id + '.invalid', close: async () => { closed.push(id); } };
   },
 }));
-const { buildFetchHandler, __testInternals__ } = await import(${JSON.stringify(path.join(SRC, 'server.ts'))});
+const { buildFetchHandler, start: startDaemon, __testInternals__ } = await import(${JSON.stringify(path.join(SRC, 'server.ts'))});
 const { resolveConfig } = await import(${JSON.stringify(path.join(SRC, 'config.ts'))});
+const { __resetRegistry } = await import(${JSON.stringify(path.join(SRC, 'token-registry.ts'))});
 const stateFile = process.env.BROWSE_STATE_FILE!;
 const token = 'tunnel-start-token-0123456789';
 const handle = buildFetchHandler({
@@ -38,7 +40,7 @@ const handle = buildFetchHandler({
 const start = () => handle.fetchLocal(new Request('http://127.0.0.1:34998/tunnel/start', { method: 'POST', headers: { Authorization: 'Bearer ' + token } }), null);
 `;
 
-async function runProbe(body: string): Promise<{ code: number; tail: string }> {
+async function runProbe(body: string, env: Record<string, string> = {}): Promise<{ code: number; tail: string }> {
   const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'tunnel-start-'));
   fs.writeFileSync(path.join(dir, 'probe.test.ts'), PRELUDE + body);
   try {
@@ -50,9 +52,11 @@ async function runProbe(body: string): Promise<{ code: number; tail: string }> {
         ...process.env,
         BROWSE_STATE_FILE: path.join(dir, 'state', 'browse.json'),
         GSTACK_HOME: path.join(dir, 'gstack'),
+        CHROMIUM_PROFILE: path.join(dir, 'profile'),
         GSTACK_PAIR_AGENT: 'on',
         NGROK_AUTHTOKEN: 'fake-token-for-test',
         NGROK_DOMAIN: '',
+        ...env,
       },
     });
     const timer = setTimeout(() => child.kill(), 60_000);
@@ -92,6 +96,37 @@ test('probe', async () => {
   expect(result.tail).toContain('1 pass');
 }, 90_000);
 
+test('a route start that overlaps the BROWSE_TUNNEL=1 auto-start shares its ngrok session', async () => {
+  const result = await runProbe(`
+test('probe', async () => {
+  (process as any).exit = () => {};
+  let release!: () => void;
+  forwardGate = new Promise<void>(r => { release = r; });
+  __resetRegistry();
+  let booting = true;
+  const booted = startDaemon().finally(() => { booting = false; });
+  for (let i = 0; i < 900 && booting && opened.length === 0; i++) await new Promise(r => setTimeout(r, 50));
+  try {
+    expect(opened).toEqual(['session-1']);
+    const state = JSON.parse(fs.readFileSync(stateFile, 'utf-8'));
+    const routed = fetch('http://127.0.0.1:' + state.port + '/tunnel/start', { method: 'POST', headers: { Authorization: 'Bearer ' + state.token } });
+    await new Promise(r => setTimeout(r, 100));
+    release();
+    const [resp] = await Promise.all([routed, booted]);
+    expect(opened).toEqual(['session-1']);
+    expect(closed).toEqual([]);
+    expect(resp.status).toBe(200);
+    expect((await resp.json()).url).toBe('https://session-1.invalid');
+  } finally {
+    release();
+    await __testInternals__.browserManager.close();
+  }
+}, 60_000);
+`, { BROWSE_TUNNEL: '1', BROWSE_PARENT_PID: '0' });
+  expect(result).toMatchObject({ code: 0 });
+  expect(result.tail).toContain('1 pass');
+}, 90_000);
+
 test('a tunnel start that fails after ngrok opens leaves the daemon idle-stoppable', async () => {
   const result = await runProbe(`
 test('probe', async () => {

changelog.d/fix-browse-tunnel-auto-start-shares-start-lock.md

@@ -0,0 +1,4 @@
+---
+important: security
+---
+- **A daemon launched with `BROWSE_TUNNEL=1` no longer opens a second ngrok tunnel when `/tunnel/start` arrives during its own start.** The earlier fix made overlapping `/tunnel/start` calls share one start, but the daemon's own startup tunnel skipped that lock. A `/tunnel/start` that arrived while the startup tunnel was still opening saw the tunnel off and opened its own ngrok session, and the daemon kept only one of the two. The other stayed public on your ngrok account until the daemon exited, and closing the tunnel didn't reach it. Now every tunnel start, from the route or from startup, goes through one shared lock and gets the same URL, so there is only ever one session.