Skip to content

Commit 7c5181b

Browse files
RhysSullivanRhysSullivan
committed
Add e2e repro for MCP OAuth health-check 500 and reconnect modal close
1 parent 68da8c2 commit 7c5181b

3 files changed

Lines changed: 323 additions & 5 deletions

File tree

‎e2e/selfhost/mcp-oauth-reconnect-health.test.ts‎

Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
// Selfhost repros for two MCP OAuth bugs seen with a DCR connection whose
2+
// refresh token is rejected by the provider as `invalid_grant`.
3+
import { randomBytes } from "node:crypto";
4+
5+
import { Cause, Effect, Exit } from "effect";
6+
import { expect } from "@effect/vitest";
7+
import type { HttpApiClient } from "effect/unstable/httpapi";
8+
import type { Page } from "playwright";
9+
import { composePluginApi } from "@executor-js/api/server";
10+
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
11+
import {
12+
AuthTemplateSlug,
13+
ConnectionName,
14+
IntegrationSlug,
15+
OAuthClientSlug,
16+
} from "@executor-js/sdk/shared";
17+
import { serveOAuthTestServer, type OAuthTestServerShape } from "@executor-js/sdk/testing";
18+
19+
import { scenario } from "../src/scenario";
20+
import { Api, Browser, Target } from "../src/services";
21+
22+
const api = composePluginApi([mcpHttpPlugin()] as const);
23+
type Client = HttpApiClient.ForApi<typeof api>;
24+
25+
const name = ConnectionName.make("main");
26+
const template = AuthTemplateSlug.make("oauth2");
27+
28+
const freshSlug = (prefix: string): string => `${prefix}-${randomBytes(4).toString("hex")}`;
29+
30+
const healthPath = (slug: IntegrationSlug): string =>
31+
`/api/connections/org/${String(slug)}/${String(name)}/health`;
32+
33+
const oauthReconnectRequest = (url: string): boolean =>
34+
url.includes("/api/oauth/probe") ||
35+
url.includes("/api/oauth/start") ||
36+
url.includes("/api/oauth/clients/register-dynamic");
37+
38+
const observedFailure = (cause: Cause.Cause<unknown>): string => String(Cause.squash(cause));
39+
40+
const connectionsSection = (page: Page) =>
41+
page.locator("section").filter({
42+
has: page.getByRole("heading", { level: 3, name: "Connections" }),
43+
});
44+
45+
const requiredRedirect = (response: Response, from: string): string => {
46+
const location = response.headers.get("location");
47+
if (!location) {
48+
throw new Error(`Expected redirect from ${from}, got HTTP ${response.status}`);
49+
}
50+
return new URL(location, from).toString();
51+
};
52+
53+
const completeAuthorization = (authorizationUrl: string) =>
54+
Effect.promise(async () => {
55+
const login = await fetch(authorizationUrl, { redirect: "manual" });
56+
const loginUrl = requiredRedirect(login, authorizationUrl);
57+
const credentials = Buffer.from("alice:password").toString("base64");
58+
const callback = await fetch(loginUrl, {
59+
method: "POST",
60+
headers: { authorization: `Basic ${credentials}` },
61+
redirect: "manual",
62+
});
63+
const callbackUrl = requiredRedirect(callback, loginUrl);
64+
const parsed = new URL(callbackUrl);
65+
const code = parsed.searchParams.get("code");
66+
if (!code) throw new Error(`OAuth callback did not include a code: ${callbackUrl}`);
67+
return { code };
68+
});
69+
70+
const seedExpiredDcrMcpOAuthConnection = (client: Client, prefix: string) =>
71+
Effect.gen(function* () {
72+
const oauth = yield* serveOAuthTestServer({
73+
scopes: ["channels:history", "users:read"],
74+
supportRefresh: false,
75+
tokenExpiresInSeconds: 0,
76+
invalidRefreshTokenDescription: "Grant not found",
77+
});
78+
const slug = IntegrationSlug.make(freshSlug(prefix));
79+
const clientSlug = OAuthClientSlug.make(freshSlug(`${prefix}-client`));
80+
81+
yield* client.mcp.addServer({
82+
payload: {
83+
transport: "remote",
84+
name: `OAuth repro ${String(slug)}`,
85+
endpoint: oauth.mcpResourceUrl,
86+
slug: String(slug),
87+
authenticationTemplate: [{ kind: "oauth2" }],
88+
},
89+
});
90+
yield* Effect.addFinalizer(() =>
91+
client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore),
92+
);
93+
94+
const probe = yield* client.oauth.probe({ payload: { url: oauth.mcpResourceUrl } });
95+
if (!probe.registrationEndpoint) {
96+
return yield* Effect.die("OAuth probe did not discover a DCR registration endpoint");
97+
}
98+
99+
const registered = yield* client.oauth.registerDynamic({
100+
payload: {
101+
owner: "org",
102+
slug: clientSlug,
103+
issuer: probe.issuer ?? null,
104+
registrationEndpoint: probe.registrationEndpoint,
105+
authorizationUrl: probe.authorizationUrl,
106+
tokenUrl: probe.tokenUrl,
107+
resource: probe.resource ?? oauth.mcpResourceUrl,
108+
scopes: probe.scopesSupported ?? [],
109+
tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported,
110+
clientName: "Executor e2e MCP OAuth repro",
111+
originIntegration: slug,
112+
},
113+
});
114+
yield* Effect.addFinalizer(() =>
115+
client.oauth
116+
.removeClient({ params: { slug: registered.client }, payload: { owner: "org" } })
117+
.pipe(Effect.ignore),
118+
);
119+
120+
const started = yield* client.oauth.start({
121+
payload: {
122+
owner: "org",
123+
client: registered.client,
124+
clientOwner: "org",
125+
name,
126+
integration: slug,
127+
template,
128+
},
129+
});
130+
expect(started.status, "DCR MCP OAuth starts an authorization-code redirect").toBe("redirect");
131+
if (started.status !== "redirect") return yield* Effect.die("OAuth start did not redirect");
132+
133+
const callback = yield* completeAuthorization(started.authorizationUrl);
134+
yield* client.oauth.complete({ payload: { state: started.state, code: callback.code } });
135+
yield* Effect.addFinalizer(() =>
136+
client.connections
137+
.remove({ params: { owner: "org", integration: slug, name } })
138+
.pipe(Effect.ignore),
139+
);
140+
yield* oauth.clearRequests;
141+
142+
return { oauth, slug };
143+
});
144+
145+
const logTokenRequests = (label: string, oauth: OAuthTestServerShape) =>
146+
Effect.gen(function* () {
147+
const requests = yield* oauth.requests;
148+
const refresh = requests
149+
.filter((request) => request.path === "/token" && request.body.includes("refresh_token"))
150+
.map((request) => `${request.method} ${request.path} ${request.body}`);
151+
console.info(`[BUG repro] ${label}: refresh token requests: ${refresh.join(" | ") || "none"}`);
152+
});
153+
154+
scenario(
155+
"MCP OAuth · invalid_grant refresh during health check returns expired instead of 500",
156+
{
157+
timeout: 180_000,
158+
expectedFailure:
159+
"BUG: invalid_grant during pre-health token refresh is folded into InternalError/500.",
160+
},
161+
Effect.scoped(
162+
Effect.gen(function* () {
163+
const target = yield* Target;
164+
const browser = yield* Browser;
165+
const { client: makeApiClient } = yield* Api;
166+
const identity = yield* target.newIdentity();
167+
const client = yield* makeApiClient(api, identity);
168+
const { oauth, slug } = yield* seedExpiredDcrMcpOAuthConnection(client, "mcp-hc-invalid");
169+
170+
const apiExit = yield* Effect.exit(
171+
client.connections.checkHealth({
172+
params: { owner: "org", integration: slug, name },
173+
query: {},
174+
}),
175+
);
176+
if (Exit.isFailure(apiExit)) {
177+
console.info(`[BUG repro] typed checkHealth failed: ${observedFailure(apiExit.cause)}`);
178+
} else {
179+
console.info(`[BUG repro] typed checkHealth status: ${apiExit.value.status}`);
180+
}
181+
yield* logTokenRequests("typed checkHealth", oauth);
182+
yield* oauth.clearRequests;
183+
184+
yield* browser.session(identity, async ({ page, step }) => {
185+
const connections = connectionsSection(page);
186+
const menuTrigger = connections.locator('button[aria-haspopup="menu"]').first();
187+
188+
await step("Open the MCP integration with its expired OAuth connection", async () => {
189+
await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
190+
await connections.getByText("main", { exact: true }).waitFor({ timeout: 30_000 });
191+
});
192+
193+
await step(
194+
"Check now should render Expired without the generic failure toast",
195+
async () => {
196+
const responsePromise = page.waitForResponse(
197+
(response) =>
198+
response.url().includes(healthPath(slug)) && response.request().method() === "POST",
199+
{ timeout: 30_000 },
200+
);
201+
await menuTrigger.click();
202+
await page.getByRole("menuitem", { name: "Check now" }).click();
203+
const response = await responsePromise;
204+
const body = await response.text();
205+
console.info(`[BUG repro] UI health response: ${response.status()} ${body}`);
206+
207+
expect(
208+
response.status(),
209+
`health check should return HTTP 200 with status expired; body: ${body}`,
210+
).toBe(200);
211+
const json = JSON.parse(body) as { readonly status?: string };
212+
expect(json.status, "unrefreshable OAuth grants are an expired credential").toBe(
213+
"expired",
214+
);
215+
await connections.getByLabel("Status: Expired").waitFor({ timeout: 30_000 });
216+
await page.getByText("Health check failed", { exact: true }).waitFor({
217+
state: "hidden",
218+
timeout: 5_000,
219+
});
220+
},
221+
);
222+
});
223+
}),
224+
),
225+
);
226+
227+
scenario(
228+
"MCP OAuth · DCR reconnect keeps the dialog open and reaches OAuth start",
229+
{
230+
timeout: 180_000,
231+
expectedFailure:
232+
"BUG: DCR reconnect opens AddAccountModal without oauthClient handoff, then closes.",
233+
},
234+
Effect.scoped(
235+
Effect.gen(function* () {
236+
const target = yield* Target;
237+
const browser = yield* Browser;
238+
const { client: makeApiClient } = yield* Api;
239+
const identity = yield* target.newIdentity();
240+
const client = yield* makeApiClient(api, identity);
241+
const { slug } = yield* seedExpiredDcrMcpOAuthConnection(client, "mcp-dcr-reconnect");
242+
243+
yield* browser.session(identity, async ({ page, step }) => {
244+
const connections = connectionsSection(page);
245+
const menuTrigger = connections.locator('button[aria-haspopup="menu"]').first();
246+
const dialog = page.getByRole("dialog");
247+
const oauthRequests: string[] = [];
248+
249+
page.on("request", (request) => {
250+
if (oauthReconnectRequest(request.url())) oauthRequests.push(request.url());
251+
});
252+
253+
await step("Open the MCP integration with its DCR OAuth connection", async () => {
254+
await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
255+
await connections.getByText("main", { exact: true }).waitFor({ timeout: 30_000 });
256+
});
257+
258+
await step("Reconnect should keep a dialog visible or reach OAuth", async () => {
259+
const oauthRequest = page
260+
.waitForRequest((request) => oauthReconnectRequest(request.url()), { timeout: 5_000 })
261+
.then((request) => request.url())
262+
.catch(() => null);
263+
264+
await menuTrigger.click();
265+
await page.getByRole("menuitem", { name: "Reconnect" }).click();
266+
267+
const sawDialog = await dialog
268+
.waitFor({ state: "visible", timeout: 1_000 })
269+
.then(() => true)
270+
.catch(() => false);
271+
272+
let reachedOAuth: string | null = null;
273+
let closedWithinTwoSeconds: boolean | null = null;
274+
if (!sawDialog) {
275+
reachedOAuth = await oauthRequest;
276+
console.info(
277+
`[BUG repro] reconnect dialog was not observed; OAuth request: ${
278+
reachedOAuth ?? "none"
279+
}`,
280+
);
281+
} else {
282+
closedWithinTwoSeconds = await dialog
283+
.waitFor({ state: "hidden", timeout: 2_000 })
284+
.then(() => true)
285+
.catch(() => false);
286+
reachedOAuth = await oauthRequest;
287+
console.info(
288+
`[BUG repro] reconnect dialog closedWithinTwoSeconds=${closedWithinTwoSeconds}; OAuth requests: ${
289+
oauthRequests.join(", ") || reachedOAuth || "none"
290+
}`,
291+
);
292+
}
293+
294+
const outcome =
295+
sawDialog && closedWithinTwoSeconds === false
296+
? "dialog-persisted"
297+
: reachedOAuth !== null
298+
? "oauth-reached"
299+
: "missing";
300+
expect(
301+
outcome,
302+
"Reconnect should keep the dialog visible or reach OAuth within 5s",
303+
).not.toBe("missing");
304+
});
305+
});
306+
}),
307+
),
308+
);

