Skip to content

Commit f64028a

Browse files
authored
Speed up and stabilize CI e2e (#1551)
* Speed up selfhost e2e: tunable sandbox deadline + 3-way sharding * Fix the local e2e suite: toolkit MCP DB lock, Bun-only spawn, stale auth selector * Stabilize CI process lifecycle
1 parent c83cdd2 commit f64028a

26 files changed

Lines changed: 714 additions & 217 deletions

.github/workflows/ci.yml

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,25 @@ jobs:
192192
fail-fast: false
193193
matrix:
194194
include:
195-
# Each cloud shard boots its own fresh dev stack. On 4 vCPU runners,
196-
# four fatter shards keep the longest shard below selfhost while saving
197-
# four runner boots and four warm cache restores.
198-
- { target: cloud, shard: 1/4, shard-name: 1of4 }
199-
- { target: cloud, shard: 2/4, shard-name: 2of4 }
200-
- { target: cloud, shard: 3/4, shard-name: 3of4 }
201-
- { target: cloud, shard: 4/4, shard-name: 4of4 }
202-
- target: selfhost
195+
# PGlite is deliberately single-connection, and under a sustained
196+
# multi-minute shard it can stop accepting postgres sockets. Keep
197+
# every hermetic dev stack short: eight serial shards remove that
198+
# lifetime-dependent failure and put cloud below the selfhost lane.
199+
- { target: cloud, shard: 1/8, shard-name: 1of8 }
200+
- { target: cloud, shard: 2/8, shard-name: 2of8 }
201+
- { target: cloud, shard: 3/8, shard-name: 3of8 }
202+
- { target: cloud, shard: 4/8, shard-name: 4of8 }
203+
- { target: cloud, shard: 5/8, shard-name: 5of8 }
204+
- { target: cloud, shard: 6/8, shard-name: 6of8 }
205+
- { target: cloud, shard: 7/8, shard-name: 7of8 }
206+
- { target: cloud, shard: 8/8, shard-name: 8of8 }
207+
# Selfhost shards the same way: each shard is its own runner booting
208+
# its own fresh instance (own port block + data dir), so the
209+
# project's shared-bootstrap-admin assumption stays intact per shard
210+
# and `fileParallelism: false` still serializes within a shard.
211+
- { target: selfhost, shard: 1/3, shard-name: 1of3 }
212+
- { target: selfhost, shard: 2/3, shard-name: 2of3 }
213+
- { target: selfhost, shard: 3/3, shard-name: 3of3 }
203214
runs-on: blacksmith-4vcpu-ubuntu-2404
204215
timeout-minutes: 30
205216
steps:
@@ -241,20 +252,20 @@ jobs:
241252

242253
# The globalsetup boots the target's own dev server (ports are claimed
243254
# per checkout, so this is hermetic) and tears it down after the run.
244-
# --retry=2: browser scenarios can still hit isolated waitFor timeouts
245-
# (single-test waitFor timeouts, not systemic failures); a retry on the
246-
# same booted stack clears them.
255+
# Do not retry scenarios: retries hide flakes and multiply slow timeout
256+
# failures. The fixtures and process lifecycle are deterministic enough
257+
# that the first result is the result.
247258
- name: Run cloud scenarios
248259
if: matrix.target == 'cloud'
249260
env:
250261
MCP_SESSION_TIMEOUT_MS: "3000"
251262
MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: "6000"
252-
run: bunx vitest run --project cloud --retry=2 ${{ matrix.shard && format('--shard={0}', matrix.shard) || '' }}
263+
run: bunx vitest run --project cloud ${{ matrix.shard && format('--shard={0}', matrix.shard) || '' }}
253264
working-directory: e2e
254265

255266
- name: Run selfhost scenarios
256267
if: matrix.target == 'selfhost'
257-
run: bunx vitest run --project selfhost --retry=2
268+
run: bunx vitest run --project selfhost ${{ matrix.shard && format('--shard={0}', matrix.shard) || '' }}
258269
working-directory: e2e
259270

260271
# Failed runs keep their trace.zip / session.mp4 / step screenshots in
@@ -268,10 +279,7 @@ jobs:
268279
retention-days: 7
269280

270281
e2e-local:
271-
name: E2E (stdio MCP)
272-
# Skipped on pull_request: the local scenario boots a real `executor web`
273-
# plus a browser and is currently flaky on PRs. Still runs on push to main.
274-
if: github.event_name != 'pull_request'
282+
name: E2E (local)
275283
runs-on: blacksmith-4vcpu-ubuntu-2404
276284
timeout-minutes: 20
277285
steps:
@@ -314,15 +322,10 @@ jobs:
314322
run: bunx playwright install --with-deps chromium chromium-headless-shell
315323
working-directory: e2e
316324

317-
# The `local` project is excluded from the default `test` chain (each
318-
# scenario boots its own `executor web`). Run just the stdio MCP scenario
319-
# here: it is the auto-connect / env-as-secret regression guard, and
320-
# running it alone avoids the boot-resource accumulation and the
321-
# pre-existing browser flakiness of the rest of the local suite. Expanding
322-
# to the full `local` project (bun run test:local) is a follow-up once
323-
# those are stabilized.
324-
- name: Run the stdio MCP scenario
325-
run: bunx vitest run --project local local/stdio-mcp.test.ts
325+
# Each scenario owns its server, browser, data directory, and descendants;
326+
# run the complete hermetic suite on PRs without scenario retries.
327+
- name: Run local scenarios
328+
run: bunx vitest run --project local
326329
working-directory: e2e
327330

328331
desktop-smoke:

apps/cli/src/main.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,11 @@ const waitForShutdownSignal = () =>
215215
const shutdown = () => resume(Effect.void);
216216
process.once("SIGINT", shutdown);
217217
process.once("SIGTERM", shutdown);
218+
process.once("SIGHUP", shutdown);
218219
return Effect.sync(() => {
219220
process.off("SIGINT", shutdown);
220221
process.off("SIGTERM", shutdown);
222+
process.off("SIGHUP", shutdown);
221223
});
222224
});
223225

