fix(config): give the render fixture's server and scratch tree back when it cannot start

CodeRabbit on the render fixture, plus its note on `release`.

The fixture. `chromium.launch` is the last step of the setup and the one that
fails in practice — no Chromium on the machine, an
`ORCA_MOBILE_WEB_RENDER_BROWSER` pointing nowhere — and by then the bundle
server is listening and the scratch tree is on disk. Rejecting there left the
caller without a handle, so `afterAll` had nothing to close and both stayed
allocated; the listening socket is the one that bites, because an open server
handle keeps the vitest worker alive after its last test has reported. The setup
after `mkdtemp` is now wrapped, gives back whatever it managed to take, and
rethrows the original error rather than anything the cleanup raised. The normal
close path awaits the server-close callback instead of firing it.

`release` in the page mount. The ownership check covered the claim but not the
two lines that make the terminal disappear, so a release that skipped the claim
would still empty the host and drop its class. The check now guards the whole
function, and round 5's caller-side check is gone as a duplicate of it: one rule,
inside the thing it governs. Both existing callers are unchanged in behaviour —
the synchronous planting catch always owns the page, and the rejection handler
was already guarded.

Pinned red first. The new case points the launch at an executable that is not
there, then asks the port the fixture actually served on for a connection and
reads the scratch directories in the temp dir. Without the rollback the port
still accepts and the scratch tree is still there; with it, neither. The port is
recorded by wrapping the real `createBundleServer` rather than standing a double
in front of it, and the case asserts a server was created at all, or the refusal
would mean nothing.

Two oracles were discarded on the way. `rejects.toThrow()` with no argument
passes for a build that broke for its own reason, so the rejection is matched by
message. `process.getActiveResourcesInfo()` reports `TCPServerWrap`, not
`TCPSERVERWRAP`, so a count filtered on the upper-case spelling was zero in both
arms and agreed with everything; it also still lists the handle at the moment
the close callback runs.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

config/scripts/mobile-web-app-terminal-render-fixture.mjs

@@ -47,37 +47,69 @@ const SHELL_HOST = {
   lastConnected: 1
 }
 
