* Add e2e repro for MCP OAuth health-check 500 and reconnect modal close * Fix MCP OAuth health and reconnect flows
Unified · Split
| @@ -0,0 +1,278 @@ | ||
| 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 { Effect } 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 connectionsSection = (page: Page) => | |
| 39 | + page.locator("section").filter({ | |
| 40 | + has: page.getByRole("heading", { level: 3, name: "Connections" }), | |
| 41 | + }); | |
| 42 | + | |
| 43 | +const requiredRedirect = (response: Response, from: string): string => { | |
| 44 | + const location = response.headers.get("location"); | |
| 45 | + if (!location) { | |
| 46 | + throw new Error(`Expected redirect from ${from}, got HTTP ${response.status}`); | |
| 47 | + } | |
| 48 | + return new URL(location, from).toString(); | |
| 49 | +}; | |
| 50 | + | |
| 51 | +const completeAuthorization = (authorizationUrl: string) => | |
| 52 | + Effect.promise(async () => { | |
| 53 | + const login = await fetch(authorizationUrl, { redirect: "manual" }); | |
| 54 | + const loginUrl = requiredRedirect(login, authorizationUrl); | |
| 55 | + const credentials = Buffer.from("alice:password").toString("base64"); | |
| 56 | + const callback = await fetch(loginUrl, { | |
| 57 | + method: "POST", | |
| 58 | + headers: { authorization: `Basic ${credentials}` }, | |
| 59 | + redirect: "manual", | |
| 60 | + }); | |
| 61 | + const callbackUrl = requiredRedirect(callback, loginUrl); | |
| 62 | + const parsed = new URL(callbackUrl); | |
| 63 | + const code = parsed.searchParams.get("code"); | |
| 64 | + if (!code) throw new Error(`OAuth callback did not include a code: ${callbackUrl}`); | |
| 65 | + return { code }; | |
| 66 | + }); | |
| 67 | + | |
| 68 | +const seedExpiredDcrMcpOAuthConnection = (client: Client, prefix: string) => | |
| 69 | + Effect.gen(function* () { | |
| 70 | + const oauth = yield* serveOAuthTestServer({ | |
| 71 | + scopes: ["channels:history", "users:read"], | |
| 72 | + supportRefresh: false, | |
| 73 | + tokenExpiresInSeconds: 0, | |
| 74 | + invalidRefreshTokenDescription: "Grant not found", | |
| 75 | + }); | |
| 76 | + const slug = IntegrationSlug.make(freshSlug(prefix)); | |
| 77 | + const clientSlug = OAuthClientSlug.make(freshSlug(`${prefix}-client`)); | |
| 78 | + | |
| 79 | + yield* client.mcp.addServer({ | |
| 80 | + payload: { | |
| 81 | + transport: "remote", | |
| 82 | + name: `OAuth repro ${String(slug)}`, | |
| 83 | + endpoint: oauth.mcpResourceUrl, | |
| 84 | + slug: String(slug), | |
| 85 | + authenticationTemplate: [{ kind: "oauth2" }], | |
| 86 | + }, | |
| 87 | + }); | |
| 88 | + yield* Effect.addFinalizer(() => | |
| 89 | + client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore), | |
| 90 | + ); | |
| 91 | + | |
| 92 | + const probe = yield* client.oauth.probe({ payload: { url: oauth.mcpResourceUrl } }); | |
| 93 | + if (!probe.registrationEndpoint) { | |
| 94 | + return yield* Effect.die("OAuth probe did not discover a DCR registration endpoint"); | |
| 95 | + } | |
| 96 | + | |
| 97 | + const registered = yield* client.oauth.registerDynamic({ | |
| 98 | + payload: { | |
| 99 | + owner: "org", | |
| 100 | + slug: clientSlug, | |
| 101 | + issuer: probe.issuer ?? null, | |
| 102 | + registrationEndpoint: probe.registrationEndpoint, | |
| 103 | + authorizationUrl: probe.authorizationUrl, | |
| 104 | + tokenUrl: probe.tokenUrl, | |
| 105 | + resource: probe.resource ?? oauth.mcpResourceUrl, | |
| 106 | + scopes: probe.scopesSupported ?? [], | |
| 107 | + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, | |
| 108 | + clientName: "Executor e2e MCP OAuth repro", | |
| 109 | + originIntegration: slug, | |
| 110 | + }, | |
| 111 | + }); | |
| 112 | + yield* Effect.addFinalizer(() => | |
| 113 | + client.oauth | |
| 114 | + .removeClient({ params: { slug: registered.client }, payload: { owner: "org" } }) | |
| 115 | + .pipe(Effect.ignore), | |
| 116 | + ); | |
| 117 | + | |
| 118 | + const started = yield* client.oauth.start({ | |
| 119 | + payload: { | |
| 120 | + owner: "org", | |
| 121 | + client: registered.client, | |
| 122 | + clientOwner: "org", | |
| 123 | + name, | |
| 124 | + integration: slug, | |
| 125 | + template, | |
| 126 | + }, | |
| 127 | + }); | |
| 128 | + expect(started.status, "DCR MCP OAuth starts an authorization-code redirect").toBe("redirect"); | |
| 129 | + if (started.status !== "redirect") return yield* Effect.die("OAuth start did not redirect"); | |
| 130 | + | |
| 131 | + const callback = yield* completeAuthorization(started.authorizationUrl); | |
| 132 | + yield* client.oauth.complete({ payload: { state: started.state, code: callback.code } }); | |
| 133 | + yield* Effect.addFinalizer(() => | |
| 134 | + client.connections | |
| 135 | + .remove({ params: { owner: "org", integration: slug, name } }) | |
| 136 | + .pipe(Effect.ignore), | |
| 137 | + ); | |
| 138 | + yield* oauth.clearRequests; | |
| 139 | + | |
| 140 | + return { oauth, slug }; | |
| 141 | + }); | |
| 142 | + | |
| 143 | +const logTokenRequests = (label: string, oauth: OAuthTestServerShape) => | |
| 144 | + Effect.gen(function* () { | |
| 145 | + const requests = yield* oauth.requests; | |
| 146 | + const refresh = requests | |
| 147 | + .filter((request) => request.path === "/token" && request.body.includes("refresh_token")) | |
| 148 | + .map((request) => `${request.method} ${request.path} ${request.body}`); | |
| 149 | + console.info(`[BUG repro] ${label}: refresh token requests: ${refresh.join(" | ") || "none"}`); | |
| 150 | + }); | |
| 151 | + | |
| 152 | +scenario( | |
| 153 | + "MCP OAuth · invalid_grant refresh during health check returns expired instead of 500", | |
| 154 | + { | |
| 155 | + timeout: 180_000, | |
| 156 | + }, | |
| 157 | + Effect.scoped( | |
| 158 | + Effect.gen(function* () { | |
| 159 | + const target = yield* Target; | |
| 160 | + const browser = yield* Browser; | |
| 161 | + const { client: makeApiClient } = yield* Api; | |
| 162 | + const identity = yield* target.newIdentity(); | |
| 163 | + const client = yield* makeApiClient(api, identity); | |
| 164 | + const { oauth, slug } = yield* seedExpiredDcrMcpOAuthConnection(client, "mcp-hc-invalid"); | |
| 165 | + | |
| 166 | + const apiResult = yield* client.connections.checkHealth({ | |
| 167 | + params: { owner: "org", integration: slug, name }, | |
| 168 | + query: {}, | |
| 169 | + }); | |
| 170 | + expect(apiResult.status, "typed checkHealth classifies the dead grant").toBe("expired"); | |
| 171 | + expect(apiResult.detail, "the provider rejection detail is surfaced").toContain( | |
| 172 | + "Grant not found", | |
| 173 | + ); | |
| 174 | + const reread = yield* client.connections.get({ | |
| 175 | + params: { owner: "org", integration: slug, name }, | |
| 176 | + }); | |
| 177 | + expect(reread.lastHealth?.status, "the expired health verdict persisted").toBe("expired"); | |
| 178 | + expect(reread.lastHealth?.detail, "the persisted detail is useful").toContain( | |
| 179 | + "Grant not found", | |
| 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 | + }, | |
| 232 | + Effect.scoped( | |
| 233 | + Effect.gen(function* () { | |
| 234 | + const target = yield* Target; | |
| 235 | + const browser = yield* Browser; | |
| 236 | + const { client: makeApiClient } = yield* Api; | |
| 237 | + const identity = yield* target.newIdentity(); | |
| 238 | + const client = yield* makeApiClient(api, identity); | |
| 239 | + const { slug } = yield* seedExpiredDcrMcpOAuthConnection(client, "mcp-dcr-reconnect"); | |
| 240 | + | |
| 241 | + yield* browser.session(identity, async ({ page, step }) => { | |
| 242 | + const connections = connectionsSection(page); | |
| 243 | + const menuTrigger = connections.locator('button[aria-haspopup="menu"]').first(); | |
| 244 | + const dialog = page.getByRole("dialog"); | |
| 245 | + const oauthRequests: string[] = []; | |
| 246 | + | |
| 247 | + page.on("request", (request) => { | |
| 248 | + if (oauthReconnectRequest(request.url())) oauthRequests.push(request.url()); | |
| 249 | + }); | |
| 250 | + | |
| 251 | + await step("Open the MCP integration with its DCR OAuth connection", async () => { | |
| 252 | + await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" }); | |
| 253 | + await connections.getByText("main", { exact: true }).waitFor({ timeout: 30_000 }); | |
| 254 | + }); | |
| 255 | + | |
| 256 | + await step("Reconnect should keep a dialog visible and reach OAuth", async () => { | |
| 257 | + const oauthRequest = page | |
| 258 | + .waitForRequest((request) => oauthReconnectRequest(request.url()), { timeout: 30_000 }) | |
| 259 | + .then((request) => request.url()); | |
| 260 | + | |
| 261 | + await menuTrigger.click(); | |
| 262 | + await page.getByRole("menuitem", { name: "Reconnect" }).click(); | |
| 263 | + | |
| 264 | + await dialog.waitFor({ state: "visible", timeout: 30_000 }); | |
| 265 | + const reachedOAuth = await oauthRequest; | |
| 266 | + await page.waitForTimeout(2_000); | |
| 267 | + await dialog.waitFor({ state: "visible", timeout: 1_000 }); | |
| 268 | + console.info( | |
| 269 | + `[MCP OAuth repro] reconnect dialog stayed open; OAuth requests: ${ | |
| 270 | + oauthRequests.join(", ") || reachedOAuth | |
| 271 | + }`, | |
| 272 | + ); | |
| 273 | + expect(reachedOAuth, "Reconnect should issue an OAuth request").toBeTruthy(); | |
| 274 | + }); | |
| 275 | + }); | |
| 276 | + }), | |
| 277 | + ), | |
| 278 | +); | |
| @@ -58,6 +58,10 @@ export interface ScenarioOptions { · 57 unmodified lines | ||
| 58 | 58 | * body never runs. Use ONLY for a scenario blocked on a tracked, out-of-scope |
| 59 | 59 | * issue; state the reason here so the skip is self-documenting in the source. */ |
| 60 | 60 | 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; | |
| 61 | 65 | } |
| 62 | 66 | |
| 63 | 67 | type AllServices = |
| @@ -129,8 +133,9 @@ export const scenario = ( · 65 unmodified lines | ||
| 129 | 133 | const dir = join(RUNS_DIR, target.name, slugify(name)); |
| 130 | 134 | const context = contextFor(target, dir); |
| 131 | 135 | const testFile = captureTestFile(); |
| 136 | + const register = options.expectedFailure ? it.live.fails : it.live; | |
| 132 | 137 | |
| 133 | − it.live( | |
| 138 | + register( | |
| 134 | 139 | name, |
| 135 | 140 | (testCtx) => |
| 136 | 141 | Effect.gen(function* () { |
| @@ -122,13 +122,19 @@ export interface IntegrationAccountHandoff { · 121 unmodified lines | ||
| 122 | 122 | readonly template?: string; |
| 123 | 123 | /** Non-secret connection label to prefill. */ |
| 124 | 124 | readonly label?: string; |
| 125 | + /** Existing display identity to preserve when reconnecting a saved row. */ | |
| 126 | + readonly identityLabel?: string; | |
| 125 | 127 | /** Present when the agent handed off a CONFIDENTIAL OAuth-app registration |
| 126 | − * (via `oauth.clients.createHandoff`): the accounts UI opens the | |
| 128 | + * (via `oauth.clients.createHandoff`) or when a saved OAuth connection is | |
| 129 | + * reconnecting through its stored app. Registration opens the | |
| 127 | 130 | * Register-OAuth-app form pre-filled with these NON-secret fields, and the |
| 128 | − * human types the client secret directly into the browser. */ | |
| 131 | + * human types the client secret directly into the browser. Reconnect starts | |
| 132 | + * OAuth with the existing public client. */ | |
| 129 | 133 | readonly oauthClient?: { |
| 134 | + readonly action?: "register" | "reconnect"; | |
| 130 | 135 | /** Preselected client slug; when set the form's slug is fixed. */ |
| 131 | 136 | readonly slug?: string; |
| 137 | + readonly owner?: "org" | "user"; | |
| 132 | 138 | readonly grant?: string; |
| 133 | 139 | readonly clientId?: string; |
| 134 | 140 | readonly authorizationUrl?: string; |
| @@ -2594,6 +2594,40 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 2593 unmodified lines | ||
| 2594 | 2594 | // can still render the connection, just without a liveness verdict. |
| 2595 | 2595 | const unknownHealth = (): HealthCheckResult => ({ status: "unknown", checkedAt: Date.now() }); |
| 2596 | 2596 | |
| 2597 | + const persistHealthResult = ( | |
| 2598 | + ref: ConnectionRef, | |
| 2599 | + result: HealthCheckResult, | |
| 2600 | + ): Effect.Effect<void, never> => | |
| 2601 | + core | |
| 2602 | + .updateMany("connection", { | |
| 2603 | + where: (b: AnyCb) => | |
| 2604 | + b.and( | |
| 2605 | + b("owner", "=", String(ref.owner)), | |
| 2606 | + b("integration", "=", String(ref.integration)), | |
| 2607 | + b("name", "=", String(ref.name)), | |
| 2608 | + ), | |
| 2609 | + set: { last_health: result, updated_at: new Date() }, | |
| 2610 | + }) | |
| 2611 | + .pipe(Effect.ignore); | |
| 2612 | + | |
| 2613 | + const healthFromCredentialResolutionError = ( | |
| 2614 | + err: CredentialResolutionError, | |
| 2615 | + ): Effect.Effect<HealthCheckResult, StorageFailure> => | |
| 2616 | + err.reauthRequired === true | |
| 2617 | + ? Effect.succeed({ | |
| 2618 | + status: "expired", | |
| 2619 | + checkedAt: Date.now(), | |
| 2620 | + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field | |
| 2621 | + detail: err.message, | |
| 2622 | + }) | |
| 2623 | + : Effect.fail( | |
| 2624 | + new StorageError({ | |
| 2625 | + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field | |
| 2626 | + message: err.message, | |
| 2627 | + cause: err, | |
| 2628 | + }), | |
| 2629 | + ); | |
| 2630 | + | |
| 2597 | 2631 | // Resolve an in-flight credential's value map (key-first validation) without |
| 2598 | 2632 | // saving anything. Mirrors `resolveConnectionValues` for the saved-row path: |
| 2599 | 2633 | // pasted `value`/`values` are used directly; `from` origins resolve through |
| @@ -2654,41 +2688,33 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 54 unmodified lines | ||
| 2654 | 2688 | const check = runtime?.plugin.checkHealth; |
| 2655 | 2689 | if (!runtime || !check) return unknownHealth(); |
| 2656 | 2690 | |
| 2657 | − const values = yield* foldResolutionFailure(resolveConnectionValues(connectionRow)); | |
| 2658 | − const record = rowToIntegrationRecord( | |
| 2659 | − integrationRow, | |
| 2660 | − describeAuthMethodsForRow(integrationRow), | |
| 2661 | − ); | |
| 2662 | − const credential: ToolInvocationCredential = { | |
| 2663 | − owner: connectionRow.owner as Owner, | |
| 2664 | − integration: ref.integration, | |
| 2665 | − connection: ConnectionName.make(connectionRow.name), | |
| 2666 | − template: AuthTemplateSlug.make(connectionRow.template), | |
| 2667 | − value: values[PRIMARY_INPUT_VARIABLE] ?? null, | |
| 2668 | − values, | |
| 2669 | − config: record.config, | |
| 2670 | − }; | |
| 2671 | − // Core resolves the declared spec (its own column) and hands it to the | |
| 2672 | − // plugin; plugins no longer read it out of their config. | |
| 2673 | − const spec = describeHealthCheckForRow(integrationRow) ?? undefined; | |
| 2674 | − const result = yield* foldPluginFailure( | |
| 2675 | − check({ ctx: runtime.ctx, integration: record, credential, spec }), | |
| 2676 | − `Health check for connection "${ref.name}" failed.`, | |
| 2677 | − ); | |
| 2691 | + const result = yield* Effect.gen(function* () { | |
| 2692 | + const values = yield* resolveConnectionValues(connectionRow); | |
| 2693 | + const record = rowToIntegrationRecord( | |
| 2694 | + integrationRow, | |
| 2695 | + describeAuthMethodsForRow(integrationRow), | |
| 2696 | + ); | |
| 2697 | + const credential: ToolInvocationCredential = { | |
| 2698 | + owner: connectionRow.owner as Owner, | |
| 2699 | + integration: ref.integration, | |
| 2700 | + connection: ConnectionName.make(connectionRow.name), | |
| 2701 | + template: AuthTemplateSlug.make(connectionRow.template), | |
| 2702 | + value: values[PRIMARY_INPUT_VARIABLE] ?? null, | |
| 2703 | + values, | |
| 2704 | + config: record.config, | |
| 2705 | + }; | |
| 2706 | + // Core resolves the declared spec (its own column) and hands it to the | |
| 2707 | + // plugin; plugins no longer read it out of their config. | |
| 2708 | + const spec = describeHealthCheckForRow(integrationRow) ?? undefined; | |
| 2709 | + return yield* foldPluginFailure( | |
| 2710 | + check({ ctx: runtime.ctx, integration: record, credential, spec }), | |
| 2711 | + `Health check for connection "${ref.name}" failed.`, | |
| 2712 | + ); | |
| 2713 | + }).pipe(Effect.catchTag("CredentialResolutionError", healthFromCredentialResolutionError)); | |
| 2678 | 2714 | // Persist the verdict on the connection row so the accounts list shows |
| 2679 | 2715 | // alive/expired at a glance. Best-effort: a write failure must not turn |
| 2680 | 2716 | // a successful probe into an error. |
| 2681 | − yield* core | |
| 2682 | − .updateMany("connection", { | |
| 2683 | − where: (b: AnyCb) => | |
| 2684 | − b.and( | |
| 2685 | − b("owner", "=", String(ref.owner)), | |
| 2686 | − b("integration", "=", String(ref.integration)), | |
| 2687 | − b("name", "=", String(ref.name)), | |
| 2688 | − ), | |
| 2689 | − set: { last_health: result, updated_at: new Date() }, | |
| 2690 | − }) | |
| 2691 | − .pipe(Effect.ignore); | |
| 2717 | + yield* persistHealthResult(ref, result); | |
| 2692 | 2718 | return result; |
| 2693 | 2719 | }); |
| 2694 | 2720 | |
| @@ -45,6 +45,11 @@ const oauthPlugin = definePlugin(() => ({ · 44 unmodified lines | ||
| 45 | 45 | }, |
| 46 | 46 | // Echo the resolved credential value (the OAuth access token) back out. |
| 47 | 47 | invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }), |
| 48 | + checkHealth: ({ credential }) => | |
| 49 | + Effect.succeed({ | |
| 50 | + status: credential.value === null ? "expired" : "healthy", | |
| 51 | + checkedAt: Date.now(), | |
| 52 | + }), | |
| 48 | 53 | extension: (ctx) => ({ |
| 49 | 54 | seed: (scopes: readonly string[] = []) => |
| 50 | 55 | ctx.core.integrations.register({ |
| @@ -621,6 +626,82 @@ describe("oauth token refresh in resolveConnectionValue", () => { · 570 unmodified lines | ||
| 621 | 626 | }), |
| 622 | 627 | ), |
| 623 | 628 | ); |
| 629 | + | |
| 630 | + it.effect("checkHealth records expired when OAuth refresh is rejected", () => | |
| 631 | + Effect.scoped( | |
| 632 | + Effect.gen(function* () { | |
| 633 | + const server = yield* serveOAuthTestServer({ | |
| 634 | + scopes: ["read"], | |
| 635 | + supportRefresh: false, | |
| 636 | + tokenExpiresInSeconds: 0, | |
| 637 | + invalidRefreshTokenDescription: "Grant not found", | |
| 638 | + }); | |
| 639 | + const harness = yield* makeTestWorkspaceHarness({ plugins }); | |
| 640 | + const { executor, config } = harness; | |
| 641 | + yield* executor.acme.seed(); | |
| 642 | + | |
| 643 | + yield* executor.oauth.createClient({ | |
| 644 | + owner: "org", | |
| 645 | + slug: CLIENT, | |
| 646 | + authorizationUrl: server.authorizationEndpoint, | |
| 647 | + tokenUrl: server.tokenEndpoint, | |
| 648 | + grant: "authorization_code", | |
| 649 | + clientId: "test-client", | |
| 650 | + clientSecret: "test-secret", | |
| 651 | + resource: server.mcpResourceUrl, | |
| 652 | + }); | |
| 653 | + | |
| 654 | + const started = yield* executor.oauth.start({ | |
| 655 | + owner: "org", | |
| 656 | + client: CLIENT, | |
| 657 | + clientOwner: "org", | |
| 658 | + name: ConnectionName.make("main"), | |
| 659 | + integration: INTEG, | |
| 660 | + template: TEMPLATE, | |
| 661 | + }); | |
| 662 | + expect(started.status).toBe("redirect"); | |
| 663 | + if (started.status !== "redirect") return; | |
| 664 | + const callback = yield* server.completeAuthorizationCodeFlow({ | |
| 665 | + authorizationUrl: started.authorizationUrl, | |
| 666 | + }); | |
| 667 | + yield* executor.oauth.complete({ | |
| 668 | + state: started.state, | |
| 669 | + code: callback.code, | |
| 670 | + }); | |
| 671 | + | |
| 672 | + yield* Effect.promise(() => | |
| 673 | + config.db.updateMany("connection", { | |
| 674 | + where: (b) => b("name", "=", "main"), | |
| 675 | + set: { expires_at: Date.now() - 60_000 }, | |
| 676 | + }), | |
| 677 | + ); | |
| 678 | + yield* server.clearRequests; | |
| 679 | + | |
| 680 | + const result = yield* executor.connections.checkHealth({ | |
| 681 | + owner: "org", | |
| 682 | + integration: INTEG, | |
| 683 | + name: ConnectionName.make("main"), | |
| 684 | + }); | |
| 685 | + | |
| 686 | + expect(result.status).toBe("expired"); | |
| 687 | + expect(result.detail).toContain("Grant not found"); | |
| 688 | + const refreshed = yield* executor.connections.get({ | |
| 689 | + owner: "org", | |
| 690 | + integration: INTEG, | |
| 691 | + name: ConnectionName.make("main"), | |
| 692 | + }); | |
| 693 | + expect(refreshed?.lastHealth).toMatchObject({ | |
| 694 | + status: "expired", | |
| 695 | + detail: expect.stringContaining("Grant not found"), | |
| 696 | + }); | |
| 697 | + const requests = yield* server.requests; | |
| 698 | + const refreshRequest = requests.find( | |
| 699 | + (r) => r.path === "/token" && r.method === "POST" && r.body.includes("refresh_token"), | |
| 700 | + ); | |
| 701 | + expect(refreshRequest).toBeDefined(); | |
| 702 | + }), | |
| 703 | + ), | |
| 704 | + ); | |
| 624 | 705 | }); |
| 625 | 706 | |
| 626 | 707 | // Multi-site providers (Datadog) statically advertise one region's token |
| @@ -53,6 +53,8 @@ export interface OAuthTestServerOptions { · 52 unmodified lines | ||
| 53 | 53 | readonly scopes?: readonly string[]; |
| 54 | 54 | readonly omitTokenResponseScopes?: readonly string[]; |
| 55 | 55 | readonly supportRefresh?: boolean; |
| 56 | + readonly tokenExpiresInSeconds?: number; | |
| 57 | + readonly invalidRefreshTokenDescription?: string; | |
| 56 | 58 | /** Gate Dynamic Client Registration on the requested redirect URIs. When set, |
| 57 | 59 | * `/register` returns `400 invalid_redirect_uri` unless every requested |
| 58 | 60 | * `redirect_uris` entry is approved. Mirrors authorization servers (e.g. |
| @@ -412,6 +414,9 @@ export const serveOAuthTestServer = ( · 353 unmodified lines | ||
| 412 | 414 | ...(options.users ?? {}), |
| 413 | 415 | }; |
| 414 | 416 | const supportRefresh = options.supportRefresh ?? true; |
| 417 | + const tokenExpiresInSeconds = options.tokenExpiresInSeconds ?? 3600; | |
| 418 | + const invalidRefreshTokenDescription = | |
| 419 | + options.invalidRefreshTokenDescription ?? "Unknown refresh token"; | |
| 415 | 420 | const scopes = options.scopes ?? defaultScopes; |
| 416 | 421 | const omittedTokenResponseScopes = new Set(options.omitTokenResponseScopes ?? []); |
| 417 | 422 | const tokenResponseScope = (scope: string | null): string | undefined => { |
| @@ -645,7 +650,7 @@ export const serveOAuthTestServer = ( · 227 unmodified lines | ||
| 645 | 650 | access_token: accessToken, |
| 646 | 651 | refresh_token: refreshToken, |
| 647 | 652 | token_type: "Bearer", |
| 648 | − expires_in: 3600, | |
| 653 | + expires_in: tokenExpiresInSeconds, | |
| 649 | 654 | ...(scope ? { scope } : {}), |
| 650 | 655 | }, |
| 651 | 656 | { "cache-control": "no-store" }, |
| @@ -656,7 +661,7 @@ export const serveOAuthTestServer = ( · 4 unmodified lines | ||
| 656 | 661 | const refreshToken = params.get("refresh_token"); |
| 657 | 662 | const record = refreshToken ? refreshTokens.get(refreshToken) : undefined; |
| 658 | 663 | if (!supportRefresh || !refreshToken || !record || record.clientId !== clientId) { |
| 659 | − return oauthError(400, "invalid_grant", "Unknown refresh token"); | |
| 664 | + return oauthError(400, "invalid_grant", invalidRefreshTokenDescription); | |
| 660 | 665 | } |
| 661 | 666 | const nextAccessToken = `at_${randomUUID()}`; |
| 662 | 667 | const nextRefreshToken = `rt_${randomUUID()}`; |
| @@ -673,7 +678,7 @@ export const serveOAuthTestServer = ( · 10 unmodified lines | ||
| 673 | 678 | access_token: nextAccessToken, |
| 674 | 679 | refresh_token: nextRefreshToken, |
| 675 | 680 | token_type: "Bearer", |
| 676 | − expires_in: 3600, | |
| 681 | + expires_in: tokenExpiresInSeconds, | |
| 677 | 682 | ...(scope ? { scope } : {}), |
| 678 | 683 | }, |
| 679 | 684 | { "cache-control": "no-store" }, |
| @@ -688,7 +693,7 @@ export const serveOAuthTestServer = ( · 8 unmodified lines | ||
| 688 | 693 | { |
| 689 | 694 | access_token: accessToken, |
| 690 | 695 | token_type: "Bearer", |
| 691 | − expires_in: 3600, | |
| 696 | + expires_in: tokenExpiresInSeconds, | |
| 692 | 697 | scope: params.get("scope") ?? scopes.join(" "), |
| 693 | 698 | }, |
| 694 | 699 | { "cache-control": "no-store" }, |
| @@ -501,6 +501,7 @@ export function AccountsSection(props: { · 500 unmodified lines | ||
| 501 | 501 | methods={methods} |
| 502 | 502 | onEdit={setEditingConnection} |
| 503 | 503 | onDcrReconnect={(connection: Connection) => { |
| 504 | + if (connection.oauthClient == null) return; | |
| 504 | 505 | setReconnectHandoff({ |
| 505 | 506 | key: `reconnect:${connection.owner}:${String(connection.integration)}:${String( |
| 506 | 507 | connection.name, |
| @@ -508,6 +509,14 @@ export function AccountsSection(props: { · 1 unmodified line | ||
| 508 | 509 | owner: connection.owner, |
| 509 | 510 | template: String(connection.template), |
| 510 | 511 | label: String(connection.name), |
| 512 | + ...(connection.identityLabel != null | |
| 513 | + ? { identityLabel: connection.identityLabel } | |
| 514 | + : {}), | |
| 515 | + oauthClient: { | |
| 516 | + action: "reconnect", | |
| 517 | + slug: String(connection.oauthClient), | |
| 518 | + owner: connection.oauthClientOwner ?? connection.owner, | |
| 519 | + }, | |
| 511 | 520 | }); |
| 512 | 521 | }} |
| 513 | 522 | declaredScopes={declaredScopes} |
| @@ -1,4 +1,4 @@ | ||
| 1 | −import { useEffect, useMemo, useRef, useState } from "react"; | |
| 1 | +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; | |
| 2 | 2 | import { useAtomSet, useAtomValue } from "@effect/atom-react"; |
| 3 | 3 | import * as Exit from "effect/Exit"; |
| 4 | 4 | import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; |
| @@ -1310,8 +1310,10 @@ function AddAccountModalView(props: AddAccountModalProps) { · 1305 unmodified lines | ||
| 1310 | 1310 | // user can cancel back out without it springing open again; retries on later |
| 1311 | 1311 | // renders only while the methods list is still empty. |
| 1312 | 1312 | const oauthHandoffOpenedKey = useRef<string | null>(null); |
| 1313 | + const oauthReconnectOpenedKey = useRef<string | null>(null); | |
| 1313 | 1314 | useEffect(() => { |
| 1314 | 1315 | if (!initialState?.oauthClient) return; |
| 1316 | + if (initialState.oauthClient.action === "reconnect") return; | |
| 1315 | 1317 | if (oauthHandoffOpenedKey.current === initialState.key) return; |
| 1316 | 1318 | const oauthMethod = allMethods.find((m: AuthMethod) => m.kind === "oauth"); |
| 1317 | 1319 | if (!oauthMethod) return; |
| @@ -1603,7 +1605,58 @@ function AddAccountModalView(props: AddAccountModalProps) { · 285 unmodified lines | ||
| 1603 | 1605 | // Just ask the parent to close. Reopening remounts this whole component (see |
| 1604 | 1606 | // AddAccountModal), so there is nothing to hand-reset: the form fields and the |
| 1605 | 1607 | // OAuth popup flow's busy state die with this instance. |
| 1606 | − const close = () => onOpenChange(false); | |
| 1608 | + const close = useCallback(() => onOpenChange(false), [onOpenChange]); | |
| 1609 | + | |
| 1610 | + useEffect(() => { | |
| 1611 | + const handoff = initialState; | |
| 1612 | + const oauthClient = handoff?.oauthClient; | |
| 1613 | + if (!handoff || oauthClient?.action !== "reconnect") return; | |
| 1614 | + if (oauthReconnectOpenedKey.current === handoff.key) return; | |
| 1615 | + const client = oauthClient.slug; | |
| 1616 | + const clientOwner = oauthClient.owner ?? handoff.owner; | |
| 1617 | + const connectionOwner = handoff.owner; | |
| 1618 | + const connectionName = handoff.label; | |
| 1619 | + const oauthMethod = handoff.template | |
| 1620 | + ? allMethods.find( | |
| 1621 | + (m: AuthMethod) => | |
| 1622 | + m.kind === "oauth" && | |
| 1623 | + (m.id === handoff.template || String(m.template) === handoff.template), | |
| 1624 | + ) | |
| 1625 | + : allMethods.find((m: AuthMethod) => m.kind === "oauth"); | |
| 1626 | + if (!client || !clientOwner || !connectionOwner || !connectionName || !oauthMethod) return; | |
| 1627 | + | |
| 1628 | + oauthReconnectOpenedKey.current = handoff.key; | |
| 1629 | + setMethodId(oauthMethod.id); | |
| 1630 | + void oauthPopup.start({ | |
| 1631 | + payload: { | |
| 1632 | + client: OAuthClientSlug.make(client), | |
| 1633 | + clientOwner, | |
| 1634 | + owner: connectionOwner, | |
| 1635 | + name: ConnectionName.make(connectionName), | |
| 1636 | + integration, | |
| 1637 | + template: oauthMethod.template, | |
| 1638 | + ...(handoff.identityLabel !== undefined ? { identityLabel: handoff.identityLabel } : {}), | |
| 1639 | + }, | |
| 1640 | + onAuthorizationStarted: () => { | |
| 1641 | + trackEvent("connection_reconnected", { | |
| 1642 | + integration_slug: String(integration), | |
| 1643 | + owner: connectionOwner, | |
| 1644 | + success: true, | |
| 1645 | + }); | |
| 1646 | + }, | |
| 1647 | + onError: () => { | |
| 1648 | + trackEvent("connection_reconnected", { | |
| 1649 | + integration_slug: String(integration), | |
| 1650 | + owner: connectionOwner, | |
| 1651 | + success: false, | |
| 1652 | + }); | |
| 1653 | + }, | |
| 1654 | + onSuccess: () => { | |
| 1655 | + toast.success("Reconnected"); | |
| 1656 | + close(); | |
| 1657 | + }, | |
| 1658 | + }); | |
| 1659 | + }, [initialState, allMethods, integration, oauthPopup, close]); | |
| 1607 | 1660 | |
| 1608 | 1661 | const probeAndAutoNameOAuthConnection = async ( |
| 1609 | 1662 | connection: OAuthCompletionPayload, |
| @@ -2082,6 +2135,9 @@ function AddAccountModalView(props: AddAccountModalProps) { · 472 unmodified lines | ||
| 2082 | 2135 | <Dialog open={open} onOpenChange={onOpenChange} modal={false}> |
| 2083 | 2136 | <DialogContent |
| 2084 | 2137 | forceOverlay |
| 2138 | + onInteractOutside={ | |
| 2139 | + oauthClientHandoff?.action === "reconnect" ? (event) => event.preventDefault() : undefined | |
| 2140 | + } | |
| 2085 | 2141 | className={cn( |
| 2086 | 2142 | "max-h-[85vh] overflow-x-hidden overflow-y-auto", |
| 2087 | 2143 | (addingMethod && createCustomMethod) || oauthRegistering || oauthEditing |