diff --git a/.github/scripts/stop-session.sh b/.github/scripts/stop-session.sh index 860561a568e..1dc62d1dab4 100755 --- a/.github/scripts/stop-session.sh +++ b/.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 diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 0fc7d4b9027..1909f3676d4 100644 --- a/.github/workflows/test-build.yml +++ b/.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: diff --git a/apps/sim/scripts/test-desktop-inbox-e2e.ts b/apps/sim/scripts/test-desktop-inbox-e2e.ts index 5ef02e09210..32c296e7424 100644 --- a/apps/sim/scripts/test-desktop-inbox-e2e.ts +++ b/apps/sim/scripts/test-desktop-inbox-e2e.ts @@ -284,20 +284,16 @@ function openDoorbell(desktop: Desktop) { function startExecutor(desktop: Desktop, doorbell: ReturnType) { const ran = new Map() const completed = new Set() - let running = false + let inFlight: Promise | 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 { + 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 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 () => { diff --git a/apps/sim/scripts/test-workflow-stop-after-e2e.ts b/apps/sim/scripts/test-workflow-stop-after-e2e.ts index 6088f489acd..2bd37c8e88c 100644 --- a/apps/sim/scripts/test-workflow-stop-after-e2e.ts +++ b/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' && diff --git a/apps/sim/scripts/test-workflow-version-compare-e2e.ts b/apps/sim/scripts/test-workflow-version-compare-e2e.ts index 15b0b8f59c0..a59b7890e0e 100644 --- a/apps/sim/scripts/test-workflow-version-compare-e2e.ts +++ b/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 { +async function observedFetch( + input: string | URL | Request, + init?: RequestInit, + timeoutMs = REQUEST_TIMEOUT_MS +): Promise { 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 () => {