Merge remote-tracking branch 'origin/main' into fix/database-connection-safeguards

Author: RhysSullivan

.changeset/fair-admin-integrations.md

@@ -0,0 +1,5 @@
+---
+"@executor-js/react": patch
+---
+
+Show restricted integration actions as disabled controls with an admin explanation. Members can browse the catalog and add personal connections to existing integrations.

.changeset/oauth-discovered-scope-budget.md

@@ -0,0 +1,5 @@
+---
+"@executor-js/sdk": patch
+---
+
+Request every scope a resource advertises during OAuth scope discovery, bounded by an 8 KiB scope-string budget instead of a 100-scope count. Resources with many fine-grained scopes previously received a token missing the ones it needed. Health checks without a probe no longer replace a tool-sync failure verdict with "healthy".

e2e/cloud/integration-creation-permissions.test.ts

@@ -0,0 +1,17 @@
+import { Effect } from "effect";
+import { scenario } from "../src/scenario";
+import { Target } from "../src/services";
+import { integrationCreationPermissions } from "../src/integration-creation-permissions";
+import { forBrowser, joinOrg } from "./support/session";
+
+scenario(
+  "Integration creation · cloud members see admin guidance and admins can add",
+  { timeout: 180_000 },
+  Effect.gen(function* () {
+    const target = yield* Target;
+    const admin = yield* target.newIdentity();
+    const invitee = yield* target.newIdentity({ org: false });
+    const member = yield* joinOrg(target, admin, invitee);
+    yield* integrationCreationPermissions(forBrowser(admin), forBrowser(member));
+  }),
+);

e2e/selfhost/integration-creation-permissions.test.ts

@@ -0,0 +1,21 @@
+import { Effect } from "effect";
+import { scenario } from "../src/scenario";
+import { Target } from "../src/services";
+import { integrationCreationPermissions } from "../src/integration-creation-permissions";
+import { createInvitedIdentity } from "../targets/selfhost";
+
+scenario(
+  "Integration creation · self-host members see admin guidance and owners can add",
+  { timeout: 180_000 },
+  Effect.gen(function* () {
+    const target = yield* Target;
+    const admin = yield* target.newIdentity();
+    const member = yield* Effect.promise(() =>
+      createInvitedIdentity(target.baseUrl, admin, {
+        role: "member",
+        emailPrefix: "integration-permissions",
+      }),
+    );
+    yield* integrationCreationPermissions(admin, member);
+  }),
+);

e2e/src/integration-creation-permissions.ts