-/** Builds the bundle, serves it under the shell's own policy, and launches the browser. */
+/**
+ * Everything the fixture allocated, in the reverse of the order it took it.
+ *
+ * Shared by the normal close and the rollback, because a setup that fell over halfway has exactly
+ * the same things to give back as one that ran to the end — it just has fewer of them. The server
+ * close is awaited rather than fired: it holds a listening socket, and a socket still open when
+ * the file finishes keeps the vitest worker alive after its last test has reported.
+ */
+async function closeTerminalRenderFixture({ browser, scratch, server }) {
+  await browser?.close()
+  if (server) {
+    await new Promise((resolve) => server.close(resolve))
+  }
+  await rm(scratch, { recursive: true, force: true })
+}
+
+/**
+ * Builds the bundle, serves it under the shell's own policy, and launches the browser.
+ *
+ * Nothing survives a setup that throws. The browser is launched last and is the step most likely
+ * to fail — no Chromium on the machine, an `ORCA_MOBILE_WEB_RENDER_BROWSER` that points nowhere —
+ * and by then the server is listening and the scratch tree is on disk. A caller that never got a
+ * handle back has nothing to close, so this closes them itself and rethrows what actually went
+ * wrong rather than whatever the cleanup might say.
+ */
 export async function startTerminalRenderFixture() {
   const cspHeader = await readShellCsp()
   const bridgeVersion = await readBridgeProtocolVersion()
   const faultGrant = await readBridgeFaultGrant()
   const scratch = await mkdtemp(join(tmpdir(), 'orca-c75-terminal-render-'))
-  const appDir = join(scratch, 'app')
-  const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT)
-  await mkdir(routeDir, { recursive: true })
-  await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE)
-  // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a real
-  // route. Naming the `.tsx` would mount the WebView wrapper no browser can render.
-  await writeFile(
-    join(routeDir, 'terminal-probe.tsx'),
-    probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView'))
-  )
-  await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE)
-  const built = await buildMobileWebAppBundle({
-    appDir,
-    outDir: join(scratch, 'bundle'),
-    pageRoutes: [
-      { pathname: PROBE_ROUTE, grants: [] },
-      { pathname: CONTROL_ROUTE, grants: [] }
-    ]
-  })
-  const { origin, server } = await createBundleServer({ outDir: built.outDir, cspHeader })
-  const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER
-  const browser = await chromium.launch({
-    headless: true,
-    ...(executablePath ? { executablePath } : {})
-  })
+  let browser = null
+  let served = null
+  try {
+    const appDir = join(scratch, 'app')
+    const routeDir = join(appDir, MOBILE_WEB_APP_ROUTE_ROOT)
+    await mkdir(routeDir, { recursive: true })
+    await writeFile(join(routeDir, '_layout.tsx'), LAYOUT_SOURCE)
+    // Extensionless, so the bundler resolves the `.web.tsx` sibling exactly as it would for a
+    // real route. Naming the `.tsx` would mount the WebView wrapper no browser can render.
+    await writeFile(
+      join(routeDir, 'terminal-probe.tsx'),
+      probeRouteSource(join(mobileDir, 'src', 'terminal', 'TerminalWebView'))
+    )
+    await writeFile(join(routeDir, 'terminal-control.tsx'), CONTROL_SOURCE)
+    const built = await buildMobileWebAppBundle({
+      appDir,
+      outDir: join(scratch, 'bundle'),
+      pageRoutes: [
+        { pathname: PROBE_ROUTE, grants: [] },
+        { pathname: CONTROL_ROUTE, grants: [] }
+      ]
+    })
+    served = await createBundleServer({ outDir: built.outDir, cspHeader })
+    const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER
+    browser = await chromium.launch({
+      headless: true,
+      ...(executablePath ? { executablePath } : {})
+    })
+  } catch (error) {
+    await closeTerminalRenderFixture({ browser, scratch, server: served?.server })
+    throw error
+  }
+  const { origin } = served
 
   async function openPage(
     pathname,
@@ -134,11 +166,7 @@ export async function startTerminalRenderFixture() {
   return {
     openPage,
     openTerminal,
-    close: async () => {
-      await browser.close()
-      server.close()
-      await rm(scratch, { recursive: true, force: true })
-    }
+    close: () => closeTerminalRenderFixture({ browser, scratch, server: served.server })
   }
 }
 

config/scripts/mobile-web-app-terminal-render-fixture.test.mjs

@@ -0,0 +1,85 @@
+import { readdir } from 'node:fs/promises'
+import { connect } from 'node:net'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { describe, expect, it, vi } from 'vitest'
+import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs'
+
+/**
+ * What the render fixture gives back when it never finishes starting.
+ *
+ * The handle is the only way to close it, so a setup that throws before returning one leaves the
+ * caller nothing to call: `afterAll` has no fixture, and the listening socket and the scratch tree
+ * stay where they are. The socket is the part that bites — an open server handle keeps the vitest
+ * worker alive after its last test has reported, so the file hangs rather than failing.
+ *
+ * The browser is the step that fails in practice and the last one taken, so by then everything
+ * else is allocated. It is made to fail the way it actually does, by pointing the launch at an
+ * executable that is not there, rather than by standing a double in front of Playwright.
+ */
+
+const SCRATCH_PREFIX = 'orca-c75-terminal-render-'
+const describeFixture = mobileWebAppDependenciesPresent() ? describe : describe.skip
+
+// The port the fixture served on, which it never hands out and which `close` makes unreachable.
+// Recorded through the real server rather than a double: what is under test is whether the thing
+// that was actually listening stopped.
+const { ports } = vi.hoisted(() => ({ ports: [] }))
+vi.mock('./mobile-web-app-render-harness.mjs', async (importOriginal) => {
+  const harness = await importOriginal()
+  return {
+    ...harness,
+    createBundleServer: async (options) => {
+      const served = await harness.createBundleServer(options)
+      ports.push(served.server.address().port)
+      return served
+    }
+  }
+})
+
+const { startTerminalRenderFixture } = await import('./mobile-web-app-terminal-render-fixture.mjs')
+
+/** Whether a connection to this port is refused, which is what a closed listener answers. */
+function refusesConnections(port) {
+  return new Promise((resolve) => {
+    const socket = connect({ host: '127.0.0.1', port })
+    socket.on('connect', () => {
+      socket.destroy()
+      resolve(false)
+    })
+    socket.on('error', () => resolve(true))
+  })
+}
+
+async function scratchDirectories() {
+  const entries = await readdir(tmpdir())
+  return entries.filter((entry) => entry.startsWith(SCRATCH_PREFIX)).sort()
+}
+
+describeFixture('the terminal render fixture', () => {
+  it('takes back the server and the scratch tree when the browser will not start', async () => {
+    const scratchBefore = await scratchDirectories()
+    const realBrowser = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER
+    process.env.ORCA_MOBILE_WEB_RENDER_BROWSER = join(tmpdir(), 'orca-c75-no-such-browser')
+    try {
+      // Named, not merely thrown: a build that broke for its own reason would also reject, and
+      // would satisfy a bare `toThrow` while saying nothing about the rollback under test. It
+      // also has to be the original error and not whatever the cleanup raised on its way out.
+      await expect(startTerminalRenderFixture()).rejects.toThrow(
+        /Failed to launch chromium because executable doesn't exist/
+      )
+    } finally {
+      if (realBrowser === undefined) {
+        delete process.env.ORCA_MOBILE_WEB_RENDER_BROWSER
+      } else {
+        process.env.ORCA_MOBILE_WEB_RENDER_BROWSER = realBrowser
+      }
+    }
+
+    // The precondition: the launch has to have been reached with a server already listening, or
+    // there is nothing for the rollback to have released and the refusal below means nothing.
+    expect(ports, 'the setup got as far as serving the bundle').toHaveLength(1)
+    expect(await refusesConnections(ports[0])).toBe(true)
+    expect(await scratchDirectories()).toEqual(scratchBefore)
+  }, 600_000)
+})

mobile/src/terminal/terminal-web-document-mount.ts

@@ -137,10 +137,19 @@ export function mountTerminalWebDocument(
   const token = Symbol('orca terminal document')
   liveDocument = token
   let started: StartedDocument | null = null
+  /**
+   * Gives the page back, and only if it is still this mount's to give.
+   *
+   * The check covers the element too, not just the claim. Emptying a host and taking its class
+   * off are what make the terminal disappear, so a release that skipped the claim but did those
+   * anyway would blank the terminal a later mount has on the screen. One rule, inside the thing
+   * it governs, rather than at each caller.
+   */
   const release = () => {
-    if (liveDocument === token) {
-      liveDocument = null
+    if (liveDocument !== token) {
+      return
     }
+    liveDocument = null
     host.innerHTML = ''
     // The sheet stays in the head; the class does not, so every rule in it matches nothing
     // again the moment the terminal is gone.
@@ -171,12 +180,9 @@ export function mountTerminalWebDocument(
     },
     (error: unknown) => {
       // The import failed, so nothing was started and the page has to go back — the overlay's
-      // Reload is a second mount and it must be allowed to make one. Only if the page is still
-      // this mount's: a later mount may already hold it, and emptying its host would take the
-      // terminal that is on the screen.
-      if (liveDocument === token) {
-        release()
-      }
+      // Reload is a second mount and it must be allowed to make one. A later mount may already
+      // hold the page, which `release` answers for.
+      release()
       throw error
     }
   )