Fix CI review findings

Author: RhysSullivan

.github/workflows/ci.yml

@@ -67,7 +67,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -90,7 +90,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -113,7 +113,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -127,15 +127,6 @@ jobs:
         with:
           node-version: 24
 
-      - name: Cache Turbo task cache
-        uses: useblacksmith/cache@v5
-        with:
-          path: .turbo
-          key: ${{ runner.os }}-turbo-typecheck-${{ hashFiles('bun.lock', 'turbo.json') }}
-          restore-keys: |
-            ${{ runner.os }}-turbo-typecheck-
-            ${{ runner.os }}-turbo-
-
       - run: bun install --frozen-lockfile
 
       - run: bun run typecheck
@@ -155,7 +146,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -169,15 +160,6 @@ jobs:
         with:
           node-version: 24
 
-      - name: Cache Turbo task cache
-        uses: useblacksmith/cache@v5
-        with:
-          path: .turbo
-          key: ${{ runner.os }}-turbo-test-${{ hashFiles('bun.lock', 'turbo.json') }}
-          restore-keys: |
-            ${{ runner.os }}-turbo-test-
-            ${{ runner.os }}-turbo-
-
       - run: bun install --frozen-lockfile
 
       - run: bun run test
@@ -199,7 +181,8 @@ jobs:
     runs-on: blacksmith-4vcpu-ubuntu-2404
     timeout-minutes: 30
     env:
-      E2E_MCP_FAST_IDLE_TEARDOWN: "true"
+      MCP_SESSION_TIMEOUT_MS: "3000"
+      MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: "6000"
     steps:
       - uses: actions/checkout@v4
 
@@ -208,7 +191,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -224,7 +207,7 @@ jobs:
       - run: bun install --frozen-lockfile
 
       - name: Cache Playwright browsers
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.cache/ms-playwright
           key: ${{ runner.os }}-playwright-1.60.0
@@ -271,7 +254,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}
@@ -287,7 +270,7 @@ jobs:
       - run: bun install --frozen-lockfile
 
       - name: Cache Playwright browsers
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.cache/ms-playwright
           key: ${{ runner.os }}-playwright-1.60.0
@@ -328,7 +311,7 @@ jobs:
           bun-version: 1.3.11
 
       - name: Cache Bun package cache
-        uses: useblacksmith/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.bun/install/cache
           key: ${{ runner.os }}-bun-1.3.11-${{ hashFiles('bun.lock') }}

apps/cloud/src/env-augment.d.ts