@@ -0,0 +1,164 @@
+import { randomBytes } from "node:crypto";
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+import { composePluginApi } from "@executor-js/api/server";
+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
+import { IntegrationSlug } from "@executor-js/sdk/shared";
+
+import { Api, Browser } from "./services";
+import type { Identity } from "./target";
+import { visit } from "./surfaces/browser";
+
+const api = composePluginApi([openApiHttpPlugin()] as const);
+
+/** Exercise integration creation and member restrictions through the shared console. */
+export const integrationCreationPermissions = (admin: Identity, member: Identity) =>
+  Effect.gen(function* () {
+    const browser = yield* Browser;
+    const { client } = yield* Api;
+    const adminClient = yield* client(api, admin);
+    const title = `Permissions API ${randomBytes(4).toString("hex")}`;
+    const slug = IntegrationSlug.make(title.toLowerCase().replaceAll(" ", "_"));
+    const spec = JSON.stringify({
+      openapi: "3.0.3",
+      info: { title, version: "1.0.0" },
+      servers: [{ url: "https://api.example.com" }],
+      paths: {},
+      components: {
+        securitySchemes: { apiKey: { type: "apiKey", in: "header", name: "X-API-Key" } },
+      },
+      security: [{ apiKey: [] }],
+    });
+
+    yield* Effect.ensuring(
+      Effect.gen(function* () {
+        yield* browser.session(admin, async ({ page, step }) => {
+          await step("Admin opens the integration catalog", async () => {
+            await visit(page, "/");
+            await page.getByRole("button", { name: "Browse integrations", exact: true }).waitFor();
+            await page.keyboard.press("ControlOrMeta+k");
+            await page.getByRole("option", { name: /^Add OpenAPI/ }).waitFor();
+            await page.keyboard.press("Escape");
+            await page.getByRole("link", { name: "Add integration", exact: true }).click();
+            await page.getByRole("heading", { name: "Add an integration", exact: true }).waitFor();
+            await page
+              .getByRole("textbox", { name: "Search integrations, or paste a URL" })
+              .waitFor();
+          });
+          await step("Admin creates an integration from the setup form", async () => {
+            await visit(page, "/integrations/add/openapi");
+            await page.getByPlaceholder("https://api.example.com/openapi.json").fill(spec);
+            await page.getByRole("button", { name: "Add integration", exact: true }).click();
+            await page.waitForURL((url) => url.pathname.endsWith(`/integrations/${slug}`), {
+              timeout: 30_000,
+            });
+            await page.getByRole("button", { name: "Edit", exact: true }).waitFor();
+            await page.getByRole("button", { name: "Delete", exact: true }).waitFor();
+          });
+        });
+        expect(yield* adminClient.integrations.get({ params: { slug } })).toMatchObject({
+          name: title,
+        });
+
+        yield* browser.session(member, async ({ page, step }) => {
+          await step(
+            "Member sees disabled creation controls with an admin explanation",
+            async () => {
+              await visit(page, "/");
+              await page.getByRole("heading", { name: "Integrations", exact: true }).waitFor();
+              await page.getByTestId(`integration-entry-${slug}`).waitFor();
+              const add = page.getByRole("button", { name: "Add integration", exact: true });
+              await add.waitFor();
+              expect(await add.isDisabled()).toBe(true);
+              expect(
+                await page
+                  .getByRole("button", { name: "Browse integrations", exact: true })
+                  .isDisabled(),
+              ).toBe(true);
+              const hint = page
+                .getByRole("group", { name: "Requires a workspace admin" })
+                .filter({ has: add });
+              await hint.hover();
+              await page.getByRole("tooltip", { name: "Requires a workspace admin" }).waitFor();
+              await hint.focus();
+              const before = page.url();
+              await page.keyboard.press("Enter");
+              expect(page.url()).toBe(before);
+            },
+          );
+          await step(
+            "Member sees disabled add commands and can still find existing integrations",
+            async () => {
+              await page.keyboard.press("ControlOrMeta+k");
+              const palette = page.getByRole("dialog");
+              await palette.getByRole("option", { name: new RegExp(title) }).waitFor();
+              const addCommand = palette.getByRole("option", { name: /^Add OpenAPI/ });
+              await addCommand.waitFor();
+              expect(await addCommand.getAttribute("aria-disabled")).toBe("true");
+              expect(await addCommand.textContent()).toContain("Admin only");
+              await page.keyboard.press("Escape");
+            },
+          );
+          await step("Member sees disabled Edit and Delete actions", async () => {
+            await page.getByTestId(`integration-entry-${slug}`).click();
+            await page.getByRole("button", { name: "Add connection", exact: true }).waitFor();
+            for (const name of ["Edit", "Delete"]) {
+              const action = page.getByRole("button", { name, exact: true });
+              await action.waitFor();
+              expect(await action.isDisabled()).toBe(true);
+            }
+          });
+          await step("Member can still add a personal connection", async () => {
+            await page.getByRole("button", { name: "Add connection", exact: true }).click();
+            const dialog = page.getByRole("dialog");
+            await dialog.waitFor();
+            expect(await dialog.getByText("Workspace", { exact: true }).count()).toBe(0);
+          });
+          await step("Member browses the catalog with disabled Add buttons", async () => {
+            await visit(page, "/integrations/browse");
+            await page.getByRole("heading", { name: "Add an integration", exact: true }).waitFor();
+            await page
+              .getByText("Requires a workspace admin to add integrations.", { exact: true })
+              .waitFor();
+            const addButtons = page.getByRole("button", { name: /^Add / });
+            await addButtons.first().waitFor();
+            for (const button of await addButtons.all())
+              expect(await button.isDisabled()).toBe(true);
+            const scratch = page.getByRole("button", {
+              name: "New OpenAPI integration from scratch",
+              exact: true,
+            });
+            expect(await scratch.isDisabled()).toBe(true);
+            const view = page.getByRole("link", { name: `View ${title}`, exact: true });
+            await view.waitFor();
+            expect(await view.isEnabled()).toBe(true);
+          });
+          await step("Member cannot add a URL with the button or Enter key", async () => {
+            const input = page.getByRole("textbox", {
+              name: "Search integrations, or paste a URL",
+            });
+            await input.fill("https://api.example.com/openapi.json");
+            expect(
+              await page.getByRole("button", { name: "Add this URL", exact: true }).isDisabled(),
+            ).toBe(true);
+            const before = page.url();
+            await input.press("Enter");
+            expect(page.url()).toBe(before);
+          });
+          for (const path of ["/integrations/add/openapi", "/integrations/add/mcp"]) {
+            await step(`Member follows ${path} and sees the admin explanation`, async () => {
+              await visit(page, path);
+              await page.getByRole("heading", { name: "An admin must add integrations" }).waitFor();
+              expect(await page.getByRole("textbox").count()).toBe(0);
+              expect(await page.getByRole("button", { name: /^Add/ }).count()).toBe(0);
+            });
+          }
+          await step("Member returns to their existing integrations", async () => {
+            await page.getByRole("link", { name: "Back to integrations" }).click();
+            await page.getByRole("heading", { name: "Integrations", exact: true }).waitFor();
+          });
+        });
+      }),
+      adminClient.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore),
+    );
+  });

packages/core/sdk/src/executor.ts