apps/host-selfhost/src/config.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ export interface SelfHostConfig {
4545
readonly organizationName: string;
4646
/** URL slug for org-prefixed console paths (`/<slug>/policies`). */
4747
readonly orgSlug: string;
48+
/**
49+
* Sandbox execution budget passed to the QuickJS runtime, or undefined for
50+
* the runtime's own default (5 minutes). An operator knob in principle, but
51+
* its real consumer is the e2e harness, which shrinks it to seconds so the
52+
* sandbox-deadline scenario proves its race without waiting out real
53+
* minutes (the same pattern as MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS on cloud).
54+
*/
55+
readonly sandboxTimeoutMs: number | undefined;
4856
}
4957

5058
export const resolveDataDir = (): string =>
@@ -151,9 +159,26 @@ export const loadConfig = (): SelfHostConfig => {
151159
bootstrapAdminName: process.env.EXECUTOR_BOOTSTRAP_ADMIN_NAME ?? "Admin",
152160
organizationName: process.env.EXECUTOR_ORG_NAME ?? "Default",
153161
orgSlug: resolveOrgSlug(),
162+
sandboxTimeoutMs: resolveSandboxTimeoutMs(),
154163
};
155164
};
156165

166+
// A malformed value is refused rather than silently ignored: an operator who
167+
// sets the knob and typos it should find out at boot, not by watching a
168+
// runaway execution use the 5-minute default.
169+
const resolveSandboxTimeoutMs = (): number | undefined => {
170+
const raw = process.env.EXECUTOR_SANDBOX_TIMEOUT_MS;
171+
if (!raw) return undefined;
172+
const parsed = Number(raw);
173+
if (!Number.isFinite(parsed) || parsed <= 0) {
174+
// oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: refuse to boot on a malformed operator knob
175+
throw new Error(
176+
`EXECUTOR_SANDBOX_TIMEOUT_MS ${JSON.stringify(raw)} is not a positive number of milliseconds`,
177+
);
178+
}
179+
return Math.floor(parsed);
180+
};
181+
157182
// The org slug doubles as a URL segment (`/<slug>/policies`), so an
158183
// operator-set value must fit the shared grammar and avoid reserved root
159184
// segments (api, mcp, login, …) — a colliding slug would shadow real routes.

