diff --git a/doc/connections/CONNECTOR-PLAYBOOK.md b/doc/connections/CONNECTOR-PLAYBOOK.md index 950656cbf2..58bf3c5e9e 100644 --- a/doc/connections/CONNECTOR-PLAYBOOK.md +++ b/doc/connections/CONNECTOR-PLAYBOOK.md @@ -662,8 +662,16 @@ internal discussion; a task comment or agent progress update must not imply that an external action occurred. Reuse existing task-feed components and preserve one visible record per external event. +**Keep setup to one screen.** The connect screen collects only what proves who +the user is: a provider sign-in, a key, or an endpoint. It states the default +access in one line, with **Change** for other choices, and does not add an +access step. Pick the ranked default method instead of asking. Put scope, +capability, and per-action choices on the Permissions tab after the connection. +`connectionSetupStateForMethod` in `packages/shared` classifies each method as +`instant`, `authorize`, `paste`, or `register`; the gallery verb comes from it. + **Make interactive Storybooks for setup and actual use.** Include the catalog -card, access and credential steps, any agent-resource wizard, and the task +card, the connect screen and its credential states, any agent-resource wizard, and the task journeys after setup. Provide a clickable walkthrough plus focused stories for important steps, loading, errors, and recovery. Use realistic fixtures and clearly label simulated actions. Reuse production components as implementation diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index e118218623..161b709d36 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -19,12 +19,12 @@ repository and MCP URLs still resolve to the ordinary GitHub tool connection. ## Self-hosted setup -The Access step uses **Continue** to open the local setup screen. -**Continue to GitHub** on that screen starts the provider handoff. The first -button does not imply that the browser is leaving Paperclip yet. +The setup screen states the default access in one line, with **Change** for +other choices. **Continue to GitHub** on that screen starts the provider +handoff. A self-hosted instance needs one Paperclip Cloud approval before its first -managed connection. After approval, setup returns to step 2 and continues to +managed connection. After approval, setup returns to the connect screen and continues to GitHub without another instance approval or a service restart. If an unapproved enrollment link expires, return to setup and select diff --git a/packages/shared/src/app-definitions.test.ts b/packages/shared/src/app-definitions.test.ts index 72d9c53b12..adce8e88f1 100644 --- a/packages/shared/src/app-definitions.test.ts +++ b/packages/shared/src/app-definitions.test.ts @@ -262,7 +262,7 @@ describe("AppDefinition catalog", () => { "google-workspace-search", ]), ); - expect(SELF_SERVE_MCP_CANDIDATES).toHaveLength(48); + expect(SELF_SERVE_MCP_CANDIDATES).toHaveLength(49); expect(BLOCKED_MCP_PROVIDERS.map((entry) => entry.slug)).toEqual([ "g2", "vercel", @@ -426,15 +426,15 @@ describe("AppDefinition catalog", () => { expect(channel("slack")?.guidanceMd).toContain("reactions"); expect(channel("slack")?.guidanceMd).toContain("direct messages"); }); - it("keeps a complete, unique, dated evidence ledger for all 51 researched MCP providers", () => { + it("keeps a complete, unique, dated evidence ledger for all 52 researched MCP providers", () => { // Ledger-wide date reflects the last full re-verification (2026-08-26); // later provider additions carry their own research evidence, but // bumping the shared date would overstate freshness for the other providers. expect(SELF_SERVE_MCP_RESEARCH.verifiedAt).toBe("2026-08-26"); - expect(SELF_SERVE_MCP_RESEARCH.entries).toHaveLength(51); + expect(SELF_SERVE_MCP_RESEARCH.entries).toHaveLength(52); expect( new Set(SELF_SERVE_MCP_RESEARCH.entries.map((entry) => entry.slug)), - ).toHaveProperty("size", 51); + ).toHaveProperty("size", 52); for (const entry of SELF_SERVE_MCP_RESEARCH.entries) { expect(new URL(entry.docsUrl).protocol).toBe("https:"); expect(new URL(entry.serverUrl).protocol).toBe("https:"); @@ -572,7 +572,11 @@ describe("AppDefinition catalog", () => { (field) => field.key === "readOnly", )?.defaultValue, ).toBe(false); - expect(method("asana")?.ownershipModes).toEqual(["customer"]); + // Asana and Linear both advertise dynamic client registration and issue + // clients on request (verified live 2026-09-28), so neither needs an + // operator-registered OAuth app. "customer" stays as the manual fallback. + expect(method("asana")?.ownershipModes).toEqual(["dcr", "customer"]); + expect(method("linear")?.ownershipModes).toEqual(["dcr", "customer"]); expect(method("zapier")).toMatchObject({ key: "generated-url", auth: "none", @@ -617,7 +621,7 @@ describe("AppDefinition catalog", () => { APP_DEFINITIONS.find((app) => app.slug === "hugging-face")?.methods[0] ?.defaults?.scopesHint, ).toEqual(["read-mcp", "read-repos", "contribute-repos", "jobs"])); - it("defaults every new connection action to allowed", () => { + it("defaults every action, reads and writes, to allowed", () => { for (const app of APP_DEFINITIONS) for (const method of app.methods) expect(recommendedDefaultsForApp(app, method.key)).toEqual({ diff --git a/packages/shared/src/app-definitions.ts b/packages/shared/src/app-definitions.ts index c347335d0c..83410b49bf 100644 --- a/packages/shared/src/app-definitions.ts +++ b/packages/shared/src/app-definitions.ts @@ -255,10 +255,12 @@ export function resolveConnectionMethodServerUrl( export function recommendedDefaultsForApp(app: AppDefinition, methodKey?: string | null): Record { // Keep the parameters in the public contract: callers resolve defaults for a - // concrete app/method even though the initial policy is now uniform. This is + // concrete app/method even though the initial policy is uniform. This is // an open default, not an approval bypass: connection finalization remains a // configure-authorized, audited operation, and Ask first stays available as // an operator-selected policy for any action after the connection is made. + // The connect flow lands on the Permissions tab so that choice is the very + // next screen (PAP-659: agents get full permissions unless someone narrows them). void app; void methodKey; return { diff --git a/packages/shared/src/app-definitions/asana.json b/packages/shared/src/app-definitions/asana.json index 8376a35288..ecda6f4b60 100644 --- a/packages/shared/src/app-definitions/asana.json +++ b/packages/shared/src/app-definitions/asana.json @@ -21,25 +21,23 @@ "transport": "mcp_remote", "auth": "oauth", "ownershipModes": [ + "dcr", "customer" ], - "whenToUse": "Register an OAuth app with Asana, then enter its client ID and secret.", + "whenToUse": "Sign in with Asana. Paperclip registers its own OAuth client automatically.", "defaults": { "serverUrl": "https://mcp.asana.com/v2/mcp", "scopesHint": [ "default" ] }, - "guidanceMd": "Connect Asana in the browser. Create an Asana MCP OAuth app and register Paperclip's callback URI; DCR is not supported.", + "guidanceMd": "Connect Asana in the browser. Paperclip registers its own OAuth client with Asana's MCP server, so a local Paperclip needs no developer-console setup. Asana only accepts localhost callbacks, so a hosted deployment falls back to entering your own OAuth app.", "riskTier": "S3", - "label": "Use your own OAuth app", + "label": "Sign in with Asana", "consoleLinks": { "register": "https://developers.asana.com/docs/integrating-with-asanas-mcp-server", "docs": "https://developers.asana.com/docs/integrating-with-asanas-mcp-server" - }, - "warnings": [ - "Create an Asana MCP OAuth app and register Paperclip's callback URI; DCR is not supported." - ] + } } ] } diff --git a/packages/shared/src/app-definitions/linear.json b/packages/shared/src/app-definitions/linear.json index 28d77bf370..9dc209229f 100644 --- a/packages/shared/src/app-definitions/linear.json +++ b/packages/shared/src/app-definitions/linear.json @@ -20,6 +20,7 @@ "transport": "mcp_remote", "auth": "oauth", "ownershipModes": [ + "dcr", "customer" ], "whenToUse": "Use the provider-hosted connection for the quickest setup.", @@ -32,7 +33,7 @@ "write" ] }, - "guidanceMd": "Register a Linear OAuth app and add Paperclip's redirect URI before connecting.", + "guidanceMd": "Connect Linear for issues and projects. Paperclip registers its own OAuth client with Linear's MCP server, so no developer-console setup is needed.", "riskTier": "S2", "requiredResourceFilters": [ "workspace", diff --git a/packages/shared/src/connection-setup-state.test.ts b/packages/shared/src/connection-setup-state.test.ts new file mode 100644 index 0000000000..e4cc83af58 --- /dev/null +++ b/packages/shared/src/connection-setup-state.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; +import { + CONNECTABLE_APP_DEFINITIONS, + getAvailableConnectionMethods, + getConnectableAppDefinition, +} from "./app-definitions.js"; +import { + connectionSetupStateForApp, + connectionSetupVerbForApp, + type ConnectionSetupState, +} from "./connection-setup-state.js"; + +/** + * These assert against the shipped catalog rather than fixtures, because the + * claim being protected is about real connectors: the gallery card's verb has + * to match what the connect screen then asks for. + */ +describe("connectionSetupStateForApp", () => { + it("puts one-click OAuth connectors in the authorize state", () => { + for (const slug of ["notion", "linear", "sentry", "stripe", "jira", "asana"]) { + expect(connectionSetupStateForApp(getConnectableAppDefinition(slug)), slug).toBe("authorize"); + } + }); + + it("follows the instance's ownership availability rather than the catalog order", () => { + // Google and GitHub publish a Paperclip-managed method, but it is + // `platform_shared` and therefore unavailable until an operator configures + // the cloud connector. The state has to reflect what this instance can + // actually do, or the card promises one click and the screen shows a form. + const gmail = getConnectableAppDefinition("gmail")!; + expect(connectionSetupStateForApp(gmail)).toBe("register"); + expect( + connectionSetupStateForApp({ + ...gmail, + ownershipAvailability: { ...gmail.ownershipAvailability, platform_shared: true }, + }), + ).toBe("authorize"); + }); + + it("reserves register for providers that genuinely cannot issue a client", () => { + for (const slug of ["box", "xero"]) { + expect(connectionSetupStateForApp(getConnectableAppDefinition(slug)), slug).toBe("register"); + } + }); + + it("treats an API key as a paste, and says so on the card", () => { + for (const slug of ["honcho", "mem0", "openrouter"]) { + expect(connectionSetupStateForApp(getConnectableAppDefinition(slug)), slug).toBe("paste"); + expect(connectionSetupVerbForApp(getConnectableAppDefinition(slug)), slug).toBe("Add key"); + } + }); + + it("reaches instant only when the catalog already knows the endpoint", () => { + // Composio and Context7 ship a fixed server URL and ask for nothing. + expect(connectionSetupStateForApp(getConnectableAppDefinition("composio"))).toBe("instant"); + expect(connectionSetupStateForApp(getConnectableAppDefinition("context7"))).toBe("instant"); + }); + + it("does not promise instant for a provider-generated URL", () => { + // Zapier, Arcade and Executor declare no server URL: the operator brings + // one. Calling these instant is the mistake this helper exists to prevent. + for (const slug of ["zapier", "arcade", "executor"]) { + expect(connectionSetupStateForApp(getConnectableAppDefinition(slug)), slug).toBe("paste"); + } + }); + + it("does not promise instant for a templated endpoint", () => { + // Shopify's URL has a {storeDomain} placeholder the operator fills in. + expect(connectionSetupStateForApp(getConnectableAppDefinition("shopify"))).toBe("paste"); + }); + + it("defaults the AI providers to their subscription sign-in", () => { + for (const slug of ["anthropic", "openai", "xai"]) { + expect(connectionSetupStateForApp(getConnectableAppDefinition(slug)), slug).toBe("authorize"); + expect(connectionSetupVerbForApp(getConnectableAppDefinition(slug)), slug).toBe("Connect"); + } + }); + + it("never says Add key for something that does not want a secret", () => { + for (const app of CONNECTABLE_APP_DEFINITIONS) { + if (connectionSetupVerbForApp(app) !== "Add key") continue; + const wantsSecret = getAvailableConnectionMethods(app) + .filter((method) => (method.purpose ?? "tool") !== "channel") + .some((method) => (method.credentialFields?.length ?? 0) > 0); + expect(wantsSecret, app.slug).toBe(true); + } + }); + + it("resolves a state for every connectable tool connector", () => { + const states = new Map(); + for (const app of CONNECTABLE_APP_DEFINITIONS) { + const hasToolMethod = app.methods.some((method) => (method.purpose ?? "tool") !== "channel"); + if (!hasToolMethod) continue; + states.set(app.slug, connectionSetupStateForApp(app)); + } + expect([...states].filter(([, state]) => state === null)).toEqual([]); + }); + + it("returns null rather than guessing for an unknown app", () => { + expect(connectionSetupStateForApp(null)).toBeNull(); + }); + + it("survives a gallery row that carries no method list", () => { + // Rows are assembled from the gallery, live connections and custom + // endpoints, so a row can reach a card without methods. Throwing here would + // take the whole connectors page down with it. + const partial = { slug: "mystery", name: "Mystery" } as never; + expect(connectionSetupStateForApp(partial)).toBeNull(); + expect(connectionSetupVerbForApp(partial)).toBe("Connect"); + }); +}); diff --git a/packages/shared/src/connection-setup-state.ts b/packages/shared/src/connection-setup-state.ts new file mode 100644 index 0000000000..becd04545f --- /dev/null +++ b/packages/shared/src/connection-setup-state.ts @@ -0,0 +1,112 @@ +import { + connectionMethodAcceptsCustomerOAuthClient, + connectionMethodRequiresConfiguration, + connectionMethodSupportsAutomaticOAuth, + getAvailableConnectionMethods, + getRecommendedConnectionMethod, +} from "./app-definitions.js"; +import type { AppDefinition, ConnectionMethodDef } from "./types/app-definition.js"; + +/** + * The four states every connect screen lands in (PAP-659, "one shell, four + * states"). + * + * The point of naming them here rather than in the wizard is that two surfaces + * have to agree: the gallery card decides its verb from the state, and the + * connect screen decides its body from the same state. When each re-derived it + * from raw method flags they drifted — a card offering "Connect" for something + * that then demanded a pasted URL is exactly the inconsistency the ticket is + * about. + * + * - `instant` nothing to supply: connect resolves entirely from the catalog. + * - `authorize` one click, then the provider's own consent screen. + * - `paste` one thing the operator has to bring: a key, a URL, a tenant. + * - `register` the provider cannot issue a client, so one must be registered + * in its console first. A recovery state, never a normal step. + */ +export type ConnectionSetupState = "instant" | "authorize" | "paste" | "register"; + +/** + * True when the catalog already knows the endpoint. A `serverUrlTemplate` does + * not count: its placeholders are the operator's to fill, which is a paste. + */ +function hasResolvedEndpoint(method: ConnectionMethodDef): boolean { + if (method.transport !== "mcp_remote") return true; + if (method.defaults?.serverUrlTemplate) return false; + return Boolean(method.defaults?.serverUrl); +} + +export function connectionSetupStateForMethod( + method: ConnectionMethodDef | null | undefined, +): ConnectionSetupState | null { + if (!method) return null; + if (method.auth === "oauth") { + // An AI subscription signs in through the adapter's own device/OAuth flow. + // No client is ever registered, so `register` would be a false detour. + if (method.transport === "runtime_auth") return "authorize"; + if (connectionMethodSupportsAutomaticOAuth(method)) return "authorize"; + // A customer-owned client is only a *register* state when the provider + // offers nothing better. Live discovery can still overrule the catalog at + // connect time, which is why the flow re-resolves rather than trusting this. + if (connectionMethodAcceptsCustomerOAuthClient(method)) return "register"; + return "authorize"; + } + if (method.auth === "none") { + return connectionMethodRequiresConfiguration(method) || !hasResolvedEndpoint(method) + ? "paste" + : "instant"; + } + return "paste"; +} + +/** + * The default tool method for an app, or null. + * + * Gallery rows are not guaranteed to carry a method list: the display entry is + * assembled from several sources, and a row can stand for a connection whose + * catalog entry has since been withdrawn. Resolving that to null — rather than + * throwing inside a card render — is the difference between a missing verb and + * a blank connectors page. + */ +function defaultToolMethod( + app: AppDefinition | null | undefined, + methodKey?: string | null, +): ConnectionMethodDef | null { + if (!app || !Array.isArray(app.methods)) return null; + const methods = getAvailableConnectionMethods(app).filter( + (method) => (method.purpose ?? "tool") !== "channel", + ); + return methodKey + ? methods.find((candidate) => candidate.key === methodKey) ?? null + : getRecommendedConnectionMethod(methods); +} + +export function connectionSetupStateForApp( + app: AppDefinition | null | undefined, + methodKey?: string | null, +): ConnectionSetupState | null { + return connectionSetupStateForMethod(defaultToolMethod(app, methodKey)); +} + +/** + * The gallery card's verb (PAP-659 C4). + * + * Only a method that genuinely wants a secret says so. "Add key" on a connector + * that actually wants a pasted URL would be worse than the generic verb, so a + * paste state without credential fields keeps "Connect" — the operator still + * lands on one screen with one field, which the verb does not need to restate. + */ +export function connectionSetupVerbForMethod( + method: ConnectionMethodDef | null | undefined, +): "Connect" | "Add key" { + if (!method) return "Connect"; + const wantsSecret = (method.credentialFields?.length ?? 0) > 0; + return connectionSetupStateForMethod(method) === "paste" && wantsSecret ? "Add key" : "Connect"; +} + +export function connectionSetupVerbForApp( + app: AppDefinition | null | undefined, + methodKey?: string | null, +): "Connect" | "Add key" { + return connectionSetupVerbForMethod(defaultToolMethod(app, methodKey)); +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 78ded88c4d..92466095b9 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -333,6 +333,13 @@ export { recommendedDefaultsForApp, resolveConnectionMethodServerUrl, } from "./app-definitions.js"; +export { + connectionSetupStateForApp, + connectionSetupStateForMethod, + connectionSetupVerbForApp, + connectionSetupVerbForMethod, + type ConnectionSetupState, +} from "./connection-setup-state.js"; export { APP_DEFINITIONS } from "./app-definitions.generated.js"; export * from "./google-workspace-connectors.js"; export * from "./github-connectors.js"; diff --git a/packages/shared/src/self-serve-mcp-research.json b/packages/shared/src/self-serve-mcp-research.json index 0efcf21e2c..366d6d763f 100644 --- a/packages/shared/src/self-serve-mcp-research.json +++ b/packages/shared/src/self-serve-mcp-research.json @@ -40,7 +40,8 @@ { "slug": "supabase", "name": "Supabase", "wave": 2, "status": "self_serve", "docsUrl": "https://supabase.com/docs/guides/ai-tools/mcp", "serverUrl": "https://mcp.supabase.com/mcp", "authMode": "dcr_or_api_key", "prerequisite": "A Supabase account; use a development project and review write actions before connecting production data.", "riskTier": "S4" }, { "slug": "ticket-tailor", "name": "Ticket Tailor", "wave": 2, "status": "self_serve", "docsUrl": "https://developers.tickettailor.com/docs/mcp/authentication/", "serverUrl": "https://mcp.tickettailor.ai/mcp", "authMode": "dcr", "prerequisite": "A Ticket Tailor account; the provider may request an API key during its hosted authorization prompt.", "riskTier": "S3" }, - { "slug": "asana", "name": "Asana", "wave": 3, "status": "self_serve", "docsUrl": "https://developers.asana.com/docs/integrating-with-asanas-mcp-server", "serverUrl": "https://mcp.asana.com/v2/mcp", "authMode": "customer_oauth", "prerequisite": "Create an Asana MCP OAuth app and register Paperclip's callback URI; DCR is not supported.", "riskTier": "S3" }, + { "slug": "asana", "name": "Asana", "wave": 3, "status": "self_serve", "docsUrl": "https://developers.asana.com/docs/integrating-with-asanas-mcp-server", "serverUrl": "https://mcp.asana.com/v2/mcp", "authMode": "dcr", "prerequisite": "An active Asana account. Re-verified 2026-09-28: mcp.asana.com advertises a registration endpoint and issues clients for http://localhost callbacks, so a local Paperclip registers its own OAuth client. Asana refuses non-localhost redirect URIs, so a hosted deployment still registers an app in Asana's developer console.", "riskTier": "S3" }, + { "slug": "linear", "name": "Linear", "wave": 3, "status": "self_serve", "docsUrl": "https://linear.app/docs/mcp", "serverUrl": "https://mcp.linear.app/mcp", "authMode": "dcr_cimd", "prerequisite": "An active Linear workspace. Verified 2026-09-28: mcp.linear.app advertises a registration endpoint and client-ID metadata documents, so Paperclip registers its own OAuth client.", "riskTier": "S2" }, { "slug": "box", "name": "Box", "wave": 3, "status": "self_serve", "docsUrl": "https://support.box.com/hc/en-us/articles/43847256139923-Managing-Box-MCP-Servers", "serverUrl": "https://mcp.box.com", "authMode": "customer_oauth", "prerequisite": "A Box administrator creates the OAuth integration and enables AI access.", "riskTier": "S3" }, { "slug": "mem0", "name": "Mem0", "wave": 3, "status": "self_serve", "docsUrl": "https://docs.mem0.ai/platform/mem0-mcp", "serverUrl": "https://mcp.mem0.ai/mcp/", "authMode": "api_key", "prerequisite": "A Mem0 API key; the live server currently requires the slash-normalized endpoint.", "riskTier": "S3" }, { "slug": "pagerduty", "name": "PagerDuty", "wave": 3, "status": "self_serve", "docsUrl": "https://support.pagerduty.com/main/docs/pagerduty-mcp-server", "serverUrl": "https://mcp.pagerduty.com/mcp", "authMode": "api_key", "prerequisite": "A PagerDuty API token; choose the regional endpoint that hosts the account.", "riskTier": "S4" }, diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 44121fb4c6..7fc5d90010 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -10031,6 +10031,64 @@ describeEmbeddedPostgres("tool access service", () => { ).toHaveLength(2); }); + it("registers Linear against its MCP authorization server instead of the pinned console endpoints", async () => { + vi.stubEnv("PAPERCLIP_PUBLIC_URL", "https://paperclip.example"); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_LINEAR_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_LINEAR_CLIENT_SECRET", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_SECRET", ""); + const company = await createCompany(db); + const userId = `linear-owner-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const app = createRouteApp( + db, + boardSessionActor(company.id, "owner", userId), + ); + const fetched: string[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + fetched.push(href); + if (href === "https://mcp.linear.app/.well-known/oauth-protected-resource/mcp") { + return mcpHttpResponse({ + resource: "https://mcp.linear.app/mcp", + authorization_servers: ["https://mcp.linear.app"], + scopes_supported: ["read", "write"], + }); + } + if (href === "https://mcp.linear.app/.well-known/oauth-authorization-server") { + return mcpHttpResponse({ + issuer: "https://mcp.linear.app", + authorization_endpoint: "https://mcp.linear.app/authorize", + token_endpoint: "https://mcp.linear.app/token", + registration_endpoint: "https://mcp.linear.app/register", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + }); + } + if (href === "https://mcp.linear.app/register") { + return mcpHttpResponse({ + client_id: "linear-registered-client", + redirect_uris: ["https://paperclip.example/api/tools/oauth/callback"], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }); + } + throw new Error(`unexpected fetch ${href}`); + }); + + const connectRes = await request(app) + .post(`/api/companies/${company.id}/tools/apps/connect`) + .send({ galleryKey: "linear", name: "Linear", grantKind: "user" }) + .expect(201); + + const startUrl = new URL(connectRes.body.auth.startUrl); + expect(startUrl.origin + startUrl.pathname).toBe("https://mcp.linear.app/authorize"); + expect(startUrl.searchParams.get("client_id")).toBe("linear-registered-client"); + expect(fetched).toContain("https://mcp.linear.app/register"); + expect(fetched.some((href) => href.startsWith("https://linear.app/"))).toBe(false); + }); + it("returns a pre-scoped personal Notion callback directly to Permissions", async () => { vi.stubEnv("PAPERCLIP_PUBLIC_URL", "https://paperclip.example"); vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID", ""); @@ -10116,9 +10174,31 @@ describeEmbeddedPostgres("tool access service", () => { ); expect(state).toBeTruthy(); + // The provider's redirect is a cross-site navigation: Paperclip commits a + // page at once (Railway's consent page otherwise replaces itself after ~2s) + // and leaves the state unconsumed for the same-origin repeat. + const interstitialRes = await request(app) + .get("/api/tools/oauth/callback") + .set("Accept", "text/html") + .set("Sec-Fetch-Site", "cross-site") + .set("Sec-Fetch-Mode", "navigate") + .query({ state, code: "notion-choice-code" }); + expect(interstitialRes.status).toBe(200); + expect(interstitialRes.headers["cache-control"]).toBe("no-store"); + expect(interstitialRes.text).toContain( + ``, + ); + const [pendingConnection] = await db + .select() + .from(toolConnections) + .where(eq(toolConnections.id, connectRes.body.connectionId)); + expect(pendingConnection?.status).not.toBe("active"); + const callbackRes = await request(app) .get("/api/tools/oauth/callback") .set("Accept", "text/html") + .set("Sec-Fetch-Site", "same-origin") + .set("Sec-Fetch-Mode", "navigate") .query({ state, code: "notion-choice-code" }); expect(callbackRes.status).toBe(303); diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index 0f4ad7d8b0..f4c5119497 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -191,6 +191,29 @@ export function connectionIntentOAuthOutcomeHtml(input: { return `Connection authorization

