From 5a29de4c672269e6f214ac0ebeb1a4c30a299460 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:26:49 -0700 Subject: [PATCH] Stabilize cloud E2E scenarios --- .github/workflows/ci.yml | 4 +- e2e/cloud/mcp-browser-resume-page.test.ts | 122 +++++----- .../graphql-introspection-health.test.ts | 224 +++++++++--------- e2e/scenarios/policies-ui.test.ts | 14 +- e2e/setup/mcp-session-timeouts.ts | 4 +- 5 files changed, 195 insertions(+), 173 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c340085a8..a9daf5378b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -304,7 +304,9 @@ jobs: if: matrix.target == 'cloud' env: MCP_SESSION_TIMEOUT_MS: "3000" - MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: "6000" + # Still 18x shorter than production, but long enough for a cold Vite + # resume route to compile under a fully loaded CI runner. + MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: "30000" run: bun scripts/run-ci-shard.ts cloud ${{ matrix['shard-index'] }} working-directory: e2e diff --git a/e2e/cloud/mcp-browser-resume-page.test.ts b/e2e/cloud/mcp-browser-resume-page.test.ts index 26be5910a4..5e82e5eb0d 100644 --- a/e2e/cloud/mcp-browser-resume-page.test.ts +++ b/e2e/cloud/mcp-browser-resume-page.test.ts @@ -18,7 +18,6 @@ import { scenario } from "../src/scenario"; import { Api, Browser, Mcp, Target } from "../src/services"; import { parseBrowserApproval } from "../src/surfaces/mcp"; import type { Identity } from "../src/target"; -import { visit } from "../src/surfaces/browser"; const coreApi = composePluginApi([] as const); @@ -196,68 +195,75 @@ scenario( openBrowserApprovalSession(target.mcpUrl, bearer), ); yield* Effect.gen(function* () { - const paused = yield* Effect.promise(() => - session.client.callTool({ name: "execute", arguments: { code: GATED_CODE } }), - ); - const approval = parseBrowserApproval({ - raw: paused, - text: textOf(paused), - ok: paused.isError !== true, - }); + yield* browser.session(identity, async ({ page, step }) => { + // Open Chromium before pausing. The suite intentionally compresses + // the production nine-minute decision window to 30 seconds, and a + // cold browser launch should not consume the user's decision time. + const paused = await session.client.callTool({ + name: "execute", + arguments: { code: GATED_CODE }, + }); + const approval = parseBrowserApproval({ + raw: paused, + text: textOf(paused), + ok: paused.isError !== true, + }); - const approvalUrl = new URL(approval.approvalUrl); - const mcpSessionId = approvalUrl.searchParams.get("mcp_session_id"); - expect( - mcpSessionId, - "approval URL carries the MCP session id that the browser page will query", - ).toEqual(expect.any(String)); - expect(mcpSessionId, "approval URL points at the session that paused").toBe( - session.transport.sessionId, - ); - - const [resumed] = yield* Effect.all( - [ - Effect.promise(() => - session.client.callTool({ - name: "resume", - arguments: { executionId: approval.executionId }, - }), - ), - browser.session(identity, async ({ page, step }) => { - await step("Open the paused execution approval page", async () => { - await visit(page, pathWithSearch(approval.approvalUrl)); - await page.getByText("User approval required").waitFor(); - }); + const approvalUrl = new URL(approval.approvalUrl); + const mcpSessionId = approvalUrl.searchParams.get("mcp_session_id"); + expect( + mcpSessionId, + "approval URL carries the MCP session id that the browser page will query", + ).toEqual(expect.any(String)); + expect(mcpSessionId, "approval URL points at the session that paused").toBe( + session.transport.sessionId, + ); - await step("Review the paused tool call details", async () => { - await page.getByText("Pending request").waitFor(); - await page.getByText(/Approve executor\.coreTools\.policies\.list\?/).waitFor(); + await step("Approve the paused tool call through the browser page", async () => { + // The resume GET is this page's concrete readiness signal. The + // generic browser `visit` helper additionally waits up to five + // seconds for network-idle, which is inappropriate while this + // deliberately short approval lease is ticking. + const pausedExecutionLoaded = page.waitForResponse((response) => { + const responseUrl = new URL(response.url()); + return ( + response.request().method() === "GET" && + responseUrl.pathname.endsWith( + `/api/mcp-sessions/${mcpSessionId}/executions/${approval.executionId}`, + ) && + response.status() === 200 + ); + }); + await page.goto(pathWithSearch(approval.approvalUrl), { waitUntil: "load" }); + await pausedExecutionLoaded; + await page.getByText("User approval required").waitFor(); + await page.getByText("Pending request").waitFor(); + await page.getByText(/Approve executor\.coreTools\.policies\.list\?/).waitFor(); - const approve = page.getByRole("button", { name: "Approve" }); - await approve.waitFor(); - expect( - await approve.isEnabled(), - "the approve control is enabled for the paused execution", - ).toBe(true); - expect( - await page.getByText(UNAVAILABLE_COPY).count(), - "the resume page does not show the expired-session failure copy", - ).toBe(0); - }); + const approve = page.getByRole("button", { name: "Approve" }); + await approve.waitFor(); + expect( + await approve.isEnabled(), + "the approve control is enabled for the paused execution", + ).toBe(true); + expect( + await page.getByText(UNAVAILABLE_COPY).count(), + "the resume page does not show the expired-session failure copy", + ).toBe(0); + await page.getByRole("button", { name: "Approve" }).click(); + await page.getByText("Approve sent").waitFor(); + }); - await step("Approve the paused tool call", async () => { - await page.getByRole("button", { name: "Approve" }).click(); - await page.getByText("Approve sent").waitFor(); - }); - }), - ], - { concurrency: "unbounded" }, - ); + const resumed = await session.client.callTool({ + name: "resume", + arguments: { executionId: approval.executionId }, + }); - expect(resumed.isError, "browser-mode resume completed after the UI approval").not.toBe( - true, - ); - expect(textOf(resumed), "the gated tool completed after approval").toContain(policy.id); + expect(resumed.isError, "browser-mode resume completed after the UI approval").not.toBe( + true, + ); + expect(textOf(resumed), "the gated tool completed after approval").toContain(policy.id); + }); }).pipe(Effect.ensuring(closeQuietly(session))); }).pipe( Effect.ensuring( diff --git a/e2e/scenarios/graphql-introspection-health.test.ts b/e2e/scenarios/graphql-introspection-health.test.ts index b0ca487fb9..aaa48979ec 100644 --- a/e2e/scenarios/graphql-introspection-health.test.ts +++ b/e2e/scenarios/graphql-introspection-health.test.ts @@ -1,14 +1,13 @@ import { randomBytes } from "node:crypto"; +import { createServer } from "node:http"; import { expect } from "@effect/vitest"; import { Effect } from "effect"; import { composePluginApi } from "@executor-js/api/server"; -import { connectEmulator } from "@executor-js/emulate"; import { graphqlHttpPlugin } from "@executor-js/plugin-graphql/api"; import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared"; import { variable } from "@executor-js/sdk/http-auth"; -import { createEmulatorInstance } from "../src/emulator-instance"; import { scenario } from "../src/scenario"; import { Api, Browser, Target } from "../src/services"; import { visit } from "../src/surfaces/browser"; @@ -16,125 +15,132 @@ import { visit } from "../src/surfaces/browser"; const api = composePluginApi([graphqlHttpPlugin()] as const); const unique = (prefix: string): string => `${prefix}_${randomBytes(4).toString("hex")}`; +const serveRejectingGraphql = () => + Effect.acquireRelease( + Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => { + const server = createServer((request, response) => { + if (request.method === "POST" && request.url === "/graphql") { + response.writeHead(401, { "content-type": "application/json" }); + response.end(JSON.stringify({ message: "Bad credentials" })); + return; + } + response.writeHead(404, { "content-type": "application/json" }); + response.end(JSON.stringify({ message: "Not found" })); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resume( + Effect.succeed({ + url: `http://127.0.0.1:${port}`, + close: () => { + server.close(); + server.closeAllConnections(); + }, + }), + ); + }); + }), + (server) => Effect.sync(server.close), + ); + scenario( "GraphQL · failed introspection blocks connection creation with an actionable error", {}, - Effect.gen(function* () { - const target = yield* Target; - const browser = yield* Browser; - const { client: makeApiClient } = yield* Api; - const identity = yield* target.newIdentity(); - const client = yield* makeApiClient(api, identity); - const slug = unique("graphql_health"); - const emulatorBaseUrl = yield* createEmulatorInstance("github", "graphql-health"); - const emulator = yield* Effect.promise(() => - connectEmulator({ baseUrl: emulatorBaseUrl, service: "github" }), - ); + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeApiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + const slug = unique("graphql_health"); + const upstream = yield* serveRejectingGraphql(); - // A budget, not a count: nothing in this scenario depends on how many - // times the connect flow introspects, and the emulator's answer when the - // budget runs out is not a neutral pass-through — an unauthenticated - // GraphQL POST to the real handler is GitHub-shaped, so it comes back 403 - // "API rate limit exceeded". The UI then honestly reports HTTP 403 and the - // assertion below fails on a message that has nothing to do with the - // product. Arm enough that one connect attempt cannot exhaust it. - yield* Effect.promise(() => - emulator.faults.arm({ - match: { method: "POST", pathPattern: "/graphql" }, - response: { status: 401, body: { message: "Bad credentials" } }, - times: 100, - }), - ); + yield* client.graphql.addIntegration({ + payload: { + endpoint: `${upstream.url}/graphql`, + slug, + name: "GraphQL health", + authenticationTemplate: [ + { + slug: "header", + type: "apiKey", + headers: { Authorization: [variable("token")] }, + }, + ], + }, + }); - yield* client.graphql.addIntegration({ - payload: { - endpoint: `${emulatorBaseUrl}/graphql`, - slug, - name: "GraphQL health", - authenticationTemplate: [ - { - slug: "header", - type: "apiKey", - headers: { Authorization: [variable("token")] }, - }, - ], - }, - }); + yield* Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the connection flow", async () => { + await visit(page, `/integrations/${slug}?addAccount=1&owner=org&template=header`); + await page.getByRole("heading", { name: /Add connection · GraphQL health/ }).waitFor(); + }); - yield* Effect.gen(function* () { - yield* browser.session(identity, async ({ page, step }) => { - await step("Open the connection flow", async () => { - await visit(page, `/integrations/${slug}?addAccount=1&owner=org&template=header`); - await page.getByRole("heading", { name: /Add connection · GraphQL health/ }).waitFor(); - }); + await step("Submit a credential rejected during schema introspection", async () => { + const dialog = page.getByRole("dialog", { + name: /Add connection · GraphQL health/, + }); + await dialog.getByRole("textbox", { name: "Authorization" }).fill("invalid-token"); + await dialog.getByRole("button", { name: "Continue" }).click(); - await step("Submit a credential rejected during schema introspection", async () => { - const dialog = page.getByRole("dialog", { - name: /Add connection · GraphQL health/, + const alert = dialog.getByRole("alert"); + await alert.waitFor(); + const message = await alert.textContent(); + expect(message).toContain("The endpoint rejected the credential with HTTP 401."); + expect(message).toContain("Check the credential and selected authentication method."); + await dialog.getByText("Step 1 of 2").waitFor(); + expect( + await page.getByText("No connections yet").count(), + "the rejected credential is not saved", + ).toBe(1); }); - await dialog.getByRole("textbox", { name: "Authorization" }).fill("invalid-token"); - await dialog.getByRole("button", { name: "Continue" }).click(); - - const alert = dialog.getByRole("alert"); - await alert.waitFor(); - const message = await alert.textContent(); - expect(message).toContain("The endpoint rejected the credential with HTTP 401."); - expect(message).toContain("Check the credential and selected authentication method."); - await dialog.getByText("Step 1 of 2").waitFor(); - expect( - await page.getByText("No connections yet").count(), - "the rejected credential is not saved", - ).toBe(1); }); - }); - - // The low-level API can still import an existing credential reference - // without the browser's preflight. This models connections created before - // the fix and proves their failed tool sync is no longer a silent zero. - yield* client.connections.create({ - payload: { - owner: "org", - name: ConnectionName.make("legacy"), - integration: IntegrationSlug.make(slug), - template: AuthTemplateSlug.make("header"), - value: "invalid-token", - }, - }); - yield* browser.session(identity, async ({ page, step }) => { - await step("A failed existing connection explains the empty tool catalogue", async () => { - await visit(page, `/integrations/${slug}?tab=tools`); - await page.getByText("Connection rejected", { exact: true }).first().waitFor(); - await page - .getByText("The endpoint rejected the credential with HTTP 401.", { - exact: false, - }) - .waitFor(); - await page.getByRole("button", { name: "Check and sync tools" }).waitFor(); + // The low-level API can still import an existing credential reference + // without the browser's preflight. This models connections created before + // the fix and proves their failed tool sync is no longer a silent zero. + yield* client.connections.create({ + payload: { + owner: "org", + name: ConnectionName.make("legacy"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("header"), + value: "invalid-token", + }, }); - await step("The account row carries the same actionable health verdict", async () => { - await page.getByRole("tab", { name: "Accounts" }).click(); - await page.getByText("Expired", { exact: true }).waitFor(); - await page - .getByText("Check the credential and selected authentication method.", { - exact: false, - }) - .waitFor(); + yield* browser.session(identity, async ({ page, step }) => { + await step("A failed existing connection explains the empty tool catalogue", async () => { + await visit(page, `/integrations/${slug}?tab=tools`); + await page.getByText("Connection rejected", { exact: true }).first().waitFor(); + await page + .getByText("The endpoint rejected the credential with HTTP 401.", { + exact: false, + }) + .waitFor(); + await page.getByRole("button", { name: "Check and sync tools" }).waitFor(); + }); + + await step("The account row carries the same actionable health verdict", async () => { + await page.getByRole("tab", { name: "Accounts" }).click(); + await page.getByText("Expired", { exact: true }).waitFor(); + await page + .getByText("Check the credential and selected authentication method.", { + exact: false, + }) + .waitFor(); + }); }); - }); - }).pipe( - Effect.ensuring( - Effect.all( - [ - client.integrations - .remove({ params: { slug: IntegrationSlug.make(slug) } }) - .pipe(Effect.ignore), - Effect.promise(() => emulator.faults.clear()).pipe(Effect.ignore), - ], - { concurrency: "unbounded" }, + }).pipe( + Effect.ensuring( + client.integrations + .remove({ params: { slug: IntegrationSlug.make(slug) } }) + .pipe(Effect.ignore), ), - ), - ); - }), + ); + }), + ), ); diff --git a/e2e/scenarios/policies-ui.test.ts b/e2e/scenarios/policies-ui.test.ts index 6e42865ea2..ad45864429 100644 --- a/e2e/scenarios/policies-ui.test.ts +++ b/e2e/scenarios/policies-ui.test.ts @@ -27,7 +27,7 @@ import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/ import { scenario } from "../src/scenario"; import { Api, Browser, Target } from "../src/services"; -import { visit } from "../src/surfaces/browser"; +import { clickToReveal, visit } from "../src/surfaces/browser"; const api = composePluginApi([openApiHttpPlugin()] as const); @@ -163,9 +163,15 @@ scenario( await step("Open the integration's Tools tab", async () => { await visit(page, `/integrations/${integration}`); - await page.getByRole("tab", { name: "Tools" }).click(); - await sectionFor(alpha).waitFor(); - await sectionFor(beta).waitFor(); + // The org-scoped redirect can replace the document between the tab + // becoming visible and React receiving the click. Reveal a node that + // exists only in the Tools panel so the Accounts panel's connection + // sections cannot satisfy the readiness check. + await clickToReveal( + page.getByRole("tab", { name: "Tools" }), + closedGroup(alpha, integration), + ); + await closedGroup(beta, integration).waitFor(); }); await step("Expand the records category in the first account", async () => { diff --git a/e2e/setup/mcp-session-timeouts.ts b/e2e/setup/mcp-session-timeouts.ts index dfd4782978..3cf2846435 100644 --- a/e2e/setup/mcp-session-timeouts.ts +++ b/e2e/setup/mcp-session-timeouts.ts @@ -1,5 +1,7 @@ const DEFAULT_E2E_MCP_SESSION_TIMEOUT_MS = 3_000; -const DEFAULT_E2E_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS = 6_000; +// Keep expiry coverage fast while leaving enough room for a cold browser route +// to compile under CI contention. Production allows nine minutes. +const DEFAULT_E2E_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS = 30_000; const PRODUCTION_MCP_SESSION_TIMEOUT_MS = 5 * 60 * 1000; const PRODUCTION_MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS = 9 * 60 * 1000;