apps/host-selfhost/src/execution.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,12 @@ export const SelfHostHostConfig: Layer.Layer<HostConfig> = Layer.sync(HostConfig
6565

6666
export const SelfHostCodeExecutorProvider: Layer.Layer<CodeExecutorProvider> = Layer.sync(
6767
CodeExecutorProvider,
68-
() => makeQuickJsExecutor(),
68+
() => {
69+
const { sandboxTimeoutMs } = loadConfig();
70+
return makeQuickJsExecutor(
71+
sandboxTimeoutMs === undefined ? {} : { timeoutMs: sandboxTimeoutMs },
72+
);
73+
},
6974
);
7075

7176
/**

apps/local/src/executor.ts

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import type { McpPluginExtension } from "@executor-js/plugin-mcp";
1919
import executorConfig from "../executor.config";
2020
import { localAnalytics } from "./analytics";
2121
import { localDataMigrations } from "./db/data-migrations";
22-
import { openOwnedLocalDatabase } from "./db/owned-database";
22+
import { openOwnedLocalDatabase, type OwnedLocalDatabase } from "./db/owned-database";
2323

2424
interface ResolvedStorage {
2525
readonly dataDir: string;
@@ -56,6 +56,16 @@ type LocalPlugins = readonly AnyPlugin[];
5656

5757
export interface LocalExecutorOptions {
5858
readonly activeToolkitSlug?: string;
59+
/**
60+
* Reuse an already-open owned database instead of opening (and locking) the
61+
* data dir again. A toolkit-scoped MCP session differs from the default one
62+
* only in its plugin set, so it must ride the running server's DB handle:
63+
* `openOwnedLocalDatabase` takes an EXCLUSIVE lock, and a second open from
64+
* inside the same process contends with the lock this process already holds.
65+
* The borrowed handle is NOT closed when the derived executor disposes —
66+
* whoever opened it still owns its lifetime.
67+
*/
68+
readonly borrowedDb?: OwnedLocalDatabase;
5969
}
6070