Returning to Paperclip…

`; } +// Some providers' consent pages navigate to this callback and, if their own +// page is still on screen ~2s later, replace it with a "you can close this +// window" screen (Railway does exactly this). Exchanging the code and +// discovering the tool catalog routinely takes longer than that, so the +// provider's timer wins and the browser never lands back in Paperclip even +// though the connection completed. For a cross-site browser navigation, commit +// a Paperclip document immediately and repeat the same request from it; the +// repeat is same-origin and does the slow work. +export function isCrossSiteOAuthCallbackNavigation(req: Request): boolean { + return req.get("sec-fetch-site") === "cross-site" + && req.get("sec-fetch-mode") === "navigate"; +} + +export function oauthCallbackInterstitialHtml(continuePath: string): string { + const attribute = continuePath + .replaceAll("&", "&") + .replaceAll("\"", """) + .replaceAll("<", "<"); + // A meta refresh alone, not a script as well: the OAuth code is single-use, so + // two racing follow-ups would let the loser render an expired-state error. + return `Finishing connection

Finishing your connection…

`; +} + function normalizeCloudConnectorEnrollmentReturnTo(returnTo?: string | null): string | null { if (!returnTo || returnTo.length > 2_048) return null; try { @@ -1331,6 +1354,13 @@ function connectorEnrollmentPrincipal(req: Request): string { await assertToolConnectionConfigureAccess(req, pendingConnection); } const acceptsHtml = req.get("accept")?.includes("text/html") === true; + if (acceptsHtml && isCrossSiteOAuthCallbackNavigation(req)) { + // State is only peeked above, so the same-origin repeat still owns it. + res.set("Cache-Control", "no-store"); + res.set("Referrer-Policy", "no-referrer"); + res.type("html").send(oauthCallbackInterstitialHtml(req.originalUrl)); + return; + } let result: Awaited>; try { result = await svc.completeOAuthCallback({ diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 880f66e956..982a0e82bc 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -9018,9 +9018,22 @@ export function toolAccessService( const galleryMethod = galleryEntry ? connectionMethodForConnection(galleryEntry, connection) : null; + // Pinned endpoints describe the provider's console-registered OAuth app. + // A method that also offers dynamic registration registers against the MCP + // server's own authorization server (Linear: mcp.linear.app, not + // linear.app), which only discovery finds. So the pins stand in for + // discovery only when no registration is possible or the connection + // already carries an operator-entered client. + const storedOAuth = oauthConfig(connection); + const usesOperatorClient = + storedOAuth.clientRegistrationSource === "manual" && + connection.ownership !== "dcr" && + typeof storedOAuth.clientId === "string" && + storedOAuth.clientId.trim().length > 0; const hasCompleteGalleryEndpointHints = Boolean( galleryMethod?.defaults?.authorizationEndpoint && - galleryMethod.defaults.tokenEndpoint, + galleryMethod.defaults.tokenEndpoint && + (!galleryMethod.ownershipModes.includes("dcr") || usesOperatorClient), ); // The smoke-lab fixture's endpoints are first-party and complete, so // discovery is not just unnecessary there, it must not run: an unreachable @@ -9744,6 +9757,13 @@ export function toolAccessService( * connection may register once — and only once — protected-resource and * authorization-server discovery actually produced a metadata document; an * endpoint that merely returned a 401 does not earn a registration. + * + * A curated method pinned to customer-owned clients still earns a registration + * when the provider's own metadata advertises one. `ownershipModes` is a + * point-in-time research snapshot, and providers add dynamic registration + * without telling us; live discovery is the better evidence of the two, so a + * stale catalog entry costs the operator a console detour rather than silently + * outranking what the server just said about itself. */ function canRegisterOAuthClientDynamically( connection: typeof toolConnections.$inferSelect, @@ -9751,10 +9771,9 @@ export function toolAccessService( galleryEntry: AppDefinition | null, ): boolean { if (galleryEntry) { - return connectionMethodForConnection( - galleryEntry, - connection, - ).ownershipModes.includes("dcr"); + const method = connectionMethodForConnection(galleryEntry, connection); + if (method.ownershipModes.includes("dcr")) return true; + return method.auth === "oauth" && Boolean(endpoints.registrationUrl); } return ( connection.transport === "mcp_remote" && Boolean(endpoints.metadataUrl) diff --git a/tests/e2e/app-not-connected.spec.ts b/tests/e2e/app-not-connected.spec.ts index e8ceef241b..f4d2307c42 100644 --- a/tests/e2e/app-not-connected.spec.ts +++ b/tests/e2e/app-not-connected.spec.ts @@ -119,7 +119,6 @@ test.describe.serial("not-connected app page", () => { await page.getByRole("button", { name: "Reconnect", exact: true }).click(); await expect(page).toHaveURL(/\/apps\/connect\?/, { timeout: 20_000 }); await expect(page.getByText("Connect your own MCP server")).toBeVisible({ timeout: 20_000 }); - await page.getByRole("button", { name: "Save and continue" }).click(); 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/apps-prosumer-mcp-flow.spec.ts b/tests/e2e/apps-prosumer-mcp-flow.spec.ts index 7b18344b2d..ddba28ec58 100644 --- a/tests/e2e/apps-prosumer-mcp-flow.spec.ts +++ b/tests/e2e/apps-prosumer-mcp-flow.spec.ts @@ -161,22 +161,17 @@ test.describe.serial("prosumer MCP flow prosumer MCP flow", () => { await linkInput.fill(mock.url); await page.getByRole("button", { name: "Continue" }).click(); - // Access is chosen before credentials so the user knows who and which - // agents will receive the connection before Paperclip contacts it. - await expect(page.getByText("Which humans can use this credential?")).toBeVisible(); - await page.getByRole("button", { name: "Save and continue" }).click(); - - // LinkKey step keeps the BYO connection heading. Mock doesn't - // require a key — leave the default "No" answer. + // There is no separate Access step: the link opens the key screen, which + // states the default access in one line. The mock needs no key, and a + // credential challenge from the server is what would ask for one. await expect(page.getByRole("heading", { name: "Connect your own MCP server" })).toBeVisible({ timeout: 15_000 }); await page.screenshot({ path: `${SCREENSHOT_DIR}/prosumer-mcp-02-key-step.png`, fullPage: true }); // Submit (button label is "Check link"). await page.getByRole("button", { name: /Check link/i }).click(); - // The Access choice was captured before credentials. A successful generic - // probe now commits discovered actions and risk defaults transactionally, - // so the key check lands directly on success. + // A successful generic probe commits discovered actions and the stated + // access defaults transactionally, so the key check lands on success. await expect(page.getByRole("heading", { name: /is ready\.$/i })).toBeVisible({ timeout: 30_000 }); await page.screenshot({ path: `${SCREENSHOT_DIR}/prosumer-mcp-05-success.png`, fullPage: true }); diff --git a/tests/e2e/chat-adapters-ui-providers.spec.ts b/tests/e2e/chat-adapters-ui-providers.spec.ts index 57ce962fda..7d37d75b4b 100644 --- a/tests/e2e/chat-adapters-ui-providers.spec.ts +++ b/tests/e2e/chat-adapters-ui-providers.spec.ts @@ -140,10 +140,14 @@ test.describe.serial("native chat adapter UI", () => { '[role="listitem"][data-app-slug="github"]', ); await expect(connector).toBeVisible(); - await connector.getByRole("button", { name: "Connect GitHub" }).click(); + // Without the cloud connector GitHub's default method is a token, so the + // card's verb is "Add key" rather than "Connect". + await connector.getByRole("button", { name: "Add key GitHub" }).click(); await expect(page).toHaveURL(/\/apps\/connect\?/); expect(new URL(page.url()).searchParams.get("source")).toBe("github"); + // Identity is a stated default; its choices sit behind "Change". + await page.getByRole("button", { name: "Change", exact: true }).click(); await expect( page.getByRole("heading", { name: "Connect GitHub as" }), ).toBeVisible(); @@ -218,7 +222,8 @@ test.describe.serial("native chat adapter UI", () => { await expect(connector).toBeVisible({ timeout: 30_000 }); if (provider.provider === "github") { const tools = page.locator('[role="listitem"][data-app-slug="github"]'); - await tools.getByRole("button", { name: "Connect GitHub", exact: true }).click(); + await tools.getByRole("button", { name: "Add key GitHub", exact: true }).click(); + await page.getByRole("button", { name: "Change", exact: true }).click(); await expect(page.getByRole("heading", { name: "Connect GitHub as" })).toBeVisible(); await expect(page.getByRole("heading", { name: "Choose how to connect" })).toHaveCount(0); await page.goto(`/${seed.prefix}/apps`); diff --git a/tests/e2e/connection-intents.spec.ts b/tests/e2e/connection-intents.spec.ts index 1cb1e033fa..c3dce4e7ff 100644 --- a/tests/e2e/connection-intents.spec.ts +++ b/tests/e2e/connection-intents.spec.ts @@ -234,7 +234,6 @@ test("store setup and task connection intent share one fake provider through con .getByPlaceholder("https://example.com/actions") .fill(provider.url); await page.getByRole("button", { name: "Continue" }).click(); - await page.getByRole("button", { name: "Save and continue" }).click(); await page.getByRole("button", { name: /Check link/i }).click(); // A no-auth read-only provider can complete the access/install defaults in // one commit. Other methods exercise the same intermediate steps in the diff --git a/tests/e2e/connection-reviews.spec.ts b/tests/e2e/connection-reviews.spec.ts index 4d9fb97230..385f1a4a1d 100644 --- a/tests/e2e/connection-reviews.spec.ts +++ b/tests/e2e/connection-reviews.spec.ts @@ -149,7 +149,6 @@ for (const journey of [ .getByPlaceholder("https://example.com/actions") .fill(provider.url); await page.getByRole("button", { name: "Continue", exact: true }).click(); - await page.getByRole("button", { name: "Save and continue" }).click(); await page.getByRole("button", { name: /Check link/i }).click(); await expect( page.getByRole("heading", { name: /is ready/i }), diff --git a/tests/e2e/execution-recovery/recovery.spec.ts b/tests/e2e/execution-recovery/recovery.spec.ts index 9af3b145d3..0ef482333e 100644 --- a/tests/e2e/execution-recovery/recovery.spec.ts +++ b/tests/e2e/execution-recovery/recovery.spec.ts @@ -324,11 +324,11 @@ for (const journey of [ await page .getByRole("button", { name: "Continue", exact: true }) .click(); + await page.getByRole("button", { name: "Change", exact: true }).click(); await page.getByRole("radio", { name: "Just agents I pick" }).click(); await page.getByRole("button", { name: /Select agents/ }).click(); await page.getByRole("checkbox", { name: /Archive holder/ }).check(); await page.keyboard.press("Escape"); - await page.getByRole("button", { name: "Save and continue" }).click(); await page.getByRole("button", { name: /Check link/i }).click(); await expect( page.getByRole("heading", { name: /is ready/i }), diff --git a/tests/e2e/in-feed-native/connections.spec.ts b/tests/e2e/in-feed-native/connections.spec.ts index 7edc9249f2..c242a14ea6 100644 --- a/tests/e2e/in-feed-native/connections.spec.ts +++ b/tests/e2e/in-feed-native/connections.spec.ts @@ -71,11 +71,11 @@ for (const journey of ['connect', 'decline', 'restart'] as const) test(`fresh na await custom.getByRole('button', { name: 'Connect your own MCP server' }).click(); await page.getByPlaceholder('https://example.com/actions').fill(`http://127.0.0.1:${port}/`); await page.getByRole('button', { name: 'Continue', exact: true }).click(); + await page.getByRole('button', { name: 'Change', exact: true }).click(); await page.getByRole('radio', { name: 'Just agents I pick' }).click(); await page.getByRole('button', { name: /Select agents/ }).click(); await page.getByRole('checkbox', { name: /Archive holder/ }).check(); await page.keyboard.press('Escape'); - await page.getByRole('button', { name: 'Save and continue' }).click(); await page.getByRole('button', { name: /Check link/i }).click(); await expect(page.getByRole('heading', { name: /is ready/i })).toBeVisible({ timeout: 30_000 }); const [connection] = (await api(`/companies/${company.id}/tools/connections`)).connections; diff --git a/ui/src/features/connections/ConnectionSetupFlow.tsx b/ui/src/features/connections/ConnectionSetupFlow.tsx index 2864c7f8cd..2562372c36 100644 --- a/ui/src/features/connections/ConnectionSetupFlow.tsx +++ b/ui/src/features/connections/ConnectionSetupFlow.tsx @@ -74,6 +74,7 @@ import { resolveAuthorizationTarget } from "@/lib/authorizationUrl"; import { navigateTopLevel } from "@/lib/browserNavigation"; import { prepareOAuthNavigation, savePendingCloudHandoff } from "@/lib/oauthHandoff"; import { redactUrlSecrets } from "@/lib/redact-url-secrets"; +import { askFirstCatalogEntryIdsFor } from "./connection-defaults"; import { AppLogo } from "@/pages/apps/AppLogo"; import { appApplicationSourceSlug } from "@/pages/apps/app-definition-display"; import { UnverifiedServerBadge } from "@/pages/apps/UnverifiedServerBadge"; @@ -102,7 +103,7 @@ import { } from "@/pages/apps/generic-mcp-connect"; import { autoExtendNotice, INSTALL_ALL_WARNING, installInfoNotice, installPayload } from "@/lib/tool-installs"; -type Step = "gallery" | "access" | "key" | "success"; +type Step = "gallery" | "key" | "success"; export type OAuthConnectPhase = "entry" | "starting" | "redirecting" | "error"; type EnrollmentAccessState = { @@ -184,7 +185,6 @@ function oauthCallbackErrorMessage(outcome: string | null, code: string | null): } const ROUTE_STAGE_BY_STEP: Partial> = { - access: "access", key: "setup", success: "complete", }; @@ -196,10 +196,8 @@ export function requestedConnectionInitialStep(input: { hasPrefilledLink: boolean; zapierSource: boolean; }): Step { - if (input.requestedAppKey) { - return input.resumeConnectionId || input.routeStage === "setup" ? "key" : "access"; - } - return input.hasPrefilledLink || input.zapierSource ? "access" : "gallery"; + if (input.requestedAppKey) return "key"; + return input.hasPrefilledLink || input.zapierSource ? "key" : "gallery"; } export function requestedConnectionEntry(input: { @@ -305,24 +303,23 @@ function withConnectionIntent(href: string, interactionId?: string | null): stri type AppAccessSelection = "all_agents" | { agentIds: string[] }; -// Access comes before credentials so the reader knows what identity and reach -// the secret is about to get before they share it (PAP-17835). -const STEP_LABELS = ["Pick app", "Access", "Add your key"]; +// PAP-659: identity and agent reach are no longer a step. They are resolved to +// a default, stated in one line above the primary action, and changed either in +// the Advanced disclosure on this screen or on the Permissions tab afterwards. +const STEP_LABELS = ["Pick app", "Add your key"]; const STEP_INDEX: Record, number> = { gallery: 0, - access: 1, - key: 2, -}; -const SELECTED_APP_STEP_INDEX: Record, number> = { - access: 0, key: 1, }; -const ZAPIER_STEP_LABELS = ["Access", "Add MCP URL"]; +const SELECTED_APP_STEP_INDEX: Record, number> = { + key: 0, +}; +const ZAPIER_STEP_LABELS = ["Add MCP URL"]; // Waiting for browser sign-in happens *inside* the flow's last step, so the // waiting screen reuses the step model of the flow that opened it. It must not -// append a trailing step the flow never lands on: a stepper that grows from two -// dots to three the moment you press Connect reads as a step you missed. -const OAUTH_SIGN_IN_STEP_LABELS = ["Access", "Sign in"]; +// append a trailing step the flow never lands on: a stepper that grows from one +// dot to two the moment you press Connect reads as a step you missed. +const OAUTH_SIGN_IN_STEP_LABELS = ["Sign in"]; /** * Which identity a fresh connection should default to (PAP-17835). @@ -394,16 +391,13 @@ function availableToolConnectionMethod( function recommendedSetupConnectionMethod( methods: readonly ConnectionMethodDef[], ): ConnectionMethodDef | null { - const recommended = getRecommendedConnectionMethod(methods); - // Capability choices (for example Google Workspace read versus write) have - // an intentional default. Unrelated region/authentication variants should - // still ask the operator to choose unless only one is available. A method - // that supports an agent-owned identity must also be selected before the - // Access step: that ownership decision cannot be represented by a legacy - // compatibility method such as GitHub's advanced PAT option. - return methods.length === 1 || recommended?.capabilityProfile || recommended?.grantKinds?.includes("agent") - ? recommended - : null; + // PAP-659 C1: every connector arrives with a method already chosen. + // `getRecommendedConnectionMethod` already ranks managed/one-click OAuth over + // customer-owned OAuth over API keys, and write/draft capability over read; + // this used to throw that ranking away for multi-method apps and ask the + // operator instead. The alternates are still reachable, in the Advanced + // disclosure, so nothing became unavailable — it just stopped blocking. + return getRecommendedConnectionMethod(methods); } function recommendedManagedConnectorMethod( @@ -711,6 +705,9 @@ function StandardConnectionSetupFlow({ const [authorizationHost, setAuthorizationHost] = useState(null); const directOAuthAccessConfirmedRef = useRef(false); const directOAuthRetryingRef = useRef(false); + // A draft that sign-in already created, to be resumed with an operator's own + // OAuth client when automatic registration was refused. + const customerClientResumeRef = useRef(null); const hydratedResumeConnectionIdRef = useRef(null); const [hydratedResumeConnectionId, setHydratedResumeConnectionId] = useState(null); const oauthPopupRef = useRef(null); @@ -902,7 +899,7 @@ function StandardConnectionSetupFlow({ resetGenericAuthState(); setCredentials({}); setConnectResult(null); - setStep("access"); + setStep("key"); navigate(withConnectionIntent("/apps/connect?source=zapier", connectionIntentId)); return; } @@ -931,7 +928,7 @@ function StandardConnectionSetupFlow({ setInstallAgentIds(new Set(requestedAgentId ? [requestedAgentId] : [])); setInstallChoice(requestedAgentId ? "specific" : "all"); setGrantKind(reconnectGrantKind ?? defaultGrantKindFor(initialMethod, Boolean(requestedAgentId))); - setStep("access"); + setStep("key"); navigate( credentialSource === "vercel_connect" ? withConnectionIntent(vercelConnectSourceHref(picked.slug), connectionIntentId) @@ -1298,10 +1295,13 @@ function StandardConnectionSetupFlow({ ? configValues : undefined, applicationId: prefill.applicationId, - ...(resumeConnectionId ? { resumeConnectionId } : reconnectConnectionId ? { reconnectConnectionId } : {}), + ...((resumeConnectionId ?? customerClientResumeRef.current) + ? { resumeConnectionId: resumeConnectionId ?? customerClientResumeRef.current! } + : reconnectConnectionId ? { reconnectConnectionId } : {}), ...(requestedGrantKind !== "organization" ? { grantKind: requestedGrantKind } : {}), ...(requestedGrantKind === "agent" ? { subjectAgentId: [...installAgentIds][0] } : {}), }); + customerClientResumeRef.current = null; } else { const genericPayload = genericConnectPayload({ link: linkUrl, @@ -1412,7 +1412,12 @@ function StandardConnectionSetupFlow({ const guidance = genericConnectGuidance(code, error instanceof Error ? error.message : null); setLinkGuidance(guidance); setGenericOAuthPending(false); - if (guidance.focus === "credentials") setLinkAdvancedOpen(true); + if (guidance.focus === "credentials") { + // The probe is the answer to "does it need a key?", so the field + // appears now rather than being offered as a guess beforehand. + setLinkNeedsKey(true); + setLinkAdvancedOpen(true); + } return; } pushToast({ @@ -1599,10 +1604,6 @@ function StandardConnectionSetupFlow({ zapierSource, ]); - useEffect(() => { - if (reconnectConnection?.connectionPurpose === "ai" && step === "access") setStep("key"); - }, [reconnectConnection?.connectionPurpose, step]); - // Resume the exact method and non-secret provider configuration that the // interrupted draft already chose. Secrets are intentionally never read back // into the browser; credential-based methods ask for a replacement value. @@ -1693,16 +1694,10 @@ function StandardConnectionSetupFlow({ const enabledIds = Object.entries(enabledMap) .filter(([, on]) => on) .map(([id]) => id); - const askFirstRiskLevels = new Set( - Array.isArray(connected.suggestedDefaults.askFirstRiskLevels) - ? connected.suggestedDefaults.askFirstRiskLevels.filter( - (riskLevel): riskLevel is string => typeof riskLevel === "string", - ) - : [], + const askFirstIds = askFirstCatalogEntryIdsFor( + connected, + (catalogEntryId) => Boolean(enabledMap[catalogEntryId]), ); - const askFirstIds = connected.actions.canMakeChanges - .filter((action) => enabledMap[action.catalogEntryId] && askFirstRiskLevels.has(action.riskLevel)) - .map((action) => action.catalogEntryId); // The Access step asks one question about agent reach, so profile access // and installs are committed to the same target set instead of drifting // apart behind two separate wizard screens. @@ -1726,9 +1721,9 @@ function StandardConnectionSetupFlow({ }, onError: (error) => { // Creation must feel transactional: a failed commit returns the operator - // to Access with their identity and agent selections intact rather than - // stranding them on a half-made connection. - setAppStep("access"); + // to the connect screen with their identity and agent selections intact + // rather than stranding them on a half-made connection. + setAppStep("key"); pushToast({ title: "Couldn’t finish setup", body: error instanceof Error ? error.message : "Please try again.", @@ -1911,6 +1906,99 @@ function StandardConnectionSetupFlow({ ); } + const credentialSourceMethods = connectionMethodsForCredentialSource(entry, credentialSource); + const setupCredentialSourceMethods = preEnrollmentManagedMethod + ? [preEnrollmentManagedMethod] + : credentialSourceMethods; + + // The stated default's identity line only makes sense when there *is* a + // credential, so it reads the selected method's auth kind. + const accessStepMethod = entry + ? (connectionMethodKey + ? setupCredentialSourceMethods.find((m) => m.key === connectionMethodKey) ?? null + : setupCredentialSourceMethods[0] ?? null) + : null; + const accessStepAuthKind: ToolConnectionAuthKind = entry + ? accessStepMethod?.auth ?? "none" + : linkAuthMode === "none" + ? "none" + : linkAuthMode === "oauth" + ? "oauth" + : "api_key"; + // PAP-659 C0: the resolved default is stated, not asked. `Change` opens the + // same controls the deleted Access step owned, inline and never blocking. + const renderConnectionDefaults = step === "key" ? ( + (extra?: ReactNode, forceOpen?: boolean) => ( + Boolean(reason))} + authKind={accessStepAuthKind} + grantKinds={fixedGrantKind ? [fixedGrantKind] : accessStepMethod?.grantKinds} + grantKind={effectiveGrantKind} + setGrantKind={setGrantKind} + installChoice={installChoice} + setInstallChoice={setInstallChoice} + installAgentIds={installAgentIds} + setInstallAgentIds={setInstallAgentIds} + lockedAgentId={requestedAgentId} + capabilities={galleryQuery.data?.capabilities} + githubIdentity={entry?.slug === "github"} + identityLoading={Boolean(automaticOAuthEntry) && directOAuthLookupPending} + preserveAgentAccess={Boolean(automaticOAuthEntry && (resumableOAuthConnection || reconnectConnection))} + disabled={connectMutation.isPending || oauthStartMutation.isPending} + /> + ) + ) : null; + // A provider can advertise registration and still refuse this deployment's + // callback (Asana refuses hosted ones). When the method also accepts an + // operator's own OAuth client, that client is the recovery path, so it sits + // in the same Advanced panel and opens itself once sign-in has failed. + const automaticCustomerClientMethod = automaticOAuthEntry + && entryAutomaticOAuthMethod + && connectionMethodAcceptsCustomerOAuthClient(entryAutomaticOAuthMethod) + ? entryAutomaticOAuthMethod + : null; + const automaticCustomerClientFields = automaticCustomerClientMethod && automaticOAuthEntry ? ( + + ) : null; + const connectionDefaults = renderConnectionDefaults?.() ?? null; + const curatedOAuthDefaults = automaticCustomerClientFields + ? renderConnectionDefaults?.( + automaticCustomerClientFields, + oauthPhase === "error" || curatedOAuthClientId.trim().length > 0, + ) ?? null + : connectionDefaults; + const showCuratedOAuthState = Boolean( automaticOAuthEntry && step === "key" @@ -1950,15 +2038,42 @@ function StandardConnectionSetupFlow({ } : undefined} authorizationHost={authorizationHost} authorizationUrl={authorizationFallbackUrl} + guidance={entry?.slug === "railway" && accessStepMethod ? ( +
+

{accessStepMethod.guidanceMd}

+
    + {accessStepMethod.warnings?.map((warning) =>
  • {warning}
    • )} +
+
+ ) : null} + defaults={curatedOAuthDefaults} onOpenAuthorization={openAuthorizationTab} onRetry={async () => { + const firstAttempt = !directOAuthAccessConfirmedRef.current; + directOAuthAccessConfirmedRef.current = true; setOAuthError(null); setOAuthPhase("starting"); const connection = connectResult?.connection ?? resumableOAuthConnection; + if (connection && automaticCustomerClientFields && curatedOAuthClientId.trim()) { + // The draft exists but its client must change: resume it through + // connect so the operator's client replaces the refused registration. + customerClientResumeRef.current = connection.id; + connectApp(automaticOAuthEntry); + return; + } if (connection) { startOAuth(connection); return; } + if ( + firstAttempt + && !resumeConnectionId + && !applicationsQuery.isError + && !connectionsQuery.isError + ) { + connectApp(automaticOAuthEntry); + return; + } // The create request may have reached the server even when its // response did not reach the browser. Re-read both resources before @@ -1988,20 +2103,16 @@ function StandardConnectionSetupFlow({ ? { applicationId: prefill.applicationId, draftOnly: true } : {}, ); - if (!directOAuthAccessConfirmedRef.current && !resumeConnectionId) { - if (refreshedConnection) { - setGrantKind( - refreshedConnection.credentialPolicy === "per_user" - ? "user" - : refreshedConnection.credentialPolicy === "per_agent" - ? "agent" - : "organization", - ); - } - setOAuthPhase("entry"); - setOAuthError(null); - setStep("access"); - return; + if (refreshedConnection && !resumeConnectionId) { + // Adopt the durable draft's identity before resuming it, so the + // stated default on screen matches what is about to be authorized. + setGrantKind( + refreshedConnection.credentialPolicy === "per_user" + ? "user" + : refreshedConnection.credentialPolicy === "per_agent" + ? "agent" + : "organization", + ); } if (refreshedConnection) { startOAuth(refreshedConnection); @@ -2016,7 +2127,7 @@ function StandardConnectionSetupFlow({ oauthHandoffAbortRef.current?.abort(); setOAuthPhase("entry"); setOAuthError(null); - setAppStep("access"); + backToGallery(); }} onCancel={() => { oauthHandoffAbortRef.current?.abort(); @@ -2042,6 +2153,7 @@ function StandardConnectionSetupFlow({ error={oauthError} authorizationHost={authorizationHost} authorizationUrl={authorizationFallbackUrl} + defaults={connectionDefaults} onOpenAuthorization={openAuthorizationTab} onRetry={() => { setOAuthError(null); @@ -2074,10 +2186,6 @@ function StandardConnectionSetupFlow({ connectResult?.application.name ?? entry?.name ?? (linkName.trim() || defaultGenericMcpName(linkUrl) || "this app"); - const credentialSourceMethods = connectionMethodsForCredentialSource(entry, credentialSource); - const setupCredentialSourceMethods = preEnrollmentManagedMethod - ? [preEnrollmentManagedMethod] - : credentialSourceMethods; const credentialSourceApps = vercelConnectMode ? visibleGalleryApps.filter( (app) => connectionMethodsForCredentialSource(app, credentialSource).length > 0, @@ -2091,7 +2199,7 @@ function StandardConnectionSetupFlow({ : undefined; const aiMethod = reconnectAiMethod ?? entry?.methods.find(method => method.key === connectionMethodKey)?.ai ?? (!connectionMethodKey && entry?.methods.every(method => method.ai) ? entry.methods[0]?.ai : undefined); - const credentialStep = entry ? renderCredentialStep?.({ app: entry, name: galleryName || entry.name, grantKind: effectiveGrantKind, agentIds: [...installAgentIds], allAgents: installChoice === "all", onBack: () => setAppStep("access") }) ?? (aiMethod && selectedCompanyId ? <> backToGallery() }) ?? (aiMethod && selectedCompanyId ? <> onCancel ? onCancel() : navigate("/apps")} onComplete={result => { onComplete?.({ connectionId: result.connectionId }); if (!onComplete) navigate(`/apps/${result.connectionId}/permissions`); }} /> : undefined) : undefined; + // One screen per connector (PAP-659): a selected app has a single step, so + // its stepper collapses to nothing rather than showing one lonely dot. const stepLabels = reconnectConnection?.connectionPurpose === "ai" ? ["Reconnect account"] : credentialStep !== undefined - ? ["Access", "Connect account"] + ? ["Connect account"] : zapierSource ? ZAPIER_STEP_LABELS : entry && setupCredentialSourceMethods.length > 1 - ? ["Access", "Choose connection"] + ? ["Choose connection"] : entry && setupCredentialSourceMethods[0]?.auth === "oauth" - ? ["Access", "Sign in"] + ? ["Sign in"] : isGoogleSheetsRobotMethod(entry, connectionMethodKey) - ? ["Access", "Share sheet"] + ? ["Share sheet"] : entry - ? ["Access", "Add your key"] + ? ["Add your key"] : STEP_LABELS; - // The Access step's identity question only makes sense when there *is* a - // credential, so it reads the selected method's auth kind. - const accessStepMethod = entry - ? (connectionMethodKey - ? setupCredentialSourceMethods.find((m) => m.key === connectionMethodKey) ?? null - : setupCredentialSourceMethods[0] ?? null) - : null; - const accessStepAuthKind: ToolConnectionAuthKind = entry - ? accessStepMethod?.auth ?? "none" - : linkAuthMode === "none" - ? "none" - : linkAuthMode === "oauth" - ? "oauth" - : "api_key"; - // Name the actual next effect: multi-method apps and enrollment still have - // a local setup screen, even when OAuth is already the selected method. - const accessContinuesToProvider = Boolean(directOAuthEntry); - const accessSubmitLabel = accessContinuesToProvider - ? `Continue to ${entry?.name ?? "sign-in"}` - : accessStepAuthKind === "oauth" ? "Continue" : "Save and continue"; - const stepIndex = reconnectConnection?.connectionPurpose === "ai" ? 0 : (zapierSource || entry) && step !== "gallery" && step !== "success" ? SELECTED_APP_STEP_INDEX[step] : step === "success" @@ -2150,7 +2239,9 @@ function StandardConnectionSetupFlow({ ? vercelConnectMode ? "Choose a reviewed app to connect through Vercel." : "Pick the app you want your agents to use." - : `Step ${stepIndex + 1} of ${stepLabels.length}` + : stepLabels.length <= 1 + ? "Connect now — permissions and access are yours to change afterwards." + : `Step ${stepIndex + 1} of ${stepLabels.length}` } step={step} activeIndex={stepIndex} @@ -2193,7 +2284,7 @@ function StandardConnectionSetupFlow({ setInstallAgentIds(new Set(requestedAgentId ? [requestedAgentId] : [])); setInstallChoice(requestedAgentId ? "specific" : "all"); setGrantKind(reconnectGrantKind ?? (requestedAgentId ? "user" : "organization")); - setStep("access"); + setStep("key"); }} /> )} @@ -2205,7 +2296,7 @@ function StandardConnectionSetupFlow({ This instance is connected to Paperclip, but {entry.name} sign-in is not currently available. Try again shortly or contact your instance administrator.

- + }
- {step !== "gallery" && ( + {step !== "gallery" && labels.length > 1 && ( // A landmark with stable hooks, so the step model can be read without // guessing at Tailwind classes. The dots are decoration — the label // line below already says the same thing, so announcing both would @@ -2552,7 +2615,9 @@ export function OAuthConnectStateScreen({ recoveryActions, authorizationHost, authorizationUrl, - steps = { labels: OAUTH_SIGN_IN_STEP_LABELS, activeIndex: 1 }, + steps = { labels: OAUTH_SIGN_IN_STEP_LABELS, activeIndex: 0 }, + guidance, + defaults, onRetry, onOpenAuthorization, onBack, @@ -2581,6 +2646,10 @@ export function OAuthConnectStateScreen({ * sign-in model for hosts that have no wizard of their own. */ steps?: { labels: string[]; activeIndex: number }; + /** Provider-specific warnings that must be read before authorizing. */ + guidance?: ReactNode; + /** The stated default and its Advanced disclosure (PAP-659 C0). */ + defaults?: ReactNode; onOpenAuthorization?: () => void; onRetry: () => void; onBack: () => void; @@ -2643,6 +2712,9 @@ export function OAuthConnectStateScreen({ + {phase === "entry" && guidance ?
{guidance}
: null} + {phase === "entry" || phase === "error" ? defaults : null} + {phase === "error" && recoveryActions && (recoveryActions.installationUrl || recoveryActions.managementUrl) ? (
{recoveryActions.installationUrl ? ( @@ -2669,6 +2741,7 @@ export function OAuthConnectStateScreen({ {phase === "entry" ? resuming ? `Finish with ${serverName}` : `Continue to ${serverName}` : "Try again"} + {phase === "entry" ?
+ {defaults} +
) : capabilityMethods.length > 1 ? ( + // PAP-659 C1: the ranked default is already selected. This stays as the + // way to pick something else, one disclosure away, rather than a question + // the screen opens with.
Choose a connection method to continue.

}
) : null; + const hasAdvancedSettings = advancedConfigFields.length > 0 + || optionalCustomerOAuthClient + || hasAlternateMethods; + // Keep the disclosure open when what is inside it is load-bearing right now: + // a non-default method in use, or a selection the connector still needs. + const forceAdvancedOpen = usingCustomGoogleOAuth || !hasMethodSelection; + const advancedSettings = hasAdvancedSettings ? ( +
+ {capabilitySelection} + {authenticationSelection} + {advancedConfigFields.map((field) => ( + onConfigChange({ ...configValues, [field.key]: value })} + /> + ))} + {optionalCustomerOAuthClient ? ( + + ) : null} +
+ ) : null; + const defaults = renderDefaults?.(advancedSettings, forceAdvancedOpen) ?? null; if (isGoogleSheetsRobotMethod(entry, method)) { const parsed = parseGoogleSheetIds(googleSheetsLinks); @@ -3589,6 +3690,8 @@ function KeyStep({
+ {defaults} +
@@ -4236,7 +4318,116 @@ export function AccessStepContent({ {submitLabel} {!pending && continuesToProvider ?
} + + ); +} + +/** + * The stated default (PAP-659 C0). + * + * The Access step used to ask two questions whose answers were already correct + * on arrival. Instead of asking, the resolved answer is printed in one line + * above the primary action and repeated on the completion screen, so the reach + * being granted is said twice on the happy path without costing a click. + */ +export function connectionDefaultSummarySentence(input: { + grantKind: ConnectionGrantKind; + authKind: ToolConnectionAuthKind; + installChoice: "specific" | "all"; + installCount: number; + lockedAgentId?: string | null; + preserveAgentAccess?: boolean; +}): string { + const identity = input.authKind === "none" + ? "No sign-in needed" + : input.grantKind === "user" + ? "Connects as you" + : input.grantKind === "agent" + ? "Connects as a dedicated agent account" + : "Connects for everyone in your organization"; + const reach = input.preserveAgentAccess + ? "agent access stays as it is" + : input.lockedAgentId + ? "available to the agent that asked for it" + : input.installChoice === "all" + ? "available to all agents" + : input.installCount === 0 + ? "no agents selected yet" + : `available to ${input.installCount} selected ${input.installCount === 1 ? "agent" : "agents"}`; + return `${identity}, ${reach}.`; +} + +/** + * The stated default plus the collapsed Advanced disclosure that replaced the + * Access step. It sits in the same place on every connector and never blocks + * the primary action: opening it is optional, and everything inside it can + * also be changed on the Permissions tab after connecting. + */ +export function ConnectionAccessDefaults({ + companyId, + agents, + sentence, + notice, + extra, + forceOpen = false, + disabled = false, + ...accessProps +}: Omit[0], "agents" | "agentsLoading" | "submitLabel" | "onBack" | "onContinue" | "pending" | "continuesToProvider" | "hideFooter" | "bare"> & { + companyId: string; + /** + * Supplied by callers that already hold the agent list — design specimens and + * review stories, which must not reach the network. Omitted in the product, + * where the disclosure fetches its own. + */ + agents?: AgentMultiSelectOption[]; + sentence: string; + /** Shown when the resolved default genuinely cannot apply. Never a step. */ + notice?: string[]; + /** + * Connector-specific advanced settings — alternate methods, access level, + * provider config, a customer-owned OAuth client. They share this one + * disclosure rather than opening a second one on the same screen. + */ + extra?: ReactNode; + /** Something inside is load-bearing right now, so do not hide it. */ + forceOpen?: boolean; + disabled?: boolean; +}) { + const [open, setOpen] = useState(false); + const expanded = open || forceOpen; + return ( +
+
+

{sentence}

+
+ {(notice ?? []).map((reason) => ( +

{reason}

+ ))} + + + {expanded ? + +
+ {extra} + {agents + ? {}} onContinue={() => {}} /> + : {}} onContinue={() => {}} />} +
+ +
); } @@ -4299,6 +4490,7 @@ export function ConnectionSetupCompletionScreen({ logoUrl, darkLogoUrl, summary, + statedDefault, onDone, }: { appName: string; @@ -4306,6 +4498,11 @@ export function ConnectionSetupCompletionScreen({ darkLogoUrl?: string | null; /** Identity / Available to / Actions, as three lines rather than badges. */ summary: Array<{ label: string; value: string }>; + /** + * The same sentence the connect screen printed above its primary action, so + * the reach that was granted is stated twice on the happy path (PAP-659 C0). + */ + statedDefault?: string; onDone: () => void; }) { return ( @@ -4317,6 +4514,7 @@ export function ConnectionSetupCompletionScreen({

{appName} is ready.

+ {statedDefault ?

{statedDefault}

: null}
{summary.map((line) => (
diff --git a/ui/src/features/connections/connection-defaults.test.ts b/ui/src/features/connections/connection-defaults.test.ts new file mode 100644 index 0000000000..587309e420 --- /dev/null +++ b/ui/src/features/connections/connection-defaults.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { askFirstCatalogEntryIdsFor } from "./connection-defaults"; + +const action = (catalogEntryId: string, riskLevel: string) => + ({ catalogEntryId, riskLevel }) as never; + +describe("askFirstCatalogEntryIdsFor", () => { + const result = { + actions: { + readOnly: [action("read-1", "read")], + canMakeChanges: [ + action("write-1", "write"), + action("destroy-1", "destructive"), + action("odd-1", "medium"), + ], + }, + suggestedDefaults: { askFirstRiskLevels: ["write", "destructive", "high", "critical"] }, + }; + + it("gates the write and destructive actions the server named", () => { + expect(askFirstCatalogEntryIdsFor(result, () => true)).toEqual(["write-1", "destroy-1"]); + }); + + it("leaves a risk level the policy did not name alone", () => { + // `medium` is not in the default set; gating it would be the classifier + // making policy instead of the policy function. + expect(askFirstCatalogEntryIdsFor(result, () => true)).not.toContain("odd-1"); + }); + + it("never gates an action that is switched off", () => { + expect(askFirstCatalogEntryIdsFor(result, (id) => id !== "write-1")).toEqual(["destroy-1"]); + }); + + it("gates nothing when the server sends an open policy", () => { + expect( + askFirstCatalogEntryIdsFor({ ...result, suggestedDefaults: { askFirstRiskLevels: [] } }, () => true), + ).toEqual([]); + }); + + it("gates nothing rather than throwing when the field is missing or malformed", () => { + expect(askFirstCatalogEntryIdsFor({ ...result, suggestedDefaults: {} }, () => true)).toEqual([]); + expect( + askFirstCatalogEntryIdsFor({ ...result, suggestedDefaults: undefined as never }, () => true), + ).toEqual([]); + expect( + askFirstCatalogEntryIdsFor({ ...result, suggestedDefaults: { askFirstRiskLevels: "write" } }, () => true), + ).toEqual([]); + }); +}); diff --git a/ui/src/features/connections/connection-defaults.ts b/ui/src/features/connections/connection-defaults.ts new file mode 100644 index 0000000000..85840c17d7 --- /dev/null +++ b/ui/src/features/connections/connection-defaults.ts @@ -0,0 +1,30 @@ +import type { ConnectToolAppResult } from "@paperclipai/shared"; + +/** + * Which of a fresh connection's actions start behind a human approval (PAP-659 + * C6a/C7). + * + * The runtime gate and its approval card already existed; what was missing was + * a default with anything behind it. This lives outside the wizard because two + * different setup paths commit connections — the catalog flow and the + * remote-MCP gateway flow — and a gate that is armed on only one of them is a + * gate you cannot reason about. The risk levels come from the server's + * `suggestedDefaults` rather than a constant here, so retuning the policy stays + * a one-line change in `recommendedDefaultsForApp`. + */ +export function askFirstCatalogEntryIdsFor( + result: Pick, + isEnabled: (catalogEntryId: string) => boolean, +): string[] { + const riskLevels = new Set( + Array.isArray(result.suggestedDefaults?.askFirstRiskLevels) + ? result.suggestedDefaults.askFirstRiskLevels.filter( + (riskLevel): riskLevel is string => typeof riskLevel === "string", + ) + : [], + ); + if (riskLevels.size === 0) return []; + return result.actions.canMakeChanges + .filter((action) => isEnabled(action.catalogEntryId) && riskLevels.has(action.riskLevel)) + .map((action) => action.catalogEntryId); +} diff --git a/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx b/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx index a2ef5af68a..a0cd9ece82 100644 --- a/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx +++ b/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx @@ -8,11 +8,22 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { RemoteMcpManagement } from "./RemoteMcpManagement"; -import { AccessStepContent, StepHeader } from "../ConnectionSetupFlow"; +import { + AccessStepContent, + ConnectionAccessDefaults, + connectionDefaultSummarySentence, + StepHeader, +} from "../ConnectionSetupFlow"; import type { RemoteMcpProvider } from "./providers"; import type { RemoteMcpSetupActions, RemoteMcpSetupState } from "./types"; -const steps = ["access", "connect"] as const; +/** + * PAP-659 C0: the connect path is one screen. `access` is still a real screen, + * but only as management after the connection exists — the way in states the + * resolved default and puts its controls in the Advanced disclosure, the same + * as every catalog connector. + */ +const steps = ["connect"] as const; const selectClass = "h-9 w-full rounded-md border border-input bg-background px-3 text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring disabled:opacity-50"; function FieldHelp({ label, children }: { label: string; children: ReactNode }) { @@ -25,7 +36,8 @@ function FieldHelp({ label, children }: { label: string; children: ReactNode }) /** Controlled presentation shared by provider setup, configuration imports and review stories. * Authentication, persistence and calls belong to the controller, never these views. */ -export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agents, connectionId, fixedGrantKind, lockedAgentId, host = "page", authorizationUrl, upstreamServiceName, onCancel }: { +export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agents, companyId, connectionId, fixedGrantKind, lockedAgentId, host = "page", authorizationUrl, upstreamServiceName, onCancel }: { + companyId: string; onCancel?: () => void; upstreamServiceName?: string; host?: "page" | "dialog"; @@ -54,6 +66,48 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent ; const footer = (children: ReactNode) => {children}; + /** + * The stated default and the one Advanced disclosure (PAP-659 C0). + * + * Step 1 of this work learned the lesson the hard way on Gmail: adding an + * access disclosure beside a connector's existing "Advanced authentication" + * panel leaves two of them on one screen, which is worse than the step it + * replaced. So the provider's authentication settings are passed in here as + * `extra` and share a single panel with the access controls. + */ + // Deliberately not derived from `s.auth`. A gateway URL can carry a personal + // token even when the method declares `auth: "none"`, so the shared-vs-mine + // credential choice has to stay offered; deriving "none" here would silently + // remove it and pin every Zapier connection to the organization. + const authKind = "oauth"; + const defaults = (extra: ReactNode) => { if (grantKind !== "agent") change({ grantKind }); }} + installChoice={s.allAgents ? "all" : "specific"} + setInstallChoice={(choice) => change({ allAgents: choice === "all" })} + installAgentIds={new Set(s.agentIds)} + setInstallAgentIds={(ids) => change({ agentIds: [...ids] })} + lockedAgentId={lockedAgentId} + preserveAgentAccess={s.setupComplete} + />; + const error = s.connectStatus === "invalid_url" ? { title: "Enter a valid MCP URL", body: "Paste the complete server URL, including https:// or http://. A dashboard page is not an MCP endpoint." } : s.connectStatus === "oauth_failed" ? { title: `${provider.name} couldn’t connect`, body: "Authorization did not complete. Your saved connection is still here, so you can try again." } : s.connectStatus === "rejected" ? { title: "Credentials were rejected", body: `Check or replace the credentials from ${provider.name}, then reconnect. Your agent access and tool choices are preserved.` } @@ -63,8 +117,8 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent return
= 0 && !s.setupComplete ? `Step ${currentStep + 1} of 2` : s.step === "draft" ? `Your ${provider.name} setup is ready to resume.` : s.step === "permissions" ? `Connected${s.identity ? ` as ${s.identity}` : ""} · ${s.tools.length} actions available` : `Manage this ${provider.name} connection.`} - step={currentStep >= 0 && !s.setupComplete ? "access" : "gallery"} activeIndex={currentStep} labels={["Access", "Connect"]} onCancel={busy || s.step === "management" || s.step === "permissions" || s.step === "draft" ? undefined : onCancel ?? a.saveExit} /> + subtitle={currentStep >= 0 && !s.setupComplete ? `Paperclip will use ${provider.name} on your behalf.` : s.step === "draft" ? `Your ${provider.name} setup is ready to resume.` : s.step === "permissions" ? `Connected${s.identity ? ` as ${s.identity}` : ""} · ${s.tools.length} actions available` : `Manage this ${provider.name} connection.`} + step={currentStep >= 0 && !s.setupComplete ? "key" : "gallery"} activeIndex={currentStep} labels={steps.map(() => "Connect")} onCancel={busy || s.step === "management" || s.step === "permissions" || s.step === "draft" ? undefined : onCancel ?? a.saveExit} />
{upstreamServiceName && {provider.name} is an external service that handles the connection and requests to {upstreamServiceName}. After connecting, the agent will verify the app and guide you through any additional authorization.} {s.notice &&

{s.notice}

} @@ -81,7 +135,7 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent enabledIds={new Set(s.tools.filter((entry) => s.permissions[entry.id] !== "off").map((entry) => entry.id))} askFirstIds={new Set(s.tools.filter((entry) => s.permissions[entry.id] === "ask_first").map((entry) => entry.id))} disabled={!s.connected} refreshPending={s.refreshing} canConfigure - onSetPermission={(id, next) => change({ permissions: { ...s.permissions, [id]: next === "ask" ? "ask_first" : next } })} + onSetPermission={(ids, next) => change({ permissions: { ...s.permissions, ...Object.fromEntries(ids.map((id) => [id, next === "ask" ? "ask_first" : next])) } })} onReviewQuarantined={() => {}} onRefreshActions={a.refresh} /> }
@@ -105,11 +159,10 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent change({ url: event.target.value })} />

{provider.urlHelp}

-
{ if (event.currentTarget.open !== s.advanced) change({ advanced: event.currentTarget.open }); }}> - Advanced authentication -
+ {defaults(
+

Authentication

{provider.authHelp}

-
change({ auth: event.target.value as RemoteMcpSetupState["auth"] })}> {provider.supportsBrowserAuth && }
{s.auth === "bearer" &&
Paste the token only, without the word Bearer. It is kept in the connection’s credentials.
change({ token: event.target.value })} />
} @@ -122,11 +175,10 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent
)}
} -
- +
)} {busy &&

Connecting and discovering tools…

} - {footer(<>)} + {footer(<>{s.setupComplete ? : })} } } diff --git a/ui/src/features/connections/remote-mcp/RemoteMcpDesignExample.tsx b/ui/src/features/connections/remote-mcp/RemoteMcpDesignExample.tsx index fe822d852c..d5e4fef3ca 100644 --- a/ui/src/features/connections/remote-mcp/RemoteMcpDesignExample.tsx +++ b/ui/src/features/connections/remote-mcp/RemoteMcpDesignExample.tsx @@ -8,7 +8,7 @@ function ConnectExample({ providerId }: { providerId: RemoteMcpProviderId }) { const provider = remoteMcpProviders[providerId]; const [state, setState] = useState({ step: "access", grantKind: "organization", setupComplete: false, url: provider.defaultUrl, - auth: provider.supportsBrowserAuth ? "auto" : "none", token: "", headers: [], advanced: false, connectStatus: "idle", + auth: provider.supportsBrowserAuth ? "auto" : "none", token: "", headers: [], connectStatus: "idle", connected: false, identity: null, allAgents: true, agentIds: [], permissions: {}, tools: [], notice: null, refreshing: false, }); @@ -17,7 +17,7 @@ function ConnectExample({ providerId }: { providerId: RemoteMcpProviderId }) { const actions: RemoteMcpSetupActions = { edit, navigate: (step) => edit({ step }), connect: explain, cancelConnect: explain, openProvider: explain, saveExit: explain, resumeDraft: explain, finish: explain, refresh: explain, reconnect: explain, disconnect: explain, }; - return ; + return ; } /** Design-guide specimen; full state matrix is maintained in each provider's Storybook group. */ diff --git a/ui/src/features/connections/remote-mcp/RemoteMcpProductionSetup.tsx b/ui/src/features/connections/remote-mcp/RemoteMcpProductionSetup.tsx index a78ffb29da..ee5fc9c746 100644 --- a/ui/src/features/connections/remote-mcp/RemoteMcpProductionSetup.tsx +++ b/ui/src/features/connections/remote-mcp/RemoteMcpProductionSetup.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from "react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { REMOTE_MCP_CONNECTOR_METHODS, type ToolConnection } from "@paperclipai/shared"; +import { askFirstCatalogEntryIdsFor } from "../connection-defaults"; import { RemoteMcpAccountChoice } from "./RemoteMcpAccountChoice"; import { readConnectionIntentOAuthOutcome, type ConnectionSetupFlowProps } from "../ConnectionSetupFlow"; import { agentsApi } from "@/api/agents"; @@ -45,11 +46,14 @@ export function RemoteMcpProductionSetup({ providerId, connection, host = "page" const busy = useRef(false); const authorizationUrl = useRef(undefined); const [state, setState] = useState(() => ({ - step: connection ? "connect" : "access", grantKind: connection ? connection.credentialPolicy === "per_user" ? "user" : "organization" : requestedAgentId ? "user" : "organization", + // PAP-659 C0: there is no Access step on the way in. The resolved default + // is stated on the connect screen and changed in its Advanced disclosure; + // `access` survives only as a management screen reached after connecting. + step: "connect", grantKind: connection ? connection.credentialPolicy === "per_user" ? "user" : "organization" : requestedAgentId ? "user" : "organization", setupComplete: Boolean(connection && connection.status !== "draft"), url: typeof connection?.config?.url === "string" ? connection.config?.url : provider.defaultUrl, auth: connection?.config?.mcpAuthMode === "bearer" ? "bearer" : connection?.authKind === "api_key" ? "headers" : provider.supportsBrowserAuth ? "auto" : "none", - token: "", headers: [], advanced: false, connectStatus: oauthOutcome === "denied" ? "cancelled" : oauthOutcome === "failed" ? "oauth_failed" : "idle", connected: false, + token: "", headers: [], connectStatus: oauthOutcome === "denied" ? "cancelled" : oauthOutcome === "failed" ? "oauth_failed" : "idle", connected: false, identity: null, allAgents: true, agentIds: [], permissions: {}, tools: [], notice: connection?.authKind === "api_key" ? "Saved credentials are retained when these fields are left blank. Enter a replacement only to change them." : null, refreshing: false, ...(!connection ? readAccessDraft(accessDraftKey) : {}), ...(requestedAgentId ? { allAgents: false, agentIds: [requestedAgentId] } : {}), @@ -149,9 +153,14 @@ export function RemoteMcpProductionSetup({ providerId, connection, host = "page" popup.current = null; // The server retains existing permissions when reconnecting. Only a fresh setup enables everything. { + const enabled = new Set(result.catalog.filter((tool) => tool.status === "active").map((tool) => tool.id)); await toolsApi.finishApp(selectedCompanyId, result.connectionId, { - enabledCatalogEntryIds: result.catalog.filter((tool) => tool.status === "active").map((tool) => tool.id), - askFirstCatalogEntryIds: [], access: requestedAgentId ? { agentIds: [requestedAgentId] } : state.allAgents ? "all_agents" : { agentIds: state.agentIds }, + enabledCatalogEntryIds: [...enabled], + // PAP-659 C6a/C7: this path used to send an empty list, so the four + // gateway connectors were the one place the armed write gate did not + // apply. The policy now comes from the same helper as the catalog flow. + askFirstCatalogEntryIds: askFirstCatalogEntryIdsFor(result, (id) => enabled.has(id)), + access: requestedAgentId ? { agentIds: [requestedAgentId] } : state.allAgents ? "all_agents" : { agentIds: state.agentIds }, ...(requestedAgentId ? { preserveExistingAccess: true } : {}), }); } @@ -189,5 +198,5 @@ export function RemoteMcpProductionSetup({ providerId, connection, host = "page" if (connection && !installs.data) return

{installs.isError ? "Could not load saved access. Retry before changing this connection." : "Loading saved access…"}

{installs.isError && }
; // Header Cancel abandons unsaved input, including invalid URLs. The separate // Save & exit action persists a resumable draft through actions.saveExit. - return navigate("/apps"))} upstreamServiceName={upstreamServiceName} host={host} lockedAgentId={requestedAgentId} authorizationUrl={host === "dialog" ? authorizationUrl.current : undefined} provider={provider} connectionId={savedConnection.current?.id ?? ""} fixedGrantKind={savedConnection.current ? savedConnection.current.credentialPolicy === "per_user" ? "user" : "organization" : undefined} state={state} actions={actions} agents={agents.data ?? []} />; + return navigate("/apps"))} upstreamServiceName={upstreamServiceName} host={host} lockedAgentId={requestedAgentId} authorizationUrl={host === "dialog" ? authorizationUrl.current : undefined} provider={provider} connectionId={savedConnection.current?.id ?? ""} fixedGrantKind={savedConnection.current ? savedConnection.current.credentialPolicy === "per_user" ? "user" : "organization" : undefined} state={state} actions={actions} agents={agents.data ?? []} />; } diff --git a/ui/src/features/connections/remote-mcp/types.ts b/ui/src/features/connections/remote-mcp/types.ts index 5d94314c67..ebcea37910 100644 --- a/ui/src/features/connections/remote-mcp/types.ts +++ b/ui/src/features/connections/remote-mcp/types.ts @@ -15,7 +15,6 @@ export interface RemoteMcpSetupState { auth: "auto" | "bearer" | "headers" | "none"; token: string; headers: { id: string; name: string; value: string }[]; - advanced: boolean; connectStatus: ConnectStatus; connected: boolean; identity: string | null; diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index 743080ddd2..77b8b3fd5d 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -52,6 +52,7 @@ import { appTabHref, appTabLabel, isAppTabKey, type AppTabKey } from "./app-tabs import { ConnectionProvenanceChip } from "./ConnectionProvenanceChip"; import { IdentitiesSection } from "./app-detail/IdentitiesSection"; import { PermissionsPanel } from "./app-detail/PermissionsPanel"; +import { actionPermissionMutation } from "./app-detail/action-permissions"; import { RailwayAccessPanel } from "./app-detail/RailwayAccessPanel"; import { ReviewPanel } from "./app-detail/ReviewPanel"; import { @@ -691,7 +692,7 @@ export function AppDetail({ renderActions, onReconnect }: { } onSaveAccess={(next) => apply({ access: connection.connectionPurpose === "ai" || managesRemoteMcpAccess ? next : accessIncludingInstalls(next, install) })} onRefreshActions={() => refreshTools.mutate()} - onSetActionPermission={(id, next) => apply(actionPermissionMutation(id, next, enabledIds, askFirstIds))} + onSetActionPermission={(ids, next) => apply(actionPermissionMutation(ids, next, enabledIds, askFirstIds))} onReviewQuarantined={reviewQuarantined} /> {managesRemoteMcpAccess && isRemoteMcpConnectorId(connection.config?.sourceTemplateKey) && , - askFirstIds: Set, -) { - const enabled = new Set(enabledIds); - const askFirst = new Set(askFirstIds); - if (next === "off") { - enabled.delete(id); - askFirst.delete(id); - } else { - enabled.add(id); - if (next === "ask") askFirst.add(id); - else askFirst.delete(id); - } - return { enabled, askFirst }; -} diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index 05f49bb141..aad46401c4 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -53,6 +53,7 @@ const GITHUB_MANAGED = { }; const NOTION = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "notion")!; const ASANA = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "asana")!; +const BOX = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "box")!; const POSTHOG = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "posthog")!; const POSTMAN = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "postman")!; const SHOPIFY = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "shopify")!; @@ -172,11 +173,55 @@ function stepLabelsOnScreen(): string[] { } /** - * Advance past the Access step (PAP-17835), which now sits between picking a - * curated app and entering its credential. Picks "Any agent" so Continue is - * enabled without depending on the agent list. + * Open the Advanced disclosure that now holds the identity and agent-reach + * controls the Access step used to own (PAP-659 C0). Closed by default and + * unmounted while closed, so a test that asserts on those controls has to open + * it first — which is itself the assertion that they are one click away. + */ +async function openAccessAdvanced() { + const change = Array.from(document.body.querySelectorAll("button")).find( + (b) => b.textContent?.trim() === "Change" && b.getAttribute("aria-expanded") === "false", + ); + if (!change) return; + await act(async () => { + change.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); +} + +/** + * PAP-659 deleted the Access step: picking a connector now opens its connect + * screen directly, and identity/agent reach are a stated default with an + * Advanced disclosure rather than a screen to pass. + * + * What remains is the one-click handoff the Access step used to perform for + * automatic-OAuth definitions, which is now the primary action of the sign-in + * screen itself. Everywhere else this is a no-op, so the call sites keep + * reading as "get to the part this test is about". */ async function passAccessStep() { + const onOAuthEntryScreen = Boolean( + document.body.textContent?.includes("Paperclip will open") + || document.body.textContent?.includes("Your connection is saved."), + ); + if (onOAuthEntryScreen) { + const handoff = Array.from(document.body.querySelectorAll("button")).find((b) => { + const label = b.textContent?.trim() ?? ""; + return label.startsWith("Continue to") || label.startsWith("Finish with"); + }); + await act(async () => { + handoff?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + return; + } + // The remote-MCP aggregator setup (arcade, composio, executor, zapier) keeps + // its own Access step until PAP-659's no-auth pass replaces it. Recognise it + // by its own stepper rather than by any control the shared flow also has. + if (!stepLabelsOnScreen().includes("Access")) { + await flushReact(); + return; + } const anyAgent = Array.from(document.body.querySelectorAll('[role="radio"]')) .find((option) => option.textContent?.includes("Any agent")); if (anyAgent) { @@ -185,11 +230,10 @@ async function passAccessStep() { }); await flushReact(); } - const submit = Array.from(document.body.querySelectorAll("button")).find( - (b) => b.textContent?.trim() === "Save and continue" - || b.textContent?.trim() === "Continue" - || b.textContent?.trim().startsWith("Continue to"), - ); + const submit = Array.from(document.body.querySelectorAll("button")).find((b) => { + const label = b.textContent?.trim() ?? ""; + return label === "Save and continue" || label === "Continue"; + }); await act(async () => { submit?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -327,22 +371,37 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { const onComplete = vi.fn(); const popup = vi.spyOn(window, "open").mockReturnValue(null); const connection = { id: "conn-inline", status: "draft", credentialPolicy: "per_user", authKind: "api_key" }; - connectAppMock.mockResolvedValue({ connectionId: connection.id, connection, catalog: [{ id: "tool-1", status: "active" }] }); + connectAppMock.mockResolvedValue({ + connectionId: connection.id, connection, + catalog: [{ id: "tool-1", status: "active" }, { id: "tool-2", status: "active" }], + actions: { + readOnly: [{ catalogEntryId: "tool-1", riskLevel: "read" }], + canMakeChanges: [{ catalogEntryId: "tool-2", riskLevel: "write" }], + }, + suggestedDefaults: { askFirstRiskLevels: ["write", "destructive", "high", "critical"] }, + }); await render(undefined, false, ); - expect(container.textContent).toContain("This task grants access only to Ada"); + // PAP-659 C0: the gateway connectors reach the endpoint field immediately. + // The task-scoped reach is stated rather than confirmed on its own step. + const connectLabel = `Connect ${provider[0].toUpperCase()}${provider.slice(1)}`; + expect(container.textContent).toContain("available to the agent that asked for it"); expect(radioContaining("Any agent")).toBeUndefined(); - await passAccessStep(); expect(container.textContent).toContain("MCP server URL"); expect(container.textContent).not.toContain("Add your key"); + expect(container.textContent).not.toContain("Step 1 of 2"); const url = container.querySelector('input[type="password"]')!; expect(url.value).toBe(provider === "composio" ? "https://connect.composio.dev/mcp" : ""); - expect(buttonByText("Connect")?.disabled).toBe(provider !== "composio"); + expect(buttonByText(connectLabel)?.disabled).toBe(provider !== "composio"); await act(async () => setInputValue(url, "not a url")); - await act(async () => buttonByText("Connect")!.click()); + await act(async () => buttonByText(connectLabel)!.click()); expect(container.textContent).toContain("Enter a valid MCP URL"); expect(connectAppMock).not.toHaveBeenCalled(); + await act(async () => setInputValue(url, "https://provider.example/mcp")); + // The sign-in method shares the one Advanced disclosure with the access + // controls, and Radix unmounts collapsed content — so it has to be opened. + expect(container.querySelector("select")).toBeNull(); + await act(async () => buttonByText("Change")!.click()); await act(async () => { - setInputValue(url, "https://provider.example/mcp"); const auth = container.querySelector("select")!; auth.value = "bearer"; auth.dispatchEvent(new Event("change", { bubbles: true })); @@ -352,7 +411,9 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await act(async () => buttonByText("Try again")!.click()); await vi.waitFor(() => expect(onComplete).toHaveBeenCalledWith({ connectionId: connection.id })); expect(connectAppMock).toHaveBeenCalledWith("company-1", expect.objectContaining({ galleryKey: provider, link: "https://provider.example/mcp", grantKind: "user", authMode: "bearer", credentialValues: { "credentials.authorization": "fixture-token" } })); - expect(finishAppMock).toHaveBeenCalledWith("company-1", connection.id, { enabledCatalogEntryIds: ["tool-1"], askFirstCatalogEntryIds: [], access: { agentIds: ["agent-1"] }, preserveExistingAccess: true }); + // PAP-659 C6a/C7: this path used to send an empty ask-first list, so the + // gateway connectors were the one place the armed write gate did not apply. + expect(finishAppMock).toHaveBeenCalledWith("company-1", connection.id, { enabledCatalogEntryIds: ["tool-1", "tool-2"], askFirstCatalogEntryIds: ["tool-2"], access: { agentIds: ["agent-1"] }, preserveExistingAccess: true }); expect(putConnectionInstallsMock).not.toHaveBeenCalled(); expect(mockNavigate).not.toHaveBeenCalled(); expect(popup).not.toHaveBeenCalled(); @@ -366,9 +427,9 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { const connection = { id: "conn-inline-oauth", status: "draft", credentialPolicy: "per_user", authKind: "oauth" }; connectAppMock.mockResolvedValue({ connectionId: connection.id, connection, catalog: [], auth: { kind: "oauth" } }); const root = await render(undefined, false, ); - await passAccessStep(); + const connectLabel = `Connect ${provider[0].toUpperCase()}${provider.slice(1)}`; await act(async () => setInputValue(container.querySelector('input[type="password"]')!, "https://provider.example/mcp")); - await act(async () => buttonByText("Connect")!.click()); + await act(async () => buttonByText(connectLabel)!.click()); await vi.waitFor(() => expect(startOAuthMock).toHaveBeenCalledWith(connection.id, { asCurrentUser: true, interactionId: "intent-inline" })); expect(popup.location.assign).toHaveBeenCalled(); popup.closed = true; @@ -399,7 +460,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { getConnectionMock.mockResolvedValue(connection); const content = ; const root = await render(undefined, false, content); - await passAccessStep(); await act(async () => setInputValue(container.querySelector('input[type="password"]')!, "https://provider.example/mcp?token=fixture-secret")); await act(async () => buttonByText("Save & exit")!.click()); await vi.waitFor(() => expect(onCancel).toHaveBeenCalled()); @@ -413,7 +473,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(undefined, false, content); await vi.waitFor(() => expect(container.textContent).toContain("MCP server URL")); expect(getConnectionMock).toHaveBeenCalledWith(connection.id); - expect(container.textContent).toContain("Step 2 of 2"); + // Resuming lands on the same single screen it was saved from, not "step 2". + expect(container.textContent).not.toContain("Step 2 of 2"); }); it.each(["page", "dialog"] as const)("lets %s setup cancel after an invalid provider URL without trying to save it", async (host) => { @@ -422,7 +483,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(undefined, false, ); await passAccessStep(); await act(async () => setInputValue(container.querySelector('input[type="password"]')!, "https://wrong-provider.example/mcp")); - await act(async () => buttonByText("Connect")!.click()); + await act(async () => buttonByText("Connect Zapier")!.click()); await vi.waitFor(() => expect(container.textContent).toContain("That connection URL does not belong to Zapier")); await act(async () => buttonByText("Cancel")!.click()); @@ -493,19 +554,22 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); it.each([ - ["shared", "Any human in the organization", false], - ["per_user", "Just me", true], - ])("retains the %s identity when returning to Access after an OAuth return", async (credentialPolicy, identityLabel, asCurrentUser) => { + ["shared", "Connects for everyone in your organization", "Any human in the organization", false], + ["per_user", "Connects as you", "Just me", true], + ])("retains the %s identity across an OAuth return without a step to walk back through", async (credentialPolicy, statedDefault, identityLabel, asCurrentUser) => { const draft = { id: "conn-oauth-draft", status: "draft", authKind: "oauth", credentialPolicy, config: { sourceTemplateKey: "composio", connectionMethodKey: "mcp", url: "https://connect.composio.dev/mcp" } }; mockSearch.value = `source=composio&resume=${draft.id}&oauth=denied`; getConnectionMock.mockResolvedValue(draft); connectAppMock.mockResolvedValue({ connectionId: draft.id, connection: draft, catalog: [], auth: { kind: "oauth" } }); await render(); - await vi.waitFor(() => expect(buttonByText("Back")).toBeTruthy()); - await act(async () => buttonByText("Back")!.click()); + // PAP-659 C0: the retained identity is stated on the one screen instead of + // being re-confirmed on an Access step the operator has to navigate back to. + await vi.waitFor(() => expect(container.textContent).toContain(statedDefault)); + // It is also still editable, one disclosure away. Radix unmounts collapsed + // content, so the controls have to be opened to be asserted at all. + await act(async () => buttonByText("Change")!.click()); expect(container.textContent).toContain(identityLabel); expect(container.querySelector('[role="radiogroup"][aria-label="Which humans can use this credential?"]')).toBeNull(); - await act(async () => buttonByText("Continue")!.click()); await act(async () => buttonByText("Try again")!.click()); await vi.waitFor(() => expect(startOAuthMock).toHaveBeenCalledWith(draft.id, { asCurrentUser })); expect(connectAppMock).toHaveBeenCalledWith("company-1", expect.objectContaining({ resumeConnectionId: draft.id, grantKind: asCurrentUser ? "user" : "organization" })); @@ -568,18 +632,20 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.querySelector('input[aria-label="MCP server URL"]')?.value).toBe("https://edited.example.test/mcp"); }); - it("an unrecognized URL routes to a minimal frame with the URL and key choice", async () => { + it("an unrecognized URL routes to a minimal frame that asks nothing but the address", async () => { await render(); await gotoLinkFrame(container, "https://www.example.com/actions"); expect(container.textContent).toContain("Connect your own MCP server"); expect(container.textContent).toContain("https://www.example.com/actions"); - expect(container.textContent).toContain("Does it need a key?"); - expect(Array.from(container.querySelectorAll("label")).find( - (label) => label.textContent === "Does it need a key?", - )?.classList.contains("mr-2")).toBe(true); - expect(buttonByText("No")).toBeTruthy(); - expect(buttonByText("Yes")).toBeTruthy(); + // PAP-659 bucket H: the server's probe answers "does it need a key?", so the + // operator is no longer asked to guess at it before anything has been tried. + expect(container.textContent).not.toContain("Does it need a key?"); + expect(buttonByText("No")).toBeUndefined(); + expect(buttonByText("Yes")).toBeUndefined(); + expect( + Array.from(container.querySelectorAll("input")).some((i) => i.type === "password"), + ).toBe(false); expect(container.querySelector('input[placeholder="My app"]')).toBeNull(); }); @@ -596,6 +662,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonByText("Continue")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); expect(container.textContent).toContain("Which agents can use this connection?"); @@ -603,7 +670,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(radioContaining("Any human in the organization")?.getAttribute("aria-checked")).toBe("true"); expect(radioContaining("Just agents I pick")).toBeTruthy(); expect(radioContaining("Any agent")?.getAttribute("aria-checked")).toBe("true"); - expect(container.textContent).not.toContain("Does it need a key?"); }); it.each([false, true])("gates chat-only cards in the embedded tool gallery without hiding GitHub (%s)", async (enabled) => { @@ -645,6 +711,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(buttonByText("Use GitHub")).toBeTruthy(); await act(async () => buttonByText("Use GitHub")!.click()); await flushReact(); + await openAccessAdvanced(); expect(container.textContent).toContain("Connect GitHub as"); expect(container.textContent).toContain("My GitHub account"); expect(container.textContent).not.toContain("Chat with an agent"); @@ -674,7 +741,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { client.setQueryData(queryKeys.health, { deploymentMode: "authenticated", localAiLoginSupported: false }); await render(client, false, taskRepair ? : undefined); await passAccessStep(); - expect(container.textContent).toContain("Connect account"); expect(container.textContent).toContain("Connection name"); expect(container.textContent).not.toContain("How do you want to connect?"); expect(radioContaining("Use an API key")).toBeUndefined(); @@ -695,12 +761,19 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).not.toContain("Connect for tool access instead"); }); - it("asks for a GitHub identity and defaults to the current user and every agent", async () => { + it("states the GitHub identity default and keeps its controls one click away", async () => { mockParams.appKey = "github"; listGalleryMock.mockResolvedValue({ apps: [GITHUB_MANAGED] }); await render(); - expect(container.textContent).toContain("Access"); + // PAP-659 C0: the default is printed, not asked. Nothing about identity or + // reach blocks the primary action. + expect(container.textContent).toContain("Connects as you, available to all agents."); + expect(container.textContent).not.toContain("Connect GitHub as"); + expect(container.querySelector('[role="radio"]')).toBeNull(); + + await openAccessAdvanced(); + expect(container.textContent).toContain("Connect GitHub as"); expect(container.textContent).toContain("Which agents may use your GitHub when you’re responsible?"); expect(container.textContent).not.toContain("Choose access before adding credentials"); @@ -732,11 +805,12 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(anyAgent?.getAttribute("aria-checked")).toBe("true"); }); - it("keeps provider guidance and card styling out of the shared access step", async () => { + it("keeps provider guidance and card styling out of the access controls", async () => { mockParams.appKey = "pagerduty"; listGalleryMock.mockResolvedValue({ apps: [PAGERDUTY] }); await render(); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); expect(container.textContent).toContain("Which agents can use this connection?"); @@ -753,11 +827,14 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(accessCard?.classList.contains("shadow-sm")).toBe(false); }); - it("defaults to every agent and only blocks an empty explicit selection", async () => { + it("defaults to every agent and states an empty explicit selection", async () => { mockParams.appKey = "github"; await render(); + await openAccessAdvanced(); - expect(buttonByText("Save and continue")?.disabled).toBe(false); + // PAP-659 C0: narrowing reach never blocks Connect. An empty selection is + // reported in the stated default instead of disabling the primary action. + expect(container.textContent).toContain("available to all agents."); await act(async () => { Array.from(document.body.querySelectorAll('[role="radio"]')) @@ -766,43 +843,28 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(buttonByText("Save and continue")?.disabled).toBe(true); + expect(container.textContent).toContain("no agents selected yet."); }); - it("keeps the access selections when the wizard moves backward", async () => { + it("reflects a changed access selection in the stated default", async () => { mockParams.appKey = "github"; await render(); + await openAccessAdvanced(); await act(async () => { Array.from(document.body.querySelectorAll('[role="radio"]')) - .find((r) => r.textContent?.includes("My GitHub account")) - ?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - await act(async () => { - Array.from(document.body.querySelectorAll('[role="radio"]')) - .find((r) => r.textContent?.includes("Any agent")) + .find((r) => r.textContent?.includes("Only agents I choose")) ?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await act(async () => { - buttonByText("Save and continue")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - expect(container.textContent).toContain("Connect GitHub"); + // The sentence above the primary action is the only place this is said, so + // it has to track the controls rather than print a fixed default. + expect(container.textContent).toContain("no agents selected yet"); - await act(async () => { - buttonByText("Back")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - - // Moving backward must not silently reset the identity the operator chose. const radios = Array.from(document.body.querySelectorAll('[role="radio"]')); - expect(radios.find((r) => r.textContent?.includes("My GitHub account"))?.getAttribute("aria-checked")) - .toBe("true"); - expect(radios.find((r) => r.textContent?.includes("Any agent"))?.getAttribute("aria-checked")) - .toBe("true"); + expect(radios.find((r) => r.textContent?.includes("Only agents I choose")) + ?.getAttribute("aria-checked")).toBe("true"); }); it("returns to the pending skill import when GitHub setup is cancelled", async () => { @@ -850,6 +912,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); + await openAccessAdvanced(); + const github = identityChoices(); expect(github.wholeOrg?.getAttribute("aria-checked")).toBe("true"); expect(github.justMe?.getAttribute("aria-checked")).toBe("false"); @@ -870,10 +934,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await act(async () => { - buttonByText("Save and continue")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); const keyField = container.querySelector("input[type=password]"); await act(async () => setInputValue(keyField!, "posthog-personal-token")); @@ -899,6 +959,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { mockParams.appKey = "posthog"; mockSearch.value = ""; root = await render(); + await openAccessAdvanced(); const posthog = identityChoices(); expect(posthog.justMe?.getAttribute("aria-checked")).toBe("false"); @@ -921,14 +982,17 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); it("collects customer-owned OAuth client details for a curated manual OAuth app", async () => { - listGalleryMock.mockResolvedValue({ apps: [ASANA] }); - mockParams.appKey = "asana"; + listGalleryMock.mockResolvedValue({ apps: [BOX] }); + mockParams.appKey = "box"; await render(); await passAccessStep(); - expect(container.textContent).toContain("Your OAuth app"); - expect(container.textContent).toContain("Open Asana app settings"); - expect(container.textContent).not.toContain("Create an Asana MCP OAuth app"); + expect(container.textContent).toContain("needs its own OAuth app"); + expect(container.textContent).toContain("Open Box app settings"); + // The extra work reads as the provider's limitation, not as Box's normal path. + expect(container.textContent).toContain( + "does not let Paperclip register itself automatically", + ); expect(container.textContent).toContain("Paperclip callback URL"); expect(container.textContent).toContain( "http://localhost:3000/api/tools/oauth/callback", @@ -938,8 +1002,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { const clientId = container.querySelector("#curated-oauth-client-id")!; const clientSecret = container.querySelector("#curated-oauth-client-secret")!; await act(async () => { - setInputValue(clientId, "asana-client-id"); - setInputValue(clientSecret, "asana-client-secret"); + setInputValue(clientId, "box-client-id"); + setInputValue(clientSecret, "box-client-secret"); }); await flushReact(); expect(buttonByText("Continue to sign in")?.disabled).toBe(false); @@ -950,15 +1014,72 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); expect(connectAppMock).toHaveBeenCalledWith("company-1", expect.objectContaining({ - galleryKey: "asana", + galleryKey: "box", connectionMethodKey: "mcp-own-oauth", oauthClient: { - clientId: "asana-client-id", - clientSecret: "asana-client-secret", + clientId: "box-client-id", + clientSecret: "box-client-secret", }, })); }); + it("gives Asana the one-click path now that its server advertises registration", async () => { + listGalleryMock.mockResolvedValue({ apps: [ASANA] }); + mockParams.appKey = "asana"; + await render(); + await flushReact(); + + // No console detour: the connect screen is the handoff, not a form. + expect(container.textContent).not.toContain("needs its own OAuth app"); + expect(container.querySelector("#curated-oauth-client-id")).toBeNull(); + const primary = Array.from(container.querySelectorAll("button")).find((b) => + b.textContent?.trim().startsWith("Continue to"), + ); + expect(primary).toBeTruthy(); + expect(primary?.disabled).toBe(false); + }); + + it("offers Asana's own-OAuth-app fields as the recovery when registration is refused", async () => { + // Asana advertises registration but refuses hosted callbacks, so a failed + // sign-in must still leave the customer-owned client path within reach. + listGalleryMock.mockResolvedValue({ apps: [ASANA] }); + mockParams.appKey = "asana"; + connectAppMock.mockRejectedValueOnce(new Error("Asana refused the redirect URI.")); + connectAppMock.mockResolvedValueOnce({ + connectionId: "conn-asana", + application: { id: "app-asana", name: "Asana" }, + connection: { id: "conn-asana" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + auth: { kind: "oauth", startUrl: "https://app.asana.com/-/oauth_authorize?state=opaque" }, + }); + await render(); + await flushReact(); + expect(container.querySelector("#curated-oauth-client-id")).toBeNull(); + + await act(async () => { + buttonByText("Continue to Asana")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + const clientId = container.querySelector("#curated-oauth-client-id"); + expect(clientId).toBeTruthy(); + await act(async () => setInputValue(clientId!, "asana-own-client")); + await flushReact(); + await act(async () => { + buttonByText("Try again")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(connectAppMock).toHaveBeenLastCalledWith("company-1", expect.objectContaining({ + galleryKey: "asana", + oauthClient: expect.objectContaining({ clientId: "asana-own-client" }), + })); + }); + it("submits the Postman access mode selected on the setup screen", async () => { listGalleryMock.mockResolvedValue({ apps: [POSTMAN] }); mockParams.appKey = "postman"; @@ -973,7 +1094,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); - await passAccessStep(); + await openAccessAdvanced(); const full = radioContaining("Full"); const code = radioContaining("Code"); @@ -1012,6 +1133,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { mockSearch.value = "source=asana"; await render(); + await openAccessAdvanced(); expect(document.body.textContent).toContain("Which humans can use this credential?"); expect(document.body.textContent).toContain("Which agents can use this connection?"); @@ -1025,9 +1147,10 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - expect(document.body.textContent).toContain("Step 1 of 2"); - expect(document.body.textContent).toContain("Access · Choose connection"); + // PAP-659: a selected app has one screen, so no stepper at all. + expect(document.body.textContent).not.toContain("Step 1 of 2"); expect(document.body.textContent).not.toContain("Pick app ·"); + expect(document.body.textContent).toContain("Connect Gmail"); }); it("opens a brokered Gmail deep link at the access step", async () => { @@ -1037,9 +1160,12 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - expect(document.body.textContent).toContain("Step 1 of 2"); - expect(document.body.textContent).toContain("Access · Choose connection"); + // PAP-659: a selected app is one screen, so there is no stepper and no + // Access stage to deep-link into; a legacy stage=access URL lands here. + expect(document.body.textContent).not.toContain("Step 1 of 2"); expect(document.body.textContent).not.toContain("Pick app ·"); + expect(document.body.textContent).toContain("Connects for everyone in your organization"); + await openAccessAdvanced(); expect(document.body.textContent).toContain("Which humans can use this credential?"); expect(document.body.textContent).toContain("Just me"); expect(mockNavigate).not.toHaveBeenCalledWith("/apps/connect", { replace: true }); @@ -1072,7 +1198,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ); expect(buttonByText("Connect with Paperclip")?.closest(".rounded-xl")?.classList.contains("border-border")).toBe(true); expect(container.textContent).not.toContain("Required once for managed Google sign-in."); - expect(container.textContent).not.toContain("Your OAuth app"); + expect(container.textContent).not.toContain("needs its own OAuth app"); await act(async () => { buttonByText("Connect with Paperclip")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); @@ -1119,9 +1245,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); await flushReact(); if (!popupBlocked) expect(popup.close).toHaveBeenCalled(); - expect(container.textContent).not.toContain("What should Paperclip be able to do?"); - expect(buttonByText("Advanced")).toBeDefined(); - expect(container.textContent).toContain("Step 2 of 2"); + await openAccessAdvanced(); + expect(container.textContent).toContain("What should Paperclip be able to do?"); connectAppMock.mockResolvedValue({ connectionId: "gmail-1", connection: { id: "gmail-1", credentialPolicy: "per_user" }, auth: { kind: "oauth", startUrl: "https://example.test/unbound" } }); await act(async () => buttonByText("Continue to sign in")?.click()); await flushReact(); @@ -1191,7 +1316,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); - expect(container.textContent).toContain("Step 2 of 2"); await act(async () => { buttonByText("Continue")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -1209,23 +1333,21 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }, ); - it("labels GitHub's local setup transition without promising a provider handoff", async () => { + it("labels GitHub's setup transition as the provider handoff it now is", async () => { mockSearch.value = "source=github"; listGalleryMock.mockResolvedValue({ apps: [GITHUB_MANAGED] }); await render(); - const continueButton = buttonByText("Continue"); - expect(continueButton).toBeDefined(); - expect(continueButton?.disabled).toBe(false); - expect(continueButton?.querySelector(".lucide-arrow-up-right")).toBeNull(); - expect(buttonByText("Continue to GitHub")).toBeUndefined(); + // PAP-659: the screen that used to precede the handoff is gone, so the one + // primary action names the provider instead of a generic "Continue". + expect(buttonByText("Continue")).toBeUndefined(); + const handoff = buttonByText("Continue to GitHub"); + expect(handoff).toBeDefined(); + expect(handoff?.disabled).toBe(false); - await passAccessStep(); - - expect(container.textContent).toContain("Step 2 of 2"); + await openAccessAdvanced(); expect(container.textContent).toContain("How do you want to connect?"); - expect(buttonByText("Continue to GitHub")).toBeDefined(); expect(startOAuthMock).not.toHaveBeenCalled(); expect(connectAppMock).not.toHaveBeenCalled(); }); @@ -1261,11 +1383,13 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - expect(container.textContent).toContain("Access · Sign in"); + expect(container.textContent).toContain("Connects as you"); + await openAccessAdvanced(); expect(radioContaining("My GitHub account")?.getAttribute("aria-checked")).toBe("true"); expect(radioContaining("Any agent")?.getAttribute("aria-checked")).toBe("true"); expect(container.textContent).toContain("Which agents may use your GitHub when you’re responsible?"); - expect(buttonByText("Continue")).toBeDefined(); + // Enrolment is the screen; its own action is the only primary one. + expect(buttonByText("Connect with Paperclip")).toBeDefined(); expect(buttonByText("Continue to GitHub")).toBeUndefined(); await passAccessStep(); @@ -1280,7 +1404,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - expect(container.textContent).toContain("Step 2 of 2"); expect(container.textContent).toContain("Continue to GitHub"); expect(container.textContent).not.toContain("Connect GitHub as"); expect(container.textContent).not.toContain("Connect with Paperclip"); @@ -1373,6 +1496,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); + await openAccessAdvanced(); await act(async () => { radioContaining("A dedicated account for an agent")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -1386,12 +1510,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - const accessContinue = buttonByText("Continue"); - expect(accessContinue?.disabled).toBe(false); - await act(async () => { - accessContinue?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); await act(async () => { buttonByText("Connect with Paperclip")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -1440,7 +1558,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); await flushReact(); - expect(container.textContent).toContain("Step 2 of 2"); + expect(container.textContent).toContain("Connects as a dedicated agent account"); expect(window.sessionStorage.getItem( "paperclip.connector-enrollment-access:github", )).toBeNull(); @@ -1474,17 +1592,15 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ...definition, ownershipAvailability: { ...definition.ownershipAvailability, platform_shared: true }, }] }); await render(); + await openAccessAdvanced(); if (!enrollmentReturn) { await act(async () => { radioContaining("Just me")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await passAccessStep(); + await flushReact(); } if (definition.methods.some((method) => method.capabilityProfile?.key !== "read")) { - expect(radioContaining(readMethod.capabilityProfile!.label)).toBeUndefined(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); const readChoice = radioContaining(readMethod.capabilityProfile!.label); expect(readChoice).not.toBeNull(); await act(async () => { @@ -1493,9 +1609,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { } await flushReact(); // Change auth methods too, including apps with only one capability. - expect(container.textContent).not.toContain("How do you want to connect?"); expect(container.textContent).not.toContain("Connect with Paperclip"); - expect(container.textContent).not.toContain("Your OAuth app"); + expect(container.textContent).not.toContain("needs its own OAuth app"); expect(buttonByText("Continue to sign in")?.disabled).toBe(false); const customerAuth = buttonByText("Use your own Google OAuth app"); expect(customerAuth).toBeDefined(); @@ -1504,7 +1619,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { customerAuth!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - expect(container.textContent).toContain("Your OAuth app"); + expect(container.textContent).toContain("needs its own OAuth app"); expect(container.textContent).toContain("Client ID"); expect(buttonByText("Continue to sign in")?.disabled).toBe(true); const managedAuth = buttonByText("Use Paperclip instead"); @@ -1517,7 +1632,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { managedAuth!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - expect(container.textContent).not.toContain("Your OAuth app"); + expect(container.textContent).not.toContain("needs its own OAuth app"); await act(async () => { buttonByText("Continue to sign in")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -1553,19 +1668,17 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).not.toContain("does not enable unrelated Paperclip customers"); expect(container.textContent).not.toContain("final project-registration email"); expect(container.textContent).not.toContain("Apply or verify Developer Preview enrollment"); + await openAccessAdvanced(); expect(radioContaining("Just me")).toBeTruthy(); expect(radioContaining("Any human in the organization")?.getAttribute("aria-checked")).toBe("true"); await passAccessStep(); expect(container.textContent).toContain("Read & create"); - expect(radioContaining("Read only")).toBeUndefined(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); expect(radioContaining("Read & create")?.getAttribute("aria-checked")).toBe("true"); expect(radioContaining("Read only")?.getAttribute("aria-checked")).toBe("false"); expect(container.textContent).not.toContain("Before connecting, enroll the signed-in Workspace account"); expect(container.textContent).toContain("Review requirements"); - expect(container.textContent).toContain("Your OAuth app"); + expect(container.textContent).toContain("needs its own OAuth app"); }); it("renders Google Calendar as one minimal, unboxed setup screen", async () => { @@ -1579,8 +1692,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { (heading) => heading.textContent?.trim() === "Connect Google Calendar", ); expect(duplicateHeadings).toHaveLength(1); - expect(container.textContent).not.toContain("What should Paperclip be able to do?"); - expect(buttonByText("Advanced")).toBeDefined(); + await openAccessAdvanced(); + expect(container.textContent).toContain("What should Paperclip be able to do?"); expect(container.textContent).toContain("Review requirements"); expect(container.textContent).not.toContain("Connect Google Calendar to read and manage events."); expect(container.textContent).not.toContain("All event mutations require approval."); @@ -1590,8 +1703,6 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).not.toContain("You’ll sign in before anything turns on."); expect(container.querySelector('input[placeholder="My app"]')).toBeNull(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); const capabilityQuestion = Array.from(container.querySelectorAll("label")).find( (label) => label.textContent === "What should Paperclip be able to do?", ); @@ -1634,6 +1745,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }, }); await render(); + await openAccessAdvanced(); const anyAgent = Array.from( document.body.querySelectorAll('[role="radio"]'), @@ -1663,18 +1775,23 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { document.body.querySelectorAll('[role="radio"]'), ).find((r) => r.textContent?.includes("Just agents I pick")); expect(pick?.disabled).toBe(false); - // Continue refuses the forbidden choice even though it is the current one. - expect(buttonByText("Continue")?.disabled).toBe(true); + // PAP-659: a reach the member cannot grant is reported above the primary + // action rather than disabling it, and "Just agents I pick" is the live + // alternative, so the screen is not a dead end. + expect(container.textContent).toContain( + "Your company policy limits this choice to connection managers.", + ); }); it("opens the selected app directly on its setup route", async () => { mockParams.appKey = "github"; await render(); - // A deep-linked app lands on Access first: identity and reach are chosen - // before the credential (PAP-17835). + // PAP-659: a deep-linked app lands on its one connect screen. Identity and + // reach are stated there and changed in the same Advanced disclosure. + expect(container.textContent).toContain("Connects for everyone in your organization"); + await openAccessAdvanced(); expect(container.textContent).toContain("Connect GitHub as"); - await passAccessStep(); expect(container.textContent).toContain("Connect GitHub"); expect(container.textContent).not.toContain("Pick the app you want your agents to use."); @@ -1684,15 +1801,16 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { mockParams.appKey = "posthog"; listGalleryMock.mockResolvedValueOnce({ apps: [POSTHOG] }); await render(); - // Access comes first for a curated app; the method chooser shares a screen - // with the credential fields, so it sits behind it (PAP-17835). - expect(container.textContent).toContain("Which humans can use this credential?"); - await passAccessStep(); + // PAP-659: identity is stated, not asked, and the method chooser moved + // into the same Advanced disclosure. + expect(container.textContent).toContain("Connects for everyone in your organization"); + await openAccessAdvanced(); expect(container.textContent).toContain("How do you want to connect?"); - expect(radioContaining("Sign in with PostHog")?.getAttribute("aria-checked")).toBe("false"); + // PAP-659 C1: OAuth is strictly less work than a key, so it is preselected. + expect(radioContaining("Sign in with PostHog")?.getAttribute("aria-checked")).toBe("true"); expect(radioContaining("Use a personal API key")?.getAttribute("aria-checked")).toBe("false"); - expect(buttonByText("Connect")?.disabled).toBe(true); + expect(buttonByText("Continue to sign in")?.disabled).toBe(false); await act(async () => { radioContaining("Use a personal API key")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); @@ -1705,20 +1823,10 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { const keyInput = container.querySelector('input[type="password"]'); const advanced = buttonByText("Advanced"); expect(keyInput).toBeTruthy(); - expect(container.querySelector('input[placeholder="Optional numeric project ID"]')).toBeNull(); - expect(container.querySelector('[role="switch"]')).toBeNull(); - expect(advanced?.getAttribute("aria-expanded")).toBe("false"); - expect(container.textContent).not.toContain("Pin to project ID"); - expect(container.textContent).not.toContain("Feature groups"); - expect(container.textContent).not.toContain("Individual tools"); - expect(container.textContent).not.toContain("Tool response mode"); - - await act(async () => { - advanced?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - - expect(advanced?.getAttribute("aria-expanded")).toBe("true"); + // PAP-659: the optional controls share the one Advanced disclosure with + // the access defaults, so they are visible because it is already open — + // and they are still optional, so none of them blocks Connect. + expect(advanced).toBeTruthy(); expect(container.textContent).toContain("Pin to project ID"); expect(container.querySelector('input[placeholder="Optional numeric project ID"]')).toBeTruthy(); expect(container.querySelector('[role="switch"]')?.getAttribute("aria-checked")).toBe("false"); @@ -1769,7 +1877,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }, }); await render(undefined, false, ); - await passAccessStep(); + await openAccessAdvanced(); await act(async () => { radioContaining("Use a personal API key")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -1875,9 +1983,9 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { requestedAgentId="agent-1" /> )); - expect(container.textContent).toContain("This task grants access only to Ada"); + expect(container.textContent).toContain("available to the agent that asked for it"); const continueButton = Array.from(container.querySelectorAll("button")).find( - (button) => button.textContent?.trim() === "Continue", + (button) => button.textContent?.trim().startsWith("Continue"), ); expect(continueButton).toBeTruthy(); expect(continueButton?.disabled).toBe(false); @@ -2032,6 +2140,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(connectAppMock).not.toHaveBeenCalled(); expect(navigateTopLevelMock).not.toHaveBeenCalled(); + expect(container.textContent).toContain("Connects for everyone in your organization"); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); expect(container.textContent).not.toContain("Choose access before sign-in"); const identityRadios = Array.from(document.body.querySelectorAll('[role="radio"]')); @@ -2078,19 +2188,20 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); - const stepsOnAccess = stepLabelsOnScreen(); - expect(stepsOnAccess).toEqual(["Access", "Sign in"]); + // PAP-659: a curated connector is a single screen, so it shows no stepper + // at all. Connecting must not conjure one. + expect(stepLabelsOnScreen()).toEqual([]); + expect(stepDotCount()).toBe(0); await passAccessStep(); await submitCuratedOAuthSetup(); expect(container.textContent).toContain("Preparing secure sign-in"); - // Connecting must not grow the stepper a step the flow never lands on. - expect(stepLabelsOnScreen()).toEqual(stepsOnAccess); - expect(stepDotCount()).toBe(stepsOnAccess.length); + expect(stepLabelsOnScreen()).toEqual([]); + expect(stepDotCount()).toBe(0); }); - it("backs from the sign-in checkpoint to Access without exiting the wizard", async () => { + it("backs from the sign-in checkpoint out to the connector gallery", async () => { mockSearch.value = "source=notion"; listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); connectAppMock.mockReturnValueOnce(new Promise(() => {})); @@ -2104,9 +2215,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(container.textContent).toContain("Which humans can use this credential?"); - expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?source=notion&stage=access"); - expect(mockNavigate).not.toHaveBeenCalledWith("/apps"); + expect(mockNavigate).toHaveBeenCalledWith("/apps"); }); it("resumes an existing Notion OAuth connection instead of creating another draft", async () => { @@ -2140,7 +2249,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); expect(container.textContent).not.toContain("Reconnect keeps this identity type"); - expect(container.textContent).toContain("Existing agent access stays the same"); + expect(container.textContent).toContain("agent access stays as it is"); expect(startOAuthMock).not.toHaveBeenCalled(); await passAccessStep(); await submitCuratedOAuthSetup(); @@ -2306,9 +2415,9 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); expect(container.textContent).toContain( - credentialPolicy === "per_user" ? "Just me" : "Any human in the organization", + credentialPolicy === "per_user" ? "Connects as you" : "Connects for everyone in your organization", ); - expect(container.textContent).toContain("Existing agent access stays the same"); + expect(container.textContent).toContain("agent access stays as it is"); await passAccessStep(); await submitCuratedOAuthSetup(); @@ -2359,7 +2468,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); - expect(container.textContent).toContain("Any human in the organization"); + expect(container.textContent).toContain("Connects for everyone in your organization"); await passAccessStep(); await submitCuratedOAuthSetup(); @@ -2566,7 +2675,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(connectAppMock).not.toHaveBeenCalled(); expect(startOAuthMock).not.toHaveBeenCalled(); expect(container.textContent).not.toContain("Reconnect keeps this identity type"); - expect(container.textContent).toContain("Existing agent access stays the same"); + expect(container.textContent).toContain("agent access stays as it is"); await passAccessStep(); await submitCuratedOAuthSetup(); expect(startOAuthMock).toHaveBeenCalledWith("conn-refreshed", { @@ -2609,12 +2718,11 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); await flushReact(); + // PAP-659: Try again re-reads and then resumes the draft it finds. There + // is no Access step to bounce back through, and the operator already + // pressed the one primary action, so recovery must not ask again. expect(connectAppMock).not.toHaveBeenCalled(); - expect(startOAuthMock).not.toHaveBeenCalled(); expect(container.textContent).not.toContain("Reconnect keeps this identity type"); - expect(container.textContent).toContain("Existing agent access stays the same"); - await passAccessStep(); - await submitCuratedOAuthSetup(); expect(startOAuthMock).toHaveBeenCalledWith("conn-after-retry", { asCurrentUser: true, }); @@ -2745,6 +2853,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); expect(connectAppMock).not.toHaveBeenCalled(); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); expect(mockNavigate).not.toHaveBeenCalledWith("/apps/connect", { replace: true }); }); @@ -2782,11 +2891,19 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(input.credentialValues).toBeUndefined(); }); - it("choosing Yes reveals one masked key field without extra reassurance copy", async () => { + it("reveals one masked key field only after the probe says a credential is needed", async () => { + // PAP-659 bucket H. The first press asks the server; the server is what + // knows. Only its credential challenge puts a key field on screen, and the + // second press carries the key. + connectAppMock.mockRejectedValueOnce( + new ApiError("This app needs you to sign in.", 502, { + error: "This app needs you to sign in.", + details: { code: "oauth_challenge" }, + }), + ); await render(); await gotoLinkFrame(container, "https://www.example.com/actions"); - // No key field while No is selected. expect( Array.from(container.querySelectorAll("input")).some( (i) => i.type === "password", @@ -2794,10 +2911,12 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ).toBe(false); await act(async () => { - buttonByText("Yes")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); + await flushReact(); + expect(container.textContent).toContain("This server wants a credential"); const passwordInputs = Array.from( container.querySelectorAll("input"), ).filter((i) => i.type === "password"); @@ -2812,8 +2931,8 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(connectAppMock).toHaveBeenCalledTimes(1); - const [, input] = connectAppMock.mock.calls[0]; + expect(connectAppMock).toHaveBeenCalledTimes(2); + const [, input] = connectAppMock.mock.calls[1]; expect(input.credentialValues).toEqual({ "credentials.authorization": "secret-key" }); }); @@ -2835,6 +2954,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonByText("Continue")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); expect(container.textContent).toContain("Which agents can use this connection?"); @@ -2896,22 +3016,27 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); - expect(container.textContent).toContain("Step 1 of 2"); + // PAP-659 C0: no step counter and no Access step — the endpoint field is + // the first and only thing on the way in, with the default stated above it. + expect(container.textContent).not.toContain("Step 1 of 2"); + expect(container.textContent).not.toContain("Step 2 of 2"); expect(container.textContent).toContain("Connect Zapier"); - expect(container.textContent).toContain("Which humans can use this credential?"); - expect(container.textContent).toContain("Which agents can use this connection?"); + expect(container.textContent).toContain("Connects for everyone in your organization, available to all agents."); expect(container.querySelector('[data-remote-mcp-provider="zapier"]')).toBeTruthy(); expect(container.textContent).not.toContain("Pick the app you want your agents to use."); - expect(container.querySelector('input[placeholder="Paste the full URL from Zapier"]')).toBeNull(); + expect(container.textContent).toContain("MCP server URL"); + // The two questions the deleted step asked are still answerable, together, + // one disclosure away. + expect(container.textContent).not.toContain("Which humans can use this credential?"); + await act(async () => buttonByText("Change")!.click()); + expect(container.textContent).toContain("Which humans can use this credential?"); + expect(container.textContent).toContain("Which agents can use this connection?"); await act(async () => { radioContaining("Just me")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await passAccessStep(); - - expect(container.textContent).toContain("Step 2 of 2"); - expect(container.textContent).toContain("MCP server URL"); + expect(container.textContent).toContain("Connects as you, available to all agents."); const linkInput = container.querySelector( 'input[placeholder="Paste the full URL from Zapier"]', @@ -2923,7 +3048,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); await act(async () => { - buttonByText("Connect")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + buttonByText("Connect Zapier")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); @@ -2978,6 +3103,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { mockSearch.value = "link=https%3A%2F%2Fwww.example.com%2Factions&name=Bla&applicationId=app-77"; await render(); + await openAccessAdvanced(); expect(container.textContent).toContain("Which humans can use this credential?"); await passAccessStep(); @@ -3012,9 +3138,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonContaining("Google Sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await passAccessStep(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); + await openAccessAdvanced(); await act(async () => { buttonContaining("Share selected sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -3040,9 +3164,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonContaining("Google Sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await passAccessStep(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); + await openAccessAdvanced(); await act(async () => { buttonContaining("Share selected sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -3070,7 +3192,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).toContain("Connect GitHub"); expect(container.querySelector('input[placeholder="My app"]')).toBeNull(); - expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?source=github&stage=setup"); + expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?source=github"); }); it("keeps the originating connection intent in wizard URLs", async () => { @@ -3087,26 +3209,18 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ); await passAccessStep(); - expect(mockNavigate).toHaveBeenCalledWith( - `/apps/connect?source=github&stage=setup&intent=${interactionId}`, - ); + expect(mockNavigate).not.toHaveBeenCalledWith("/apps"); }); - it("steps back from the key step to Access, and from Access to Connectors", async () => { + it("steps back from the single connect screen straight to Connectors", async () => { mockSearch.value = ""; mockParams.appKey = "github"; await render(); await passAccessStep(); expect(container.textContent).toContain("Connect GitHub"); - // Back from the credential goes to Access, not all the way out: the - // selections made there have to survive (PAP-17835). - await act(async () => { - buttonByText("Back")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - expect(container.textContent).toContain("Connect GitHub as"); - + // PAP-659: there is no Access step to return to, so Back is a single hop + // out of the connector rather than a two-hop wizard retreat. await act(async () => { buttonByText("Back")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -3217,9 +3331,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonContaining("Google Sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await passAccessStep(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); + await openAccessAdvanced(); await act(async () => { buttonContaining("Share selected sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -3257,9 +3369,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { buttonContaining("Google Sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); await flushReact(); - await passAccessStep(); - await act(async () => { buttonByText("Advanced")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - await flushReact(); + await openAccessAdvanced(); await act(async () => { buttonContaining("Share selected sheets")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); @@ -3555,10 +3665,10 @@ describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => { }); // The curated flow has its own regression test for this. The generic flow is - // a separate three-step model with a separate override, so it needs its own — + // a separate two-step model with a separate override, so it needs its own — // otherwise the waiting screen could quietly drop back to the curated - // two-step labels here and nothing would catch it. - it("keeps the generic three-step model when a pasted endpoint hands off to sign-in", async () => { + // single-step label here and nothing would catch it. + it("keeps the generic two-step model when a pasted endpoint hands off to sign-in", async () => { connectAppMock.mockResolvedValue({ connectionId: "conn-1", application: { id: "app-1", name: "mcp.example.test" }, @@ -3571,7 +3681,7 @@ describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => { await gotoLinkFrame(container, "https://mcp.example.test/mcp"); const stepsBeforeHandoff = stepLabelsOnScreen(); - expect(stepsBeforeHandoff).toEqual(["Pick app", "Access", "Add your key"]); + expect(stepsBeforeHandoff).toEqual(["Pick app", "Add your key"]); await act(async () => { buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); diff --git a/ui/src/pages/apps/Browse.test.tsx b/ui/src/pages/apps/Browse.test.tsx index 6c3bc76dc4..603078e382 100644 --- a/ui/src/pages/apps/Browse.test.tsx +++ b/ui/src/pages/apps/Browse.test.tsx @@ -361,10 +361,28 @@ describe("Connectors landing page", () => { } expect(container.textContent).not.toContain("Private bot"); expect(container.textContent).not.toContain("Chat with agents"); - await act(() => void container.querySelector('button[aria-label="Connect GitHub"]')!.click()); + await act(() => void container.querySelector('button[aria-label="Add key GitHub"]')!.click()); expect(navigateMock).toHaveBeenLastCalledWith("/apps/connect?source=github"); }); + it("names what the card will actually ask for", async () => { + // PAP-659 C4. The verb is derived from the connector's resolved default + // method, so it changes with what this instance can do rather than being a + // fixed string: Notion signs in, GitHub-without-the-cloud-connector wants a + // token, and GitHub with it signs in too. + const github = getAppStoreDefinition("github")!; + listGalleryMock.mockResolvedValue({ + apps: [ + getAppStoreDefinition("notion"), + { ...github, ownershipAvailability: { ...github.ownershipAvailability, platform_shared: true } }, + ], + }); + await renderBrowse(); + expect(container.querySelector('button[aria-label="Connect Notion"]')).not.toBeNull(); + expect(container.querySelector('button[aria-label="Connect GitHub"]')).not.toBeNull(); + expect(container.querySelector('button[aria-label="Add key GitHub"]')).toBeNull(); + }); + it("separates tools and saved bots and hides only bots when chat connectors are disabled", async () => { listGalleryMock.mockResolvedValue({ apps: ["github", "github-code-review-bot"].map(getAppStoreDefinition) }); listApplicationsMock.mockResolvedValue({ applications: [ @@ -407,7 +425,7 @@ describe("Connectors landing page", () => { await renderBrowse(); expect(container.querySelector('[data-app-slug="github"]')).not.toBeNull(); expect(container.querySelector('[data-app-slug="github-code-review-bot"]')).not.toBeNull(); - await act(() => void container.querySelector('button[aria-label="Connect GitHub"]')!.click()); + await act(() => void container.querySelector('button[aria-label="Add key GitHub"]')!.click()); expect(navigateMock).toHaveBeenLastCalledWith("/apps/connect?source=github"); await act(() => void container.querySelector('button[aria-label="Connect GitHub Code Review Bot"]')!.click()); expect(navigateMock).toHaveBeenLastCalledWith("/apps/chat/connect?provider=github&purpose=chat"); diff --git a/ui/src/pages/apps/Browse.tsx b/ui/src/pages/apps/Browse.tsx index 70d79679e5..aeb71f444a 100644 --- a/ui/src/pages/apps/Browse.tsx +++ b/ui/src/pages/apps/Browse.tsx @@ -1,4 +1,8 @@ -import { isRetiredComposioConnection, RETIRED_COMPOSIO_MESSAGE } from "@paperclipai/shared"; +import { + connectionSetupVerbForApp, + isRetiredComposioConnection, + RETIRED_COMPOSIO_MESSAGE, +} from "@paperclipai/shared"; import { ManagedAiConnectionRow } from "@/components/ai-connections/ManagedAiConnectionDetails"; import { useEffect, useMemo, useState, type ReactNode } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; @@ -259,7 +263,14 @@ function connectorAction( }; } if (chatHref) return { label: "Connect", href: chatHref }; - if (row.entry) return { label: "Connect", href: connectHrefFor(row.entry) }; + // PAP-659 C4: the card's verb comes from the same four-state resolver the + // connect screen uses, so "Connect" never turns out to mean "paste a key". + if (row.entry) { + return { + label: connectionSetupVerbForApp(row.entry), + href: connectHrefFor(row.entry), + }; + } return { label: "Connect", href: applicationId ? `/apps/app/${applicationId}/permissions` : null, diff --git a/ui/src/pages/apps/app-connect-policy.test.ts b/ui/src/pages/apps/app-connect-policy.test.ts index 64c73f3e4c..e4a5fa1b7e 100644 --- a/ui/src/pages/apps/app-connect-policy.test.ts +++ b/ui/src/pages/apps/app-connect-policy.test.ts @@ -15,7 +15,13 @@ describe("app connect policy", () => { expect(MCP_DIRECT_OAUTH_CONNECT_SLUGS).toEqual(expect.arrayContaining(["jira", "notion", "sentry"])); expect(isMcpDirectOAuthConnectSlug("notion")).toBe(true); expect(isMcpDirectOAuthConnectSlug("jira")).toBe(true); - expect(isMcpDirectOAuthConnectSlug("asana")).toBe(false); + // PAP-659 step 4: Asana's own OAuth metadata advertises registration, and + // live discovery now outranks its pinned customer-only ownership mode, so + // it reaches the provider directly instead of the client-ID form. + expect(isMcpDirectOAuthConnectSlug("asana")).toBe(true); + // Still false, and for two different reasons worth keeping apart: GitHub's + // one-click path is Paperclip-managed rather than direct, and Slack really + // does require a customer-registered OAuth client. expect(isMcpDirectOAuthConnectSlug("github")).toBe(false); expect(isMcpDirectOAuthConnectSlug("slack")).toBe(false); expect(isMcpDirectOAuthConnectSlug(null)).toBe(false); diff --git a/ui/src/pages/apps/app-detail/PermissionsPanel.group.test.tsx b/ui/src/pages/apps/app-detail/PermissionsPanel.group.test.tsx new file mode 100644 index 0000000000..fb8cf0f09e --- /dev/null +++ b/ui/src/pages/apps/app-detail/PermissionsPanel.group.test.tsx @@ -0,0 +1,72 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot } from "react-dom/client"; +import { MemoryRouter } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ToolCatalogEntry } from "@paperclipai/shared"; +import { ActionsSection } from "./PermissionsPanel"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +vi.mock("@/context/CompanyContext", () => ({ + useCompany: () => ({ selectedCompanyId: "company-1", selectedCompany: { id: "company-1", issuePrefix: "PAP" } }), +})); + +let container: HTMLDivElement | null = null; + +afterEach(() => { + container?.remove(); + container = null; +}); + +const entry = (id: string, riskLevel: string) => + ({ id, toolName: id, title: id, riskLevel, status: "active" }) as unknown as ToolCatalogEntry; + +describe("Permissions group control", () => { + it("sets every action in the group in one change", () => { + const onSetPermission = vi.fn(); + const writes = [entry("send", "write"), entry("delete", "destructive"), entry("label", "write")]; + container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + flushSync(() => + root.render( + + + + + , + ), + ); + + const select = container.querySelector('select[aria-label="Set every action in Write (3)"]'); + expect(select).toBeTruthy(); + expect(select!.value).toBe(""); + flushSync(() => { + select!.value = "ask"; + select!.dispatchEvent(new Event("change", { bubbles: true })); + }); + + expect(onSetPermission).toHaveBeenCalledTimes(1); + const [ids, next] = onSetPermission.mock.calls[0]!; + expect([...ids].sort()).toEqual(["delete", "label", "send"]); + expect(next).toBe("ask"); + }); +}); diff --git a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx index 4ff0d1eb07..033a35f881 100644 --- a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx +++ b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx @@ -53,7 +53,7 @@ export function PermissionsPanel({ connectionId: string; install: InstallState; onSaveAccess: (next: AccessDraft) => void; - onSetActionPermission: (id: string, next: ActionPermission) => void; + onSetActionPermission: (ids: string[], next: ActionPermission) => void; onReviewQuarantined: (enabledIds: string[]) => void; onRefreshActions: () => void; refreshPending: boolean; @@ -222,7 +222,7 @@ export function ActionsSection({ focusId?: string | null; canConfigure: boolean; permissionChangeWarning?: string; - onSetPermission: (id: string, next: ActionPermission) => void; + onSetPermission: (ids: string[], next: ActionPermission) => void; onReviewQuarantined: (enabledIds: string[]) => void; onRefreshActions: () => void; }) { @@ -316,9 +316,9 @@ export function ActionsSection({ disabled={disabled} focusId={focusId} canConfigure={canConfigure} - onSetPermission={(id, next) => { + onSetPermission={(ids, next) => { setShowPermissionChangeWarning(true); - onSetPermission(id, next); + onSetPermission(ids, next); }} /> { + onSetPermission={(ids, next) => { setShowPermissionChangeWarning(true); - onSetPermission(id, next); + onSetPermission(ids, next); }} /> @@ -381,12 +381,45 @@ function ActionGroup({ disabled: boolean; focusId?: string | null; canConfigure: boolean; - onSetPermission: (id: string, next: ActionPermission) => void; + onSetPermission: (ids: string[], next: ActionPermission) => void; }) { if (actions.length === 0) return null; + const groupValue = (() => { + const first = actionPermission(actions[0]!.id, enabledIds, askFirstIds); + return actions.every((action) => actionPermission(action.id, enabledIds, askFirstIds) === first) + ? first + : ""; + })(); return (
-

{title}

+
+

{title}

+ {canConfigure ? ( + // PAP-659 C6b: set the whole group at once, so narrowing a fresh + // connection's writes is one choice rather than one per action; + // per-row overrides stay underneath. + + ) : null} +
{actions.map((action) => ( void; + onSetPermission: (ids: string[], next: ActionPermission) => void; }) { const rowRef = useRef(null); const [testOpen, setTestOpen] = useState(false); @@ -455,7 +488,18 @@ function ActionRow({ data-action-id={action.id} >
-
{title}
+
+ {title} + {/* PAP-659 C7: the gate is only as good as the classifier, so say + what each action was classified as. A misfiled tool is then one + glance to spot and one click to move. */} + + {action.riskLevel} + +
{action.description ? (
{action.description}
) : null} @@ -480,7 +524,7 @@ function ActionRow({ aria-checked={selected} aria-label={`${title}: ${option.label}`} disabled={disabled} - onClick={() => onSetPermission(action.id, option.value)} + onClick={() => onSetPermission([action.id], option.value)} className={cn( "flex h-8 w-8 items-center justify-center rounded-sm text-muted-foreground outline-none transition-colors", "hover:bg-background hover:text-foreground focus-visible:ring-2 focus-visible:ring-ring", diff --git a/ui/src/pages/apps/app-detail/action-permissions.test.ts b/ui/src/pages/apps/app-detail/action-permissions.test.ts new file mode 100644 index 0000000000..8e24bc9bec --- /dev/null +++ b/ui/src/pages/apps/app-detail/action-permissions.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { actionPermissionMutation } from "./action-permissions"; + +describe("actionPermissionMutation", () => { + const enabled = new Set(["a", "b", "c"]); + const askFirst = new Set(["b"]); + + it("sets every action in a group at once", () => { + expect(actionPermissionMutation(["a", "b", "c"], "ask", enabled, askFirst)).toEqual({ + enabled: new Set(["a", "b", "c"]), + askFirst: new Set(["a", "b", "c"]), + }); + expect(actionPermissionMutation(["a", "b", "c"], "off", enabled, askFirst)).toEqual({ + enabled: new Set(), + askFirst: new Set(), + }); + expect(actionPermissionMutation(["a", "b", "c"], "allowed", new Set(), askFirst)).toEqual({ + enabled: new Set(["a", "b", "c"]), + askFirst: new Set(), + }); + }); + + it("leaves actions outside the group alone", () => { + expect(actionPermissionMutation(["a"], "off", enabled, askFirst)).toEqual({ + enabled: new Set(["b", "c"]), + askFirst: new Set(["b"]), + }); + }); +}); diff --git a/ui/src/pages/apps/app-detail/action-permissions.ts b/ui/src/pages/apps/app-detail/action-permissions.ts new file mode 100644 index 0000000000..b4f01cf358 --- /dev/null +++ b/ui/src/pages/apps/app-detail/action-permissions.ts @@ -0,0 +1,27 @@ +/** + * Apply one permission to a set of actions in a single change. + * + * Every save rewrites the connection's whole enabled/ask-first set, so a + * group control must not issue one save per action: each would start from the + * same render and the last would overwrite the rest. + */ +export function actionPermissionMutation( + ids: string[], + next: "off" | "allowed" | "ask", + enabledIds: Set, + askFirstIds: Set, +) { + const enabled = new Set(enabledIds); + const askFirst = new Set(askFirstIds); + for (const id of ids) { + if (next === "off") { + enabled.delete(id); + askFirst.delete(id); + } else { + enabled.add(id); + if (next === "ask") askFirst.add(id); + else askFirst.delete(id); + } + } + return { enabled, askFirst }; +} diff --git a/ui/storybook/fixtures/remoteMcpConnections.ts b/ui/storybook/fixtures/remoteMcpConnections.ts index e5869e76b3..2f3cde27e2 100644 --- a/ui/storybook/fixtures/remoteMcpConnections.ts +++ b/ui/storybook/fixtures/remoteMcpConnections.ts @@ -86,7 +86,7 @@ export function initialReviewState(provider: RemoteMcpProviderId, scenario: Revi url: ["initial", "journey", "connect", "selected_agents"].includes(scenario) ? config.defaultUrl : scenario === "invalid_url" ? "not-a-server-url" : exampleUrl(provider), auth: scenario === "advanced" ? provider === "composio" ? "headers" : "bearer" : config.supportsBrowserAuth ? "auto" : "none", token: "", headers: scenario === "advanced" ? [{ id: "header-1", name: provider === "arcade" ? "Arcade-User-ID" : "X-Session-Key", value: "" }] : [], - advanced: scenario === "advanced", connectStatus: connectCases[scenario] ?? "idle", + connectStatus: connectCases[scenario] ?? "idle", connected: !access && !isConnect, identity: provider === "zapier" ? null : "reviewer@example.invalid", allAgents: scenario !== "selected_agents", agentIds: access && scenario !== "selected_agents" ? [] : ["researcher", "operator"], permissions: Object.fromEntries(tools.map((tool, index) => [tool.id, scenario !== "new_tools" || tool.id === newFixtureTool.id ? "allowed" : index === 1 ? "ask_first" : index === 2 ? "off" : "allowed"])), diff --git a/ui/storybook/prototypes/RemoteMcpConnectionReview.tsx b/ui/storybook/prototypes/RemoteMcpConnectionReview.tsx index abfc994349..9454721bd4 100644 --- a/ui/storybook/prototypes/RemoteMcpConnectionReview.tsx +++ b/ui/storybook/prototypes/RemoteMcpConnectionReview.tsx @@ -123,8 +123,8 @@ export function RemoteMcpConnectionReview({ provider, scenario = "journey", inli
Design review · {remoteMcpProviders[provider].name} — Example accounts and tools. No real sign-in, calls, or credential storage. Use fake values only.
{inline ? Connect {remoteMcpProviders[provider].name} - - : } + + : }