‎e2e/src/scenario.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export interface ScenarioOptions {
5858
* body never runs. Use ONLY for a scenario blocked on a tracked, out-of-scope
5959
* issue; state the reason here so the skip is self-documenting in the source. */
6060
readonly skip?: string;
61+
/** When set, the scenario is registered as a Vitest expected failure. The body
62+
* still runs and records artifacts; Vitest keeps CI green only while the
63+
* tracked bug still reproduces. */
64+
readonly expectedFailure?: string;
6165
}
6266

6367
type AllServices =
@@ -129,8 +133,9 @@ export const scenario = (
129133
const dir = join(RUNS_DIR, target.name, slugify(name));
130134
const context = contextFor(target, dir);
131135
const testFile = captureTestFile();
136+
const register = options.expectedFailure ? it.live.fails : it.live;
132137

133-
it.live(
138+
register(
134139
name,
135140
(testCtx) =>
136141
Effect.gen(function* () {

‎packages/core/sdk/src/testing/oauth-test-server.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export interface OAuthTestServerOptions {
5353
readonly scopes?: readonly string[];
5454
readonly omitTokenResponseScopes?: readonly string[];
5555
readonly supportRefresh?: boolean;
56+
readonly tokenExpiresInSeconds?: number;
57+
readonly invalidRefreshTokenDescription?: string;
5658
/** Gate Dynamic Client Registration on the requested redirect URIs. When set,
5759
* `/register` returns `400 invalid_redirect_uri` unless every requested
5860
* `redirect_uris` entry is approved. Mirrors authorization servers (e.g.
@@ -412,6 +414,9 @@ export const serveOAuthTestServer = (
412414
...(options.users ?? {}),
413415
};
414416
const supportRefresh = options.supportRefresh ?? true;
417+
const tokenExpiresInSeconds = options.tokenExpiresInSeconds ?? 3600;
418+
const invalidRefreshTokenDescription =
419+
options.invalidRefreshTokenDescription ?? "Unknown refresh token";
415420
const scopes = options.scopes ?? defaultScopes;
416421
const omittedTokenResponseScopes = new Set(options.omitTokenResponseScopes ?? []);
417422
const tokenResponseScope = (scope: string | null): string | undefined => {
@@ -645,7 +650,7 @@ export const serveOAuthTestServer = (
645650
access_token: accessToken,
646651
refresh_token: refreshToken,
647652
token_type: "Bearer",
648-
expires_in: 3600,
653+
expires_in: tokenExpiresInSeconds,
649654
...(scope ? { scope } : {}),
650655
},
651656
{ "cache-control": "no-store" },
@@ -656,7 +661,7 @@ export const serveOAuthTestServer = (
656661
const refreshToken = params.get("refresh_token");
657662
const record = refreshToken ? refreshTokens.get(refreshToken) : undefined;
658663
if (!supportRefresh || !refreshToken || !record || record.clientId !== clientId) {
659-
return oauthError(400, "invalid_grant", "Unknown refresh token");
664+
return oauthError(400, "invalid_grant", invalidRefreshTokenDescription);
660665
}
661666
const nextAccessToken = `at_${randomUUID()}`;
662667
const nextRefreshToken = `rt_${randomUUID()}`;
@@ -673,7 +678,7 @@ export const serveOAuthTestServer = (
673678
access_token: nextAccessToken,
674679
refresh_token: nextRefreshToken,
675680
token_type: "Bearer",
676-
expires_in: 3600,
681+
expires_in: tokenExpiresInSeconds,
677682
...(scope ? { scope } : {}),
678683
},
679684
{ "cache-control": "no-store" },
@@ -688,7 +693,7 @@ export const serveOAuthTestServer = (
688693
{
689694
access_token: accessToken,
690695
token_type: "Bearer",
691-
expires_in: 3600,
696+
expires_in: tokenExpiresInSeconds,
692697
scope: params.get("scope") ?? scopes.join(" "),
693698
},
694699
{ "cache-control": "no-store" },

0 commit comments

Comments
 (0)