6171
const loadLocalPlugins = (options: LocalExecutorOptions = {}) =>
@@ -92,6 +102,10 @@ const loadLocalPlugins = (options: LocalExecutorOptions = {}) =>
92102
interface LocalExecutorBundle {
93103
readonly executor: Executor<LocalPlugins>;
94104
readonly plugins: LocalPlugins;
105+
/** The owned DB this bundle opened (or borrowed). Surfaced so a
106+
* toolkit-scoped executor can ride the SAME handle instead of contending
107+
* with this process's own exclusive data-dir lock. */
108+
readonly db: OwnedLocalDatabase;
95109
/** Where this daemon's web UI is reachable, resolved once at boot. Surfaced
96110
* so callers building user-facing links (MCP artifact deep links) use the
97111
* same origin the executor itself was configured with. */
@@ -151,23 +165,27 @@ const createLocalExecutorLayer = (options: LocalExecutorOptions = {}) => {
151165
const tenantId = makeTenantId(cwd);
152166
const tables = collectTables();
153167

154-
const owned = yield* Effect.acquireRelease(
155-
Effect.tryPromise({
156-
try: () =>
157-
openOwnedLocalDatabase({
158-
dataDir: storage.dataDir,
159-
tables,
160-
namespace: localNamespace,
161-
tenantId,
168+
// A borrowed handle is owned by its opener, so it is used as-is and left
169+
// open on release; only a handle opened here is closed here.
170+
const owned = options.borrowedDb
171+
? options.borrowedDb
172+
: yield* Effect.acquireRelease(
173+
Effect.tryPromise({
174+
try: () =>
175+
openOwnedLocalDatabase({
176+
dataDir: storage.dataDir,
177+
tables,
178+
namespace: localNamespace,
179+
tenantId,
180+
}),
181+
catch: (cause) =>
182+
new LocalExecutorCreateError({
183+
message: CREATE_SQLITE_ERROR_MESSAGE,
184+
cause,
185+
}),
162186
}),
163-
catch: (cause) =>
164-
new LocalExecutorCreateError({
165-
message: CREATE_SQLITE_ERROR_MESSAGE,
166-
cause,
167-
}),
168-
}),
169-
(database) => Effect.promise(() => database.close()).pipe(Effect.ignore),
170-
);
187+
(database) => Effect.promise(() => database.close()).pipe(Effect.ignore),
188+
);
171189
const sqlite = owned.db;
172190
const migration = owned.migration;
173191

@@ -243,7 +261,7 @@ const createLocalExecutorLayer = (options: LocalExecutorOptions = {}) => {
243261
);
244262
}
245263

246-
return { executor, plugins, webBaseUrl };
264+
return { executor, plugins, webBaseUrl, db: owned };
247265
}),
248266
);
249267
};
@@ -257,6 +275,7 @@ export const createExecutorHandle = async (options: LocalExecutorOptions = {}) =
257275
executor: bundle.executor,
258276
plugins: bundle.plugins,
259277
webBaseUrl: bundle.webBaseUrl,
278+
db: bundle.db,
260279
dispose: async () => {
261280
await Effect.runPromise(Effect.ignore(bundle.executor.close()));
262281
await ignorePromiseFailure("disposeRuntime", () => runtime.dispose());

apps/local/src/main.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,13 @@ export const createServerHandlers = async (token: string): Promise<ServerHandler
137137
},
138138
};
139139
}
140+
// Borrow the running server's DB handle: this process already holds the
141+
// data dir's exclusive ownership lock, so opening it a second time here
142+
// fails against ourselves. The toolkit executor differs only in its
143+
// plugin set, and the borrowed handle stays open when it disposes.
140144
const handle = await createExecutorHandle({
141145
activeToolkitSlug: resource.slug,
146+
borrowedDb: (await getExecutorBundle()).db,
142147
});
143148
const toolkitEngine = withExecutionAnalytics(
144149
createExecutionEngine({

apps/local/src/serve.ts

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ interface ViteChild {
113113
readonly stop: () => Promise<void>;
114114
}
115115

116+
const viteChildSignals = ["SIGINT", "SIGTERM", "SIGHUP"] as const;
117+
116118
async function allocatePort(): Promise<number> {
117119
const probe = Bun.serve({
118120
port: 0,
@@ -127,15 +129,15 @@ async function allocatePort(): Promise<number> {
127129
async function startViteChild(): Promise<ViteChild> {
128130
const vitePort = await allocatePort();
129131
const cwd = resolve(import.meta.dirname, "..");
132+
const viteEntrypoint = resolve(cwd, "node_modules/vite/bin/vite.js");
130133
const env = { ...process.env };
131134
delete env.PORT;
132-
// `bunx --bun vite` runs vite under Bun, matching the `dev:vite` script
133-
// already in apps/local. --strictPort keeps the URL we hand back stable.
135+
// Run Vite directly under Bun, matching the `dev:vite` script without a
136+
// bunx wrapper that can outlive its child. --strictPort keeps the URL stable.
134137
const child: Subprocess = Bun.spawn(
135138
[
136-
"bunx",
137-
"--bun",
138-
"vite",
139+
process.execPath,
140+
viteEntrypoint,
139141
"dev",
140142
"--port",
141143
String(vitePort),
@@ -158,33 +160,59 @@ async function startViteChild(): Promise<ViteChild> {
158160
},
159161
);
160162

163+
let stopping = false;
164+
const stop = async (): Promise<void> => {
165+
if (stopping) {
166+
await child.exited;
167+
return;
168+
}
169+
stopping = true;
170+
for (const signal of viteChildSignals) process.off(signal, stopOnParentSignal);
171+
if (child.exitCode === null) child.kill();
172+
await Promise.race([child.exited, Bun.sleep(5_000)]);
173+
if (child.exitCode === null) child.kill("SIGKILL");
174+
await child.exited;
175+
};
176+
const stopOnParentSignal = (): void => {
177+
// A PTY/session teardown can signal the CLI while Vite is still optimizing
178+
// dependencies, before the server's normal stop handle exists. Reap the
179+
// owned child immediately; the CLI's signal waiter performs full cleanup
180+
// once startup has completed.
181+
void stop();
182+
};
183+
for (const signal of viteChildSignals) process.once(signal, stopOnParentSignal);
184+
161185
const url = `http://127.0.0.1:${vitePort}`;
162186
const deadline = Date.now() + 30_000;
163187
while (Date.now() < deadline) {
164188
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: probing a child process that may not be listening yet
165189
try {
166-
const r = await fetch(`${url}/`, { redirect: "manual" });
190+
const r = await fetch(`${url}/`, {
191+
redirect: "manual",
192+
// A listening socket is not proof that Vite can answer. Bound each
193+
// probe so one accepted-but-stalled request cannot defeat the 30s boot
194+
// deadline and wedge the entire local e2e suite.
195+
signal: AbortSignal.timeout(5_000),
196+
});
167197
if (r.status < 500) {
168198
await r.body?.cancel();
169199
return {
170200
url,
171-
stop: async () => {
172-
child.kill();
173-
await child.exited;
174-
},
201+
stop,
175202
};
176203
}
177204
await r.body?.cancel();
178205
} catch {
179206
// not up yet
180207
}
181208
if (child.exitCode !== null) {
209+
await stop();
182210
// oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: child process aborted before becoming ready
183211
throw new Error(`vite dev exited with code ${child.exitCode} before becoming ready`);
184212
}
185213
await Bun.sleep(150);
186214
}
187-
child.kill();
215+
await stop();
188216
// oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: vite never became reachable
189217
throw new Error(`vite dev did not become reachable on ${url} within 30s`);
190218
}

e2e/local/auth.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ scenario(
3636
await page.goto(url, { waitUntil: "domcontentloaded" });
3737
await page.getByRole("link", { name: "Secrets" }).first().waitFor({ timeout: 30_000 });
3838
// Integrations actually LOAD (the built-in Executor integration) — proves
39-
// auth + data, not just the static shell.
40-
await page.getByText("built-in").first().waitFor({ timeout: 30_000 });
39+
// auth + data, not just the static shell. Matched on the row's stable
40+
// testid: the list renders each integration's name + slug, never the
41+
// literal "built-in" (that string is only an internal `kind`).
42+
await page.getByTestId("integration-entry-executor").first().waitFor({ timeout: 30_000 });
4143
// The token is moved out of the URL and persisted to localStorage.
4244
expect(new URL(page.url()).searchParams.has("_token")).toBe(false);
4345
const stored = await page.evaluate(() => localStorage.getItem("executor.authToken"));
@@ -70,7 +72,7 @@ scenario(
7072
await page.getByRole("button", { name: "Connect" }).click();
7173
await page.getByRole("link", { name: "Secrets" }).first().waitFor({ timeout: 30_000 });
7274
// The reconnect fully restores — integrations LOAD, not a stale 401.
73-
await page.getByText("built-in").first().waitFor({ timeout: 30_000 });
75+
await page.getByTestId("integration-entry-executor").first().waitFor({ timeout: 30_000 });
7476
});
7577
}),
7678
);

0 commit comments

Comments
 (0)