@@ -3605,7 +3605,15 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
                           ? { tools_synced_at: Date.now() }
                           : {
                               tools_synced_at: Date.now(),
-                              last_health: health ?? toolSyncHealth(reason),
+                              // A plugin-supplied verdict (e.g. the MCP server
+                              // rejecting the token during discovery) is still
+                              // sync-stamped: mark it so credential-only health
+                              // checks cannot bury it under "healthy", and a
+                              // later successful sync clears it.
+                              last_health:
+                                health === undefined
+                                  ? toolSyncHealth(reason)
+                                  : { ...health, reason: health.reason ?? "tool_sync_failed" },
                               updated_at: new Date(),
                             },
                     })
@@ -5138,7 +5146,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
     ): Effect.Effect<void> =>
       findConnectionRow(ref).pipe(
         Effect.flatMap((fresh) =>
-          fresh === null || oauthReauthRequiredFromProviderState(fresh.provider_state) !== null
+          fresh === null ||
+          oauthReauthRequiredFromProviderState(fresh.provider_state) !== null ||
+          // A credential verdict cannot refute a failed tool sync; only a
+          // successful sync clears that record (see `isToolSyncHealth`).
+          isToolSyncHealth(Option.getOrNull(decodeLastHealth(fresh.last_health)))
             ? Effect.void
             : persistHealthResult(ref, fresh, result),
         ),
@@ -5241,6 +5253,17 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
                 // failure is the one real signal this path can produce, and it
                 // must not hide inside a green span.
                 oauthCredentialHealthWithoutProbe(connectionRow).pipe(
+                  // A resolvable token says nothing about whether the
+                  // upstream accepts it. When tool sync has already recorded
+                  // that it does not (a rejected discovery handshake, an
+                  // unreachable server), that verdict stands until a sync
+                  // succeeds — serving "healthy" here would hide a connection
+                  // that has no tools behind a green badge.
+                  Effect.map((result) =>
+                    result.status === "healthy" && previous !== null && isToolSyncHealth(previous)
+                      ? previous
+                      : result,
+                  ),
                   Effect.tap((result) => persistProbeHealthResult(ref, result)),
                   Effect.map((result) => ({
                     source: "credential_only" as const,

packages/core/sdk/src/health-check.ts

@@ -145,6 +145,7 @@ export type HealthCheckResult = typeof HealthCheckResult.Type;
 export const toolSyncHealthDetailPrefix = "Tool sync failing";
 
 export const isToolSyncHealth = (result: HealthCheckResult | null | undefined): boolean =>
+  result?.reason === "tool_sync_failed" ||
   result?.detail?.startsWith(toolSyncHealthDetailPrefix) === true;
 
 // ---------------------------------------------------------------------------

packages/core/sdk/src/oauth-flow.test.ts

@@ -2033,6 +2033,73 @@ describe("oauth token refresh in resolveConnectionValue", () => {
       ),
   );
 
+  it.effect(
+    "checkHealth without a probe serves a sync-stamped verdict instead of burying it under healthy",
+    () =>
+      Effect.scoped(
+        Effect.gen(function* () {
+          const server = yield* serveOAuthTestServer({ scopes: ["read"] });
+          const { executor, config } = yield* makeTestWorkspaceHarness({ plugins });
+          yield* executor.acme.seed();
+
+          yield* executor.oauth.createClient({
+            owner: "org",
+            slug: CLIENT,
+            authorizationUrl: server.authorizationEndpoint,
+            tokenUrl: server.tokenEndpoint,
+            grant: "authorization_code",
+            clientId: "test-client",
+            clientSecret: "test-secret",
+            resource: server.mcpResourceUrl,
+          });
+
+          const started = yield* executor.oauth.start({
+            owner: "org",
+            client: CLIENT,
+            clientOwner: "org",
+            name: ConnectionName.make("main"),
+            integration: INTEG,
+            template: TEMPLATE,
+          });
+          expect(started.status).toBe("redirect");
+          if (started.status !== "redirect") return;
+          const callback = yield* server.completeAuthorizationCodeFlow({
+            authorizationUrl: started.authorizationUrl,
+          });
+          yield* executor.oauth.complete({ state: started.state, code: callback.code });
+
+          // Tool sync found the upstream rejecting the freshly minted token
+          // (e.g. an MCP discovery handshake answering 401) and stamped it.
+          // The token itself still resolves, so a credential-only check would
+          // otherwise report healthy and hide a connection that has no tools.
+          const stamped = {
+            status: "expired",
+            checkedAt: Date.now(),
+            detail: "MCP OAuth reauthorization required",
+            reason: "tool_sync_failed",
+          };
+          yield* Effect.promise(() =>
+            config.db.updateMany("connection", {
+              where: (b) => b("name", "=", "main"),
+              set: { last_health: stamped },
+            }),
+          );
+
+          const result = yield* executor.connections.checkHealth({
+            owner: "org",
+            integration: INTEG,
+            name: ConnectionName.make("main"),
+          });
+          expect(result).toMatchObject(stamped);
+
+          const row = yield* Effect.promise(() =>
+            config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }),
+          );
+          expect(row?.last_health).toMatchObject(stamped);
+        }),
+      ),
+  );
+
   it.effect("records missing authorization-code scopes without blocking the connection", () =>
     Effect.scoped(
       Effect.gen(function* () {