fix(e2e): compile every timed route first in the HTTP suites and stop sessions completely (#8701)

* fix(e2e): compile every timed route first in the HTTP suites and stop sessions completely

- version compare: a first check compiles the deployment, version, compare and MCP
  routes under a 300s budget; later requests stay at 60s and a timeout names the route
- desktop inbox: the warm-up also reads the inbox, so the 10s executor window covers
  no compile; stopping the executor waits for its in-flight pull, so teardown never
  runs under a claim and a timeout is reported as itself instead of a later 401
- stop-session.sh signals every process in the session, including one in its own
  process group, escalates to SIGKILL 10s after SIGTERM, and reports processes that
  outlived the leader only once the leader has exited
- stop-after: a CLI that overflows its output buffer is reported as an overflow, not
  as a timeout

* fix(ci): run the embedded CLI app with the same isolated lifecycle as the other HTTP end-to-end apps

Author: waleedlatif1

.github/scripts/stop-session.sh

@@ -5,39 +5,58 @@
 #
 # `next dev` runs its server and workers as child processes. Waiting on `next dev` alone returns
 # while those can still be running, and the next app in the job reuses the same .next directory.
+# Every member of the session is signalled, including one that moved to its own process group.
 # The leader stays a zombie until the shell that started it waits on it, so zombies don't count.
 set -u
 
 leader=$1
+grace_seconds=10
+
+session_pids() {
+  ps -s "$leader" -o pid=,stat= 2>/dev/null | awk '$2 !~ /^Z/ { print $1 }'
+}
+
+signal_session() {
+  local pids
+  pids=$(session_pids)
+  [ -z "$pids" ] || kill "-$1" $pids 2>/dev/null || true
+}
 
 running() {
-  ps -s "$leader" -o pid=,stat= 2>/dev/null | awk '$2 !~ /^Z/ { found = 1 } END { exit !found }'
+  [ -n "$(session_pids)" ]
 }
 
 leader_running() {
   ps -p "$leader" -o stat= 2>/dev/null | grep -qv '^Z'
 }
 
-wait_for() {
-  for _ in $(seq 1 100); do
+now_us() {
+  echo "${EPOCHREALTIME/./}"
+}
+
+# Waits while the given check holds, until the shared deadline. Succeeds once it stops holding.
+wait_while() {
+  while (($(now_us) < deadline)); do
     "$@" || return 0
     sleep 0.1
   done
-  return 1
+  ! "$@"
 }
 
-kill -TERM -- "-$leader" 2>/dev/null || true
-wait_for leader_running
-if running; then
+deadline=$(($(now_us) + grace_seconds * 1000000))
+signal_session TERM
+wait_while leader_running
+if ! leader_running && running; then
   echo "Processes from session $leader outlived its leader:"
   ps -s "$leader" -o pid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
 fi
-wait_for running && exit 0
+wait_while running && exit 0
 
-echo "::warning::Processes from session $leader were still running 10s after SIGTERM:"
-ps -s "$leader" -o pid,stat,etimes,args 2>/dev/null || true
-kill -KILL -- "-$leader" 2>/dev/null || true
-wait_for running && exit 0
+echo "::warning::Processes from session $leader were still running ${grace_seconds}s after SIGTERM:"
+ps -s "$leader" -o pid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
+deadline=$(($(now_us) + grace_seconds * 1000000))
+signal_session KILL
+wait_while running && exit 0
 
-echo "::error::Processes from session $leader survived SIGKILL."
+echo "::error::Processes from session $leader survived SIGKILL for ${grace_seconds}s."
 exit 1

.github/workflows/test-build.yml

@@ -206,6 +206,13 @@ jobs:
       # immediately, and either way the server log tail lands in the job log. No
       # step timeout: the job's bound covers a hang without cutting a slow but
       # healthy suite short of writing its report.
+      #
+      # Every `next dev` app in this job shares apps/sim/.next, so each one follows the
+      # same lifecycle. It starts from an empty Turbopack dev cache: a cache written under
+      # other NEXT_PUBLIC_* values, by a server that `next dev` SIGKILLs 100ms after
+      # SIGTERM, can panic Turbopack or wedge a route compile on restore. It runs in its
+      # own session, and stop-session.sh returns only once all of it has exited, so no app
+      # starts beside one still writing .next/dev.
       - name: Verify SCIM, administration and workflow comparisons over real HTTP
         working-directory: apps/sim
         env:
@@ -285,16 +292,21 @@ jobs:
           report_dir="$RUNNER_TEMP/e2e"
           server_log="$report_dir/cli-next.log"
           mkdir -p "$report_dir"
-          node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
+          rm -rf .next/dev
+          setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
           server_pid=$!
           finish() {
-            kill "$server_pid" 2>/dev/null || true
+            status=$?
+            bash ../../.github/scripts/stop-session.sh "$server_pid" || status=1
             wait "$server_pid" 2>/dev/null || true
+            if [ "$status" -ne 0 ]; then
+              tail -n 200 "$server_log"
+              exit "$status"
+            fi
           }
           trap finish EXIT
           fail_startup() {
             echo "::error::$1"
-            tail -n 200 "$server_log"
             exit 1
           }
           started=$SECONDS
@@ -315,12 +327,6 @@ jobs:
       # A self-hosted app: hosted billing admits a run only through a Redis usage
       # reservation, and the SCIM suite above asserts PostgreSQL rate-limit storage,
       # so workflow execution gets its own app rather than adding Redis to that one.
-      #
-      # The apps in this job share apps/sim/.next. Each starts from an empty Turbopack
-      # dev cache: one written under other NEXT_PUBLIC_* values, by a server `next dev`
-      # SIGKILLs 100ms after SIGTERM, can panic Turbopack or wedge a route compile on
-      # restore. Each runs in its own session, and stop-session.sh returns only once
-      # all of it has exited, so no app starts beside one still writing .next/dev.
       - name: Verify single-block workflow runs over real HTTP
         working-directory: apps/sim
         env:

apps/sim/scripts/test-desktop-inbox-e2e.ts

@@ -284,20 +284,16 @@ function openDoorbell(desktop: Desktop) {
 function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbell>) {
   const ran = new Map<string, { toolName: string; chatId: string; executionToken: string }>()
   const completed = new Set<string>()
-  let running = false
+  let inFlight: Promise<void> | null = null
   let rerun = false
   let stopped = false
-  const pull = async () => {
-    if (running) {
-      rerun = true
-      return
-    }
-    running = true
+  const drain = async () => {
     try {
       do {
         rerun = false
         const inbox = await pullInbox(desktop)
         for (const item of inbox.items) {
+          if (stopped) return
           if (item.kind !== 'call' || ran.has(item.toolCallId)) continue
           const claimBody: ClaimDesktopToolBody = {
             deviceId: desktop.deviceId,
@@ -351,8 +347,16 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
         }
       } while (rerun && !stopped)
     } finally {
-      running = false
+      inFlight = null
+    }
+  }
+  const pull = () => {
+    if (inFlight) {
+      rerun = true
+      return inFlight
     }
+    inFlight = drain()
+    return inFlight
   }
   const offDoorbell = doorbell.onEvent(
     () => void pull().catch((error) => logger.error('pull failed', error))
@@ -361,14 +365,19 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
     () => void pull().catch((error) => logger.error('reconcile failed', error)),
     RECONCILE_MS
   )
-  void pull()
+  void pull().catch((error) => logger.error('pull failed', error))
   return {
     ran,
     completed,
-    stop() {
+    /**
+     * Resolves once the pull in flight has settled, so the fixture is never torn down under a
+     * request the executor already started.
+     */
+    async stop() {
       stopped = true
       offDoorbell()
       clearInterval(timer)
+      await inFlight?.catch(() => {})
     },
   }
 }
@@ -414,15 +423,19 @@ async function run() {
   })
 
   /** `next dev` compiles a route on its first request, which must not count against the timed checks. */
-  await check('refuses malformed claim, lease and completion bodies', async () => {
-    for (const path of [
-      '/api/desktop/tool/claim',
-      '/api/desktop/tool/lease',
-      '/api/desktop/tool/complete',
-    ]) {
-      await request(desktop, 'POST', path, { body: {}, expected: 400 })
+  await check(
+    'reads the inbox and refuses malformed claim, lease and completion bodies',
+    async () => {
+      await pullInbox(desktop)
+      for (const path of [
+        '/api/desktop/tool/claim',
+        '/api/desktop/tool/lease',
+        '/api/desktop/tool/complete',
+      ]) {
+        await request(desktop, 'POST', path, { body: {}, expected: 400 })
+      }
     }
-  })
+  )
 
   /** Starts absent, so only the stream open below can mark the device present. */
   await redis.del(`desktop:presence:${desktop.deviceId}`)
@@ -544,7 +557,7 @@ async function run() {
       })
     })
   } finally {
-    executor.stop()
+    await executor.stop()
   }
 
   await check('tells the device to cancel a call whose chat was stopped', async () => {

apps/sim/scripts/test-workflow-stop-after-e2e.ts

@@ -324,6 +324,10 @@ async function execCli(
     return { exitCode: 0, stdout, stderr }
   } catch (error) {
     assert(isRecordLike(error), getErrorMessage(error))
+    assert(
+      error.code !== 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER',
+      `sim workflows run printed more than ${MAX_RESPONSE_BYTES} bytes`
+    )
     assert(!error.killed, `sim workflows run did not exit within ${REQUEST_TIMEOUT_MS / 1000}s`)
     assert(
       typeof error.code === 'number' &&

apps/sim/scripts/test-workflow-version-compare-e2e.ts

@@ -36,6 +36,9 @@ import type { WorkflowState } from '@/stores/workflows/workflow/types'
 const logger = createLogger('WorkflowVersionCompareE2E')
 const execFileAsync = promisify(execFile)
 const MAX_RESPONSE_BYTES = 2 * 1024 * 1024
+/** The first request to each route cold-compiles it under `next dev`. */
+const ROUTE_COMPILE_TIMEOUT_MS = 300_000
+/** Every later request hits a compiled route. */
 const REQUEST_TIMEOUT_MS = 60_000
 const startedAt = new Date().toISOString()
 
@@ -309,30 +312,39 @@ async function seedBrowserWorkflows() {
   }
 }
 
-async function observedFetch(input: string | URL | Request, init?: RequestInit): Promise<Response> {
+async function observedFetch(
+  input: string | URL | Request,
+  init?: RequestInit,
+  timeoutMs = REQUEST_TIMEOUT_MS
+): Promise<Response> {
   const url = new URL(input instanceof Request ? input.url : input)
   assert.equal(
     url.origin,
     baseUrl.origin,
     'E2E requests must remain on the configured loopback app'
   )
+  const method = init?.method ?? (input instanceof Request ? input.method : 'GET')
   const started = performance.now()
   const signal = init?.signal ?? (input instanceof Request ? input.signal : undefined)
-  // boundary-raw-fetch: protocol E2E exercises a separately running local app over real HTTP
-  const response = await fetch(input, {
-    ...init,
-    redirect: 'error',
-    signal: signal
-      ? AbortSignal.any([signal, AbortSignal.timeout(REQUEST_TIMEOUT_MS)])
-      : AbortSignal.timeout(REQUEST_TIMEOUT_MS),
-  })
-  requests.push({
-    method: init?.method ?? (input instanceof Request ? input.method : 'GET'),
-    path: url.pathname,
-    status: response.status,
-    durationMs: Math.round(performance.now() - started),
-  })
-  return response
+  const timeout = AbortSignal.timeout(timeoutMs)
+  try {
+    // boundary-raw-fetch: protocol E2E exercises a separately running local app over real HTTP
+    const response = await fetch(input, {
+      ...init,
+      redirect: 'error',
+      signal: signal ? AbortSignal.any([signal, timeout]) : timeout,
+    })
+    requests.push({
+      method,
+      path: url.pathname,
+      status: response.status,
+      durationMs: Math.round(performance.now() - started),
+    })
+    return response
+  } catch (error) {
+    if (!timeout.aborted) throw error
+    throw new Error(`${method} ${url.pathname}: no response within ${timeoutMs / 1000}s`)
+  }
 }
 
 async function get(path: string, auth: { key?: string; session?: string } = {}, expected = 200) {
@@ -432,6 +444,22 @@ async function runMcp() {
 try {
   await check('seed disposable workspace, versions and credentials', seed)
   if (browserFixturesPath) await check('seed browser scenarios', seedBrowserWorkflows)
+  await check('every route under test compiles and refuses an anonymous request', async () => {
+    for (const [method, path] of [
+      ['GET', `/api/workflows/${workflowId}/deployments/1`],
+      ['GET', `/api/v2/workflows/${workflowId}/versions/1`],
+      ['GET', comparisonPath(1, 2)],
+      ['POST', '/api/mcp'],
+    ] as const) {
+      const response = await observedFetch(
+        new URL(path, baseUrl),
+        { method, headers: { 'x-forwarded-for': '127.0.0.1' } },
+        ROUTE_COMPILE_TIMEOUT_MS
+      )
+      await response.body?.cancel()
+      assert.equal(response.status, 401, `${method} ${path}: unexpected HTTP status`)
+    }
+  })
   await check(
     'session preview migrates both versions while the public archive stays pinned',
     async () => {