From 13dd4c1eaecd842be60b08fbcafe4606e23edf9e Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 20 Aug 2026 22:11:52 +0000 Subject: [PATCH] fix(apps): preserve grant and approval defaults Co-authored-by: Codex --- .../src/migrations/0228_absurd_hedge_knight.sql | 2 +- packages/shared/src/app-definitions.ts | 5 ++--- .../src/__tests__/generic-mcp-connection.test.ts | 3 +++ server/src/services/tool-access.ts | 3 +++ tests/e2e/app-not-connected.spec.ts | 2 +- tests/e2e/application-delete-screenshot.spec.ts | 2 +- tests/e2e/applications-crud.spec.ts | 2 +- tests/e2e/apps-dark-mode-shots.spec.ts | 2 +- tests/e2e/mcp-user-stories.spec.ts | 16 +++++++++++----- tests/e2e/playwright.config.ts | 1 + ui/src/pages/apps/Browse.test.tsx | 2 +- ui/src/pages/apps/store-cards.tsx | 2 +- 12 files changed, 27 insertions(+), 15 deletions(-) diff --git a/packages/db/src/migrations/0228_absurd_hedge_knight.sql b/packages/db/src/migrations/0228_absurd_hedge_knight.sql index 97da105586b..4148770ab86 100644 --- a/packages/db/src/migrations/0228_absurd_hedge_knight.sql +++ b/packages/db/src/migrations/0228_absurd_hedge_knight.sql @@ -1,4 +1,4 @@ -ALTER TABLE "connection_grant_members" DROP CONSTRAINT "connection_grant_members_grant_id_connection_grants_id_fk"; +ALTER TABLE "connection_grant_members" DROP CONSTRAINT IF EXISTS "connection_grant_members_grant_id_connection_grants_id_fk"; --> statement-breakpoint ALTER TABLE "connection_grants" ADD CONSTRAINT "connection_grants_company_id_uq" UNIQUE("company_id","id");--> statement-breakpoint ALTER TABLE "connection_grant_members" ADD CONSTRAINT "connection_grant_members_company_grant_fk" FOREIGN KEY ("company_id","grant_id") REFERENCES "public"."connection_grants"("company_id","id") ON DELETE cascade ON UPDATE no action; diff --git a/packages/shared/src/app-definitions.ts b/packages/shared/src/app-definitions.ts index 69a10824c91..7ac34d87dc8 100644 --- a/packages/shared/src/app-definitions.ts +++ b/packages/shared/src/app-definitions.ts @@ -68,10 +68,9 @@ export function credentialConfigPath(field: FieldDef): string { } export function recommendedDefaultsForApp(app: AppDefinition, methodKey?: string | null): Record { - void app; - void methodKey; + const method = getAvailableConnectionMethod(app, methodKey); return { access: "all_agents", - askFirstRiskLevels: [], + askFirstRiskLevels: method?.riskTier === "S1" ? [] : ["write", "destructive"], }; } diff --git a/server/src/__tests__/generic-mcp-connection.test.ts b/server/src/__tests__/generic-mcp-connection.test.ts index c88765e1691..ab18cd54b72 100644 --- a/server/src/__tests__/generic-mcp-connection.test.ts +++ b/server/src/__tests__/generic-mcp-connection.test.ts @@ -366,6 +366,9 @@ describeEmbeddedPostgres("generic remote MCP connections", () => { // this connection cannot be depending on gallery metadata for anything. expect(connection!.config).not.toHaveProperty("sourceTemplateKey"); expect(connection!.config).not.toHaveProperty("connectionMethodKey"); + await expect(service.listConnectionGrants(result.connectionId, company.id)).resolves.toMatchObject({ + grants: [expect.objectContaining({ kind: "organization", isDefault: true, credentialSecretRefs: [] })], + }); const [profile] = await db.select().from(toolProfiles).where(eq( toolProfiles.profileKey, `app:${result.connectionId}`, diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 2ff1469eff8..a2d1a1bf7ae 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -4585,6 +4585,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }) .where(eq(toolConnections.id, existing.id)) .returning(); + await ensureDefaultOrganizationGrant(updated); await syncCredentialBindings(updated); await ensureRuntimeSlot(updated); return { row: updated, created: false }; @@ -4605,6 +4606,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} credentialRefs: [], credentialSecretRefs: [], }).returning(); + await ensureDefaultOrganizationGrant(created); await syncCredentialBindings(created); await ensureRuntimeSlot(created); return { row: created, created: true }; @@ -6568,6 +6570,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} createdByUserId: actor?.actorType === "user" ? actor.actorId ?? null : null, }).returning(); } + await ensureDefaultOrganizationGrant(connectionRow); await syncCredentialBindings(connectionRow); await ensureRuntimeSlot(connectionRow); diff --git a/tests/e2e/app-not-connected.spec.ts b/tests/e2e/app-not-connected.spec.ts index c338b237d64..1c263757376 100644 --- a/tests/e2e/app-not-connected.spec.ts +++ b/tests/e2e/app-not-connected.spec.ts @@ -118,7 +118,7 @@ test.describe.serial("not-connected app page", () => { await page.goto(`/${seed.prefix}/apps/app/${applicationId}`); await page.getByRole("button", { name: "Reconnect", exact: true }).click(); await expect(page).toHaveURL(/\/apps\/connect\?/, { timeout: 20_000 }); - await expect(page.getByText("Connect with a link")).toBeVisible({ timeout: 20_000 }); + await expect(page.getByText("Connect your own MCP server")).toBeVisible({ timeout: 20_000 }); await expect(page.getByText(mock.url)).toBeVisible(); await page.screenshot({ path: `${SCREENSHOT_DIR}/apps-nav-w6-02-reconnect-prefilled.png`, fullPage: true }); diff --git a/tests/e2e/application-delete-screenshot.spec.ts b/tests/e2e/application-delete-screenshot.spec.ts index 0202971fcd0..0f165215254 100644 --- a/tests/e2e/application-delete-screenshot.spec.ts +++ b/tests/e2e/application-delete-screenshot.spec.ts @@ -33,7 +33,7 @@ test("captures the current app removal confirmations", async ({ page }) => { applicationName: "Guarded MCP", name: "Primary connection", transport: "mcp_remote", - config: { url: "https://fixture.example/mcp" }, + config: { url: "http://localhost:65535/mcp" }, }, }); expect(conn.ok(), `connection create failed ${conn.status()}: ${await conn.text()}`).toBe(true); diff --git a/tests/e2e/applications-crud.spec.ts b/tests/e2e/applications-crud.spec.ts index fc0697849a7..e25f9d9c6a7 100644 --- a/tests/e2e/applications-crud.spec.ts +++ b/tests/e2e/applications-crud.spec.ts @@ -46,7 +46,7 @@ async function createConnection( const res = await request.post(`/api/companies/${companyId}/tools/connections`, { data: { transport: "mcp_remote", - config: { url: "https://fixture.example/mcp" }, + config: { url: "http://localhost:65535/mcp" }, enabled: true, status: "active", ...data, diff --git a/tests/e2e/apps-dark-mode-shots.spec.ts b/tests/e2e/apps-dark-mode-shots.spec.ts index 18937694a4f..053b1eeacbd 100644 --- a/tests/e2e/apps-dark-mode-shots.spec.ts +++ b/tests/e2e/apps-dark-mode-shots.spec.ts @@ -137,7 +137,7 @@ test.describe.serial("dark-mode Apps surfaces", () => { await forceDark(page); await page.goto(`/${seed.prefix}/apps/connections`); await expect(page.getByRole("heading", { name: "Connections" })).toBeVisible({ timeout: 30_000 }); - await expect(page.getByText(/app needs attention/i).first()).toBeVisible({ timeout: 30_000 }); + await expect(page.getByText(/needs attention/i).first()).toBeVisible({ timeout: 30_000 }); await page.screenshot({ path: `${SCREENSHOT_DIR}/apps-nav-02-attention-dark.png`, fullPage: true }); }); diff --git a/tests/e2e/mcp-user-stories.spec.ts b/tests/e2e/mcp-user-stories.spec.ts index 7e5d4600cd6..6314a32d90d 100644 --- a/tests/e2e/mcp-user-stories.spec.ts +++ b/tests/e2e/mcp-user-stories.spec.ts @@ -95,6 +95,7 @@ async function setScoutScript( ) { await json(await request.patch(`/api/agents/${scout.id}`, { data: { + adapterType: "process", adapterConfig: { command: process.execPath, args: ["--input-type=module", "-e", script], @@ -287,17 +288,22 @@ test.describe.serial("MCP prod Phase 5a user-story harness", () => { test(`${storyById("US-1").id} ${storyById("US-1").title} @mcp-runnable @mcp-us1`, async ({ page, request }) => { const { seed, scout, mock, connectionId } = await seedConnectedFixture(request, "us1"); try { - await page.goto(`/${seed.prefix}/apps/${connectionId}`); - await expect(page.getByRole("heading", { name: /Sheets Fixture us1/i })).toBeVisible({ timeout: 30_000 }); - await screenshot(page, "US-1", "01-connected-app"); - await setScoutScript(request, scout, buildGatewayCallScript(connectionId, "sheets:list_rows")); const invoked = await invokeHeartbeat(request, scout.id); const run = await waitForRun(request, invoked.id); - expect(run.status, run.error ?? `heartbeat run ${run.id} did not succeed`).toBe("succeeded"); + const failedLog = run.status === "succeeded" + ? null + : await json<{ content?: string }>(await request.get(`/api/heartbeat-runs/${run.id}/log?offset=0&limitBytes=65536`)); + expect( + run.status, + [run.error ?? `heartbeat run ${run.id} did not succeed`, failedLog?.content].filter(Boolean).join("\n"), + ).toBe("succeeded"); expect(mock.captures.some((capture) => capture.method === "tools/call" && capture.toolName === "sheets:list_rows")).toBe(true); await expectAuditEvent(request, seed.companyId, { connectionId, agentId: scout.id, search: "sheets:list_rows" }); + await page.goto(`/${seed.prefix}/apps/${connectionId}`); + await expect(page.getByRole("heading", { name: /Sheets Fixture us1/i })).toBeVisible({ timeout: 30_000 }); + await screenshot(page, "US-1", "01-connected-app"); await page.goto(`/${seed.prefix}/apps/${connectionId}/activity`); await screenshot(page, "US-1", "02-activity"); } finally { diff --git a/tests/e2e/playwright.config.ts b/tests/e2e/playwright.config.ts index 8fc3beae1fa..09ce17617e9 100644 --- a/tests/e2e/playwright.config.ts +++ b/tests/e2e/playwright.config.ts @@ -70,6 +70,7 @@ export default defineConfig({ ...process.env, NODE_ENV: "test", PORT: String(PORT), + PAPERCLIP_API_URL: BASE_URL, PAPERCLIP_HOME, PAPERCLIP_INSTANCE_ID, PAPERCLIP_CONFIG, diff --git a/ui/src/pages/apps/Browse.test.tsx b/ui/src/pages/apps/Browse.test.tsx index 4337908a69a..bc5977903c4 100644 --- a/ui/src/pages/apps/Browse.test.tsx +++ b/ui/src/pages/apps/Browse.test.tsx @@ -178,7 +178,7 @@ describe("Browse store door (PAP-13254 door 1)", () => { await act(async () => { byoCard?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - expect(navigateMock).toHaveBeenCalledWith("/apps/byo"); + expect(navigateMock).toHaveBeenCalledWith("/apps/connect?byo=1"); }); it("filters the gallery by the search query", async () => { diff --git a/ui/src/pages/apps/store-cards.tsx b/ui/src/pages/apps/store-cards.tsx index a84641053c4..0a500823dd5 100644 --- a/ui/src/pages/apps/store-cards.tsx +++ b/ui/src/pages/apps/store-cards.tsx @@ -7,7 +7,7 @@ import { appSourceConnectHref } from "./app-connect-policy"; export const POPULAR_KEYS = ["zapier", "github", "slack", "notion", "posthog", "linear"]; /** Deep-link into the Connect wizard's bring-your-own-tool URL flow. */ -export const BYO_CONNECT_HREF = "/apps/byo"; +export const BYO_CONNECT_HREF = "/apps/connect?byo=1"; /** Zapier connects with the complete MCP URL issued by Zapier. */ export const ZAPIER_CONNECT_HREF = "/apps/connect?byo=1&source=zapier";