test: synchronize OAuth connect popup and simplify coverage

e2e/mcp/oauth-connect.test.ts

@@ -4,16 +4,16 @@
 // credential breaks discovery. The fake server and its authorization server
 // are one Bun.serve on loopback.
 import { afterAll, expect, test } from "bun:test";
+import { type } from "arktype";
 import { bootBrowserApp, browserGate } from "../lib/browser";
-import { runFirstRunFlow, STEP_TIMEOUT } from "../lib/first-run";
+import { clickText, runFirstRunFlow, STEP_TIMEOUT, waitForText } from "../lib/first-run";
 
 const describeBrowser = browserGate(import.meta.path);
 
 const ACCESS_TOKEN = "acme-access-token";
 
 function startFakeMcpServer() {
   let origin = "";
-  const seen = { authorizedCalls: 0, unauthorizedCalls: 0 };
   const json = (body: unknown, status = 200) =>
     new Response(JSON.stringify(body), {
       status,
@@ -59,10 +59,8 @@ function startFakeMcpServer() {
       }
       if (url.pathname === "/mcp" && req.method === "POST") {
         if (req.headers.get("authorization") !== `Bearer ${ACCESS_TOKEN}`) {
-          seen.unauthorizedCalls += 1;
           return new Response("unauthorized", { status: 401 });
         }
-        seen.authorizedCalls += 1;
         const rpc = (await req.json()) as { id?: number; method: string };
         if (rpc.id === undefined) return new Response(null, { status: 202 });
         if (rpc.method === "initialize") {
@@ -97,7 +95,7 @@ function startFakeMcpServer() {
     },
   });
   origin = `http://127.0.0.1:${String(server.port)}`;
-  return { server, origin, seen };
+  return { server, origin };
 }
 
 describeBrowser("oauth mcp connect", () => {
@@ -111,64 +109,87 @@ describeBrowser("oauth mcp connect", () => {
     const { page, errors } = await app().newPage();
     try {
       await runFirstRunFlow(page, app().origin);
-      // The catalog lives on the workspace tenant; its id shows up in the page's own reads.
-      let workspaceId = "";
-      page.on("request", (request) => {
-        const match = /\/api\/tenants\/([^/]+)\/credentials$/.exec(new URL(request.url()).pathname);
-        if (match?.[1] !== undefined) workspaceId = decodeURIComponent(match[1]);
-      });
-      await page.goto(`${app().origin}/tools`, { waitUntil: "networkidle0" });
+      // Capture the workspace catalog read once, rather than tracking unrelated later requests.
+      const [credentialsRequest] = await Promise.all([
+        page.waitForRequest(
+          (request) =>
+            request.method() === "GET" &&
+            /\/api\/tenants\/[^/]+\/credentials$/.test(new URL(request.url()).pathname),
+          { timeout: STEP_TIMEOUT },
+        ),
+        page.goto(`${app().origin}/tools`, { waitUntil: "domcontentloaded" }),
+      ]);
+      const credentialsPath = new URL(credentialsRequest.url()).pathname;
       await page.waitForSelector("input[aria-label='Server URL']", { timeout: STEP_TIMEOUT });
       await page.type("input[aria-label='Server URL']", `${fake.origin}/mcp`);
-      // The catalog tiles have Sign in buttons too; this one sits in the add-by-URL card.
-      await page.evaluate(`(() => {
-        const input = document.querySelector("input[aria-label='Server URL']");
-        const button = Array.from(input.parentElement.querySelectorAll("button")).find(
-          (b) => b.textContent.trim() === "Sign in",
-        );
-        button.click();
-      })()`);
-      // The sign-in tab takes the foreground, which pauses rAF polling here.
-      // The tile appears once the catalog write and the Worker redeploy settle.
+      // Register before clicking: the fake provider redirects immediately. With noopener,
+      // use the callback URL rather than relying on the popup's opener relationship.
+      const [callback] = await Promise.all([
+        page.browserContext().waitForTarget(
+          (target) => {
+            if (!URL.canParse(target.url())) return false;
+            const url = new URL(target.url());
+            return (
+              url.protocol === "http:" &&
+              ["localhost", "127.0.0.1"].includes(url.hostname) &&
+              url.port === new URL(app().origin).port &&
+              url.pathname === "/api/oauth/callback"
+            );
+          },
+          { timeout: STEP_TIMEOUT },
+        ),
+        clickText(page, ".tool-add-actions button", "Sign in"),
+      ]);
+      const popup = await callback.page();
+      if (popup === null) throw new Error("OAuth callback did not open a page");
+      await waitForText(popup, "Signed in. You can close this tab.");
+      await popup.close();
+      // Model returning to Tools: its sign-in query does not poll while backgrounded.
+      await page.bringToFront();
       await page.waitForFunction(`document.body.innerText.includes("1 tools live")`, {
         timeout: 120_000,
         polling: 500,
       });
 
       // Runs in the page so the signed-in session cookie applies.
-      const result = (await page.evaluate(`(async () => {
-        const workspace = ${JSON.stringify(workspaceId)};
-        const creds = await (await fetch("/api/tenants/" + workspace + "/credentials")).json();
-        const row = creds.data.find((c) => c.metadata?.mcp?.url?.endsWith("/mcp"));
-        const discover = () => fetch("/api/tenants/" + workspace + "/mcp/discover", {
+      const result = type({
+        auth: "string",
+        tools: "string[]",
+        before: "number",
+        deleted: "boolean",
+        after: "number",
+      }).assert(
+        await page.evaluate(`(async () => {
+        const credentials = ${JSON.stringify(credentialsPath)};
+        const response = await fetch(credentials);
+        if (!response.ok) throw new Error("Listing credentials failed");
+        const creds = await response.json();
+        const row = creds.data.find((c) => c.metadata?.mcp?.url === ${JSON.stringify(`${fake.origin}/mcp`)});
+        if (!row) throw new Error("Connected MCP credential not found");
+        const discover = () => fetch(credentials.replace(/credentials$/, "mcp/discover"), {
           method: "POST",
           headers: { "content-type": "application/json" },
           body: JSON.stringify({ url: row.metadata.mcp.url, credentialId: row.id }),
         }).then((r) => r.status);
         const before = await discover();
-        await fetch("/api/tenants/" + workspace + "/credentials/" + row.id, { method: "DELETE" });
+        const deleted = (await fetch(credentials + "/" + row.id, { method: "DELETE" })).ok;
         const after = await discover();
-        return { type: row.type, meta: row.metadata, before, after };
-      })()`)) as {
-        type: string;
-        meta: { mcp: { auth: string; tools: { name: string }[] }; oauthClientId?: string };
-        before: number;
-        after: number;
-      };
+        return { auth: row.metadata.mcp.auth, tools: row.metadata.mcp.tools.map((t) => t.name), before, deleted, after };
+      })()`),
+      );
 
-      expect(result.type).toBe("oauth_token");
-      expect(result.meta.mcp.auth).toBe("oauth");
-      expect(result.meta.mcp.tools.map((t) => t.name)).toEqual(["acme.list_things"]);
-      expect(result.meta.oauthClientId).toBe("acme-client");
+      expect(result.auth).toBe("oauth");
+      expect(result.tools).toEqual(["acme.list_things"]);
       expect(result.before).toBe(200);
-      expect(result.after).not.toBe(200);
-      expect(fake.seen.authorizedCalls).toBeGreaterThan(0);
+      expect(result.deleted).toBe(true);
+      expect(result.after).toBe(404);
     } catch (cause) {
       process.stderr.write(
         `oauth-connect failed at ${page.url()}\nconsole: ${errors.join(" | ")}\n`,
       );
       throw cause;
+    } finally {
+      await page.close();
     }
-    await page.close();
   }, 400_000);
 });