@@ -58,6 +58,8 @@ declare global {
       EXECUTOR_MCP_DEBUG?: string;
       MCP_AUTHKIT_DOMAIN?: string;
       MCP_RESOURCE_ORIGIN?: string;
+      MCP_SESSION_TIMEOUT_MS?: string;
+      MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS?: string;
       NODE_ENV?: string;
 
       // Shared with frontend

apps/cloud/src/mcp/session-durable-object.ts

@@ -77,6 +77,13 @@ const LONG_LIVED_DB_IDLE_TIMEOUT_SECONDS = 5;
 const LONG_LIVED_DB_MAX_LIFETIME_SECONDS = 120;
 const TELEMETRY_FLUSH_TIMEOUT_MS = 1_000;
 
+const positiveMilliseconds = (raw: string | undefined): number | undefined => {
+  if (!raw) return undefined;
+  const parsed = Number(raw);
+  if (!Number.isFinite(parsed) || parsed <= 0) return undefined;
+  return Math.floor(parsed);
+};
+
 type CloudSessionDbHandle = DbServiceShape & {
   readonly sql: Sql;
   readonly end: () => Promise<void>;
@@ -158,6 +165,16 @@ const makeSessionServices = (dbHandle: CloudSessionDbHandle) => {
 // ---------------------------------------------------------------------------
 
 export class McpSessionDOSqlite extends McpAgentSessionDOBase<Env, CloudSessionDbHandle> {
+  protected override sessionTimeoutMs(): number {
+    return positiveMilliseconds(env.MCP_SESSION_TIMEOUT_MS) ?? super.sessionTimeoutMs();
+  }
+
+  protected override maxPausedSessionIdleMs(): number {
+    return (
+      positiveMilliseconds(env.MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS) ?? super.maxPausedSessionIdleMs()
+    );
+  }
+
   protected override openSessionDb(): CloudSessionDbHandle {
     return makeDbHandle({
       idleTimeout: LONG_LIVED_DB_IDLE_TIMEOUT_SECONDS,

e2e/cloud/mcp-client-sessions.test.ts

@@ -18,6 +18,7 @@ import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/
 import { scenario } from "../src/scenario";
 import { Api, Mcp, Target } from "../src/services";
 import type { Identity } from "../src/target";
+import { configuredMcpPausedSessionIdleTimeoutMs } from "../setup/mcp-session-timeouts";
 
 const coreApi = composePluginApi([] as const);
 
@@ -216,19 +217,19 @@ return JSON.stringify(result);
 const executionIdOf = (text: string): string | undefined =>
   /\bexecutionId:\s*(\S+)/.exec(text)?.[1];
 
-const FAST_IDLE_TEARDOWN = process.env.E2E_MCP_FAST_IDLE_TEARDOWN === "true";
+const IDLE_TEARDOWN_BUFFER_MS = 2_000;
+const IDLE_TEARDOWN_GAP_MS = configuredMcpPausedSessionIdleTimeoutMs() + IDLE_TEARDOWN_BUFFER_MS;
+const IDLE_TEARDOWN_SCENARIO_TIMEOUT_MS = IDLE_TEARDOWN_GAP_MS + 120_000;
 
-// The session DO tears its runtime down after 5 minutes without a request
-// (SESSION_TIMEOUT_MS in McpSessionDOBase) and rebuilds it from storage on
-// the next one — the same engine-state wipe a workerd eviction or a deploy
-// causes. Paused approvals deliberately do NOT survive this (durable pause
-// state is out of scope); the contract is that an expired pause fails with
-// recovery guidance and the session keeps working.
-const IDLE_TEARDOWN_GAP = "6 minutes";
+// The session DO tears its runtime down after the configured idle ceiling and
+// rebuilds it from storage on the next request. The e2e harness shortens that
+// ceiling when it owns the cloud dev stack. Paused approvals deliberately do
+// not survive this (durable pause state is out of scope); the contract is that
+// an expired pause fails with recovery guidance and the session keeps working.
 
 scenario(
   "MCP sessions · an approval paused past the idle window expires with re-run guidance, not a dead end",
-  { timeout: 480_000 },
+  { timeout: IDLE_TEARDOWN_SCENARIO_TIMEOUT_MS },
   Effect.gen(function* () {
     const target = yield* Target;
     const { client } = yield* Api;
@@ -243,7 +244,9 @@ scenario(
 
     yield* Effect.gen(function* () {
       const first = yield* Effect.promise(() => connectClient(target.mcpUrl, bearer));
-      const sessionId = first.transport.sessionId ?? "";
+      const sessionId = first.transport.sessionId;
+      expect(sessionId, "the first client got a session id").toEqual(expect.any(String));
+      if (sessionId === undefined) return yield* Effect.die("missing session id");
       const paused = yield* Effect.promise(() =>
         first.client.callTool({ name: "execute", arguments: { code: GATED_CODE } }),
       ).pipe(Effect.ensuring(closeQuietly(first)));
@@ -253,23 +256,18 @@ scenario(
       );
       const executionId = executionIdOf(pausedText);
       expect(executionId, "the paused result carries the executionId").toEqual(expect.any(String));
+      if (executionId === undefined) return yield* Effect.die("missing paused execution id");
 
       // The user thinks the approval over for longer than the session keeps
-      // its runtime warm; the pause is gone when they come back. CI skips the
-      // real idle wait by resuming a deliberately stale id, which exercises the
-      // same recovery branch without spending six minutes in this shard.
-      const expiredExecutionId =
-        FAST_IDLE_TEARDOWN && executionId ? `${executionId}:expired-for-ci` : executionId;
-      if (!FAST_IDLE_TEARDOWN) {
-        yield* Effect.sleep(IDLE_TEARDOWN_GAP);
-      }
+      // its runtime warm; the pause is gone when they come back.
+      yield* Effect.sleep(IDLE_TEARDOWN_GAP_MS);
 
       const second = yield* Effect.promise(() => connectClient(target.mcpUrl, bearer, sessionId));
       yield* Effect.gen(function* () {
         const resumed = yield* Effect.promise(() =>
           second.client.callTool({
             name: "resume",
-            arguments: { executionId: expiredExecutionId ?? "", action: "accept", content: "{}" },
+            arguments: { executionId, action: "accept", content: "{}" },
           }),
         );
         const resumedText = textOf(resumed);
@@ -291,11 +289,12 @@ scenario(
         );
         const freshExecutionId = executionIdOf(reExecutedText);
         expect(freshExecutionId, "the re-run mints a different executionId").not.toBe(executionId);
+        if (freshExecutionId === undefined) return yield* Effect.die("missing fresh execution id");
 
         const resumedFresh = yield* Effect.promise(() =>
           second.client.callTool({
             name: "resume",
-            arguments: { executionId: freshExecutionId ?? "", action: "accept", content: "{}" },
+            arguments: { executionId: freshExecutionId, action: "accept", content: "{}" },
           }),
         );
         expect(resumedFresh.isError, "the fresh approval resumes to completion").not.toBe(true);

e2e/setup/cloud.boot.ts

@@ -90,6 +90,8 @@ export const bootCloud = async (options: CloudBootOptions): Promise<CloudBooted>
     // The AuthKit domain (MCP OAuth metadata + JWKS) is the emulator too.
     MCP_AUTHKIT_DOMAIN: workosUrl,
     MCP_RESOURCE_ORIGIN: options.publicUrl,
+    MCP_SESSION_TIMEOUT_MS: process.env.MCP_SESSION_TIMEOUT_MS,
+    MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: process.env.MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS,
     ALLOW_LOCAL_NETWORK: "true",
     // Shrink the per-org hourly execution cap (prod default 1000) to a number
     // the rate-limit-backstop scenario can actually exhaust with real

e2e/setup/cloud.globalsetup.ts

@@ -10,6 +10,7 @@ import { claimAndBoot } from "../src/ports";
 import { E2E_COOKIE_PASSWORD, E2E_WORKOS_CLIENT_ID } from "../targets/cloud";
 import { waitForHttp } from "./boot";
 import { bootCloud } from "./cloud.boot";
+import { ensureE2eMcpSessionTimeoutEnv } from "./mcp-session-timeouts";
 import { bootMotel, motelExporterEnv } from "./motel";
 import { RUNS_DIR } from "../src/scenario";
 
@@ -31,6 +32,7 @@ export default async function setup(): Promise<(() => Promise<void>) | void> {
   }
 
   if (bootLogFile) mkdirSync(resolve(bootLogFile, ".."), { recursive: true });
+  ensureE2eMcpSessionTimeoutEnv();
 
   // Suite-owned trace store — every run captures distributed traces. Booted
   // once outside the retry: it binds its own OS-assigned port (not a claimed

e2e/setup/mcp-session-timeouts.ts

@@ -0,0 +1,34 @@
+const DEFAULT_E2E_MCP_SESSION_TIMEOUT_MS = 3_000;
+const DEFAULT_E2E_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS = 6_000;
+const PRODUCTION_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS = 9 * 60 * 1000;
+
+export const MCP_SESSION_TIMEOUT_ENV = "MCP_SESSION_TIMEOUT_MS";
+export const MCP_PAUSED_SESSION_IDLE_TIMEOUT_ENV = "MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS";
+
+const positiveMilliseconds = (raw: string | undefined): number | undefined => {
+  if (!raw) return undefined;
+  const parsed = Number(raw);
+  if (!Number.isFinite(parsed) || parsed <= 0) return undefined;
+  return Math.floor(parsed);
+};
+
+export const ensureE2eMcpSessionTimeoutEnv = (): {
+  readonly sessionTimeoutMs: number;
+  readonly pausedSessionIdleTimeoutMs: number;
+} => {
+  const sessionTimeoutMs =
+    positiveMilliseconds(process.env[MCP_SESSION_TIMEOUT_ENV]) ??
+    DEFAULT_E2E_MCP_SESSION_TIMEOUT_MS;
+  const pausedSessionIdleTimeoutMs =
+    positiveMilliseconds(process.env[MCP_PAUSED_SESSION_IDLE_TIMEOUT_ENV]) ??
+    DEFAULT_E2E_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS;
+
+  process.env[MCP_SESSION_TIMEOUT_ENV] = String(sessionTimeoutMs);
+  process.env[MCP_PAUSED_SESSION_IDLE_TIMEOUT_ENV] = String(pausedSessionIdleTimeoutMs);
+
+  return { sessionTimeoutMs, pausedSessionIdleTimeoutMs };
+};
+
+export const configuredMcpPausedSessionIdleTimeoutMs = (): number =>
+  positiveMilliseconds(process.env[MCP_PAUSED_SESSION_IDLE_TIMEOUT_ENV]) ??
+  PRODUCTION_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS;