From 467125fafb47a8520856504fecc48d6e32055db1 Mon Sep 17 00:00:00 2001 From: scotttong Date: Wed, 30 Sep 2026 23:13:47 -0700 Subject: [PATCH] feat(connections): one-screen connector setup with stated defaults (#14811) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents use Connections (the Apps catalog) to act in services like Notion, GitHub, Google Workspace and Railway > - Each connector asked the user to answer setup questions before it went to the provider. Most of the questions already had the correct answer selected > - ROADMAP.md lists "simpler setup" for Apps and Connections as ongoing work. This change continues that work > - This pull request removes the questions that Paperclip can answer itself. It states the defaults in one line and moves the choices behind "Change" and onto the Permissions tab > - The benefit is that most connectors take one click in Paperclip and then the provider's own consent screen ## Linked Issues or Issue Description No public issue exists. This is the description, from the enhancement template. **What existing behavior does this improve?** The setup flow for tool connectors in the Apps catalog. **Subsystem affected** Apps and Connections: `ui/src/features/connections`, `ui/src/pages/apps`, the `packages/shared` app definitions, and the OAuth routes in `server/src/routes/tool-access.ts`. **Current behavior** Every connector opened with an Access step. The step asked who can use the connection and which agents get it, and both answers were already selected. 18 connectors also asked "How do you want to connect?" when Paperclip could rank the methods. The Google apps and Postman also asked "What should Paperclip be able to do?" before sign-in. The four gateway connectors (Zapier, Arcade, Composio, Executor) used a separate two-step wizard. Asana was pinned to a customer-owned OAuth app, so the user had to register an app in Asana's developer console. The "Set all" control on the Permissions tab changed only one action. After the user approved access, Railway's consent page showed "you can close this window" and did not return to Paperclip. **Proposed behavior** One screen per connector, with one primary button. The screen states the defaults in one sentence, for example "Connects for everyone in your organization, available to all agents". A "Change" link opens one Advanced panel. When the provider's metadata allows dynamic client registration, Paperclip registers a client itself. Connecting lands on the Permissions tab. On that tab, "Set all" changes every action in the group. **Reason and benefit** The user makes fewer decisions before the connection exists. Most choices are easier to make after the connection, on the Permissions tab, where a change has an immediate effect. **Breaking changes** None. No schema or API change. Existing connections keep their settings. ## What Changed - **No Access step.** `ConnectionSetupFlow` no longer has the Access step. The flow shows the resolved default above the primary button and on the completion screen. The access controls moved into one Advanced panel. The panel opens automatically only when a setting in it is required. - **A default method for every app.** The flow always picks the ranked default method. Alternate methods are in the Advanced panel. The Google and Postman capability choice is not asked before sign-in. The write-capable method is the default. - **Gateway connectors.** `RemoteMcpProductionSetup` (Zapier, Arcade, Composio, Executor) no longer has its own Access step. Its commit path and the main commit path use one helper, `askFirstCatalogEntryIdsFor`, for server-suggested defaults. - **Dynamic registration from live metadata.** `canRegisterOAuthClientDynamically` now allows registration when the provider advertises a registration endpoint, even if the catalog entry lists only customer-owned clients. The Asana and Linear definitions and catalog text match live probes. Asana issues clients for loopback callbacks only, so a hosted deployment still needs an Asana app. - **Connection setup states.** New `packages/shared/src/connection-setup-state.ts` sorts each method into `instant`, `authorize`, `paste` or `register`. The gallery card verb ("Connect" or "Add key") comes from this resolver and the instance's ownership availability. - **Generic MCP.** The generic path no longer asks "Does it need a key?" first. A credential challenge from the server shows the key field. - **Permissions tab.** Each action row shows its risk level. Each group has a "Set all" control. The control sends one change for the whole group. Before, each row's save started from the same render, so the saves overwrote each other. The Zapier/Arcade/Composio/Executor setup screen had the same defect. - **OAuth callback interstitial.** A cross-site browser navigation to `/api/tools/oauth/callback` gets a small same-origin "Finishing your connection…" page. That page repeats the request, and the repeat does the code exchange. Railway's consent page replaces itself after about two seconds, and the code exchange plus tool discovery takes longer than that. The interstitial uses only a meta refresh, because the OAuth code is single-use. Requests without `Sec-Fetch-Site: cross-site` take the old path. - **Linear registers through its MCP server.** Linear pins the console endpoints at `linear.app`. Pinned endpoints now replace discovery only when the method cannot register, or when the connection has an operator-entered client. So a Linear connection now finds the registration endpoint at `mcp.linear.app`. - **Own-OAuth-app recovery stays on the one-click screen.** When the method also accepts a customer-owned client, the client fields are in the Advanced panel. The panel opens after a failed sign-in. "Try again" resumes the draft with the operator's client. - **E2E specs** follow the one-screen flow. The Access-step clicks are removed, the specs open **Change** before they pick agents, and they expect GitHub's **Add key** verb. - **Default permissions do not change.** New connections still allow every action. The user can set actions to Ask first or Off on the Permissions tab. ## Verification - `cd ui && npx vitest run src/pages/apps src/features/connections --no-file-parallelism` - `cd packages/shared && npx vitest run src/app-definitions.test.ts src/connection-setup-state.test.ts` - `cd server && npx vitest run src/__tests__/tool-access-service.test.ts src/__tests__/remote-mcp-connectors.test.ts` - `pnpm check:token-gates` - New tests: - `PermissionsPanel.group.test.tsx` checks that "Set all" sends one change for the whole group. It fails on the old code. - `action-permissions.test.ts` checks the group update. - `connection-setup-state.test.ts` checks the four setup states. - A server test checks that a cross-site callback gets the interstitial and does not use the OAuth state, and that the same-origin repeat completes the connection. - Manual check on a hosted staging deployment. GitHub, Google Drive, Composio, Notion, PostHog and Railway each connected from one screen and returned to the Permissions tab. On Railway, "Set all" changed all 65 write actions, and the change remained after a reload. - Visual changes: snapshot baselines are intentionally not updated. See the `doc/design/DECISION-SHEET.md` entry "Per-change snapshot verification demoted to dormant (Jul 13 2026)". ## Risks - **Fewer confirmation clicks.** Organization-wide access is the default, and the user does not confirm it on a separate step. This was already the preselected answer. The flow shows the default before the user clicks and again after the connection. - **Google write scope.** Google apps now request the write-capable scope by default. A narrower scope needs a new sign-in. - **Dynamic registration from live metadata.** A provider can advertise registration and then reject a redirect URI. Asana rejects hosted callbacks, for example. In that case registration fails, and the customer-owned client path remains available for recovery. - **Callback interstitial.** The OAuth callback adds one same-origin step for cross-site browser navigations. Browsers without `Sec-Fetch-*` headers use the old direct path. - Chat and bot connectors (Discord, Telegram, Microsoft Teams, iMessage) do not change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected β€” check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file editing, browser automation) and extended thinking. It wrote the code, the tests and this description. A human product owner directed the work and tested it by hand. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge πŸ€– Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: scotttong Co-authored-by: Claude Opus 5.5 --- doc/connections/CONNECTOR-PLAYBOOK.md | 10 +- doc/connections/GITHUB.md | 8 +- packages/shared/src/app-definitions.test.ts | 16 +- packages/shared/src/app-definitions.ts | 4 +- .../shared/src/app-definitions/asana.json | 12 +- .../shared/src/app-definitions/linear.json | 3 +- .../shared/src/connection-setup-state.test.ts | 111 +++ packages/shared/src/connection-setup-state.ts | 112 +++ packages/shared/src/index.ts | 7 + .../shared/src/self-serve-mcp-research.json | 3 +- .../src/__tests__/tool-access-service.test.ts | 80 +++ server/src/routes/tool-access.ts | 30 + server/src/services/tool-access.ts | 29 +- tests/e2e/app-not-connected.spec.ts | 1 - tests/e2e/apps-prosumer-mcp-flow.spec.ts | 15 +- tests/e2e/chat-adapters-ui-providers.spec.ts | 9 +- tests/e2e/connection-intents.spec.ts | 1 - tests/e2e/connection-reviews.spec.ts | 1 - tests/e2e/execution-recovery/recovery.spec.ts | 2 +- tests/e2e/in-feed-native/connections.spec.ts | 2 +- .../connections/ConnectionSetupFlow.tsx | 652 ++++++++++++------ .../connections/connection-defaults.test.ts | 49 ++ .../connections/connection-defaults.ts | 30 + .../remote-mcp/RemoteMcpConnectionSetup.tsx | 78 ++- .../remote-mcp/RemoteMcpDesignExample.tsx | 4 +- .../remote-mcp/RemoteMcpProductionSetup.tsx | 19 +- .../features/connections/remote-mcp/types.ts | 1 - ui/src/pages/apps/AppDetail.tsx | 21 +- ui/src/pages/apps/AppsConnect.test.tsx | 536 ++++++++------ ui/src/pages/apps/Browse.test.tsx | 22 +- ui/src/pages/apps/Browse.tsx | 15 +- ui/src/pages/apps/app-connect-policy.test.ts | 8 +- .../PermissionsPanel.group.test.tsx | 72 ++ .../apps/app-detail/PermissionsPanel.tsx | 66 +- .../app-detail/action-permissions.test.ts | 29 + .../apps/app-detail/action-permissions.ts | 27 + ui/storybook/fixtures/remoteMcpConnections.ts | 2 +- .../prototypes/RemoteMcpConnectionReview.tsx | 4 +- ui/storybook/stories/fireflies-pr.stories.tsx | 2 +- 39 files changed, 1550 insertions(+), 543 deletions(-) create mode 100644 packages/shared/src/connection-setup-state.test.ts create mode 100644 packages/shared/src/connection-setup-state.ts create mode 100644 ui/src/features/connections/connection-defaults.test.ts create mode 100644 ui/src/features/connections/connection-defaults.ts create mode 100644 ui/src/pages/apps/app-detail/PermissionsPanel.group.test.tsx create mode 100644 ui/src/pages/apps/app-detail/action-permissions.test.ts create mode 100644 ui/src/pages/apps/app-detail/action-permissions.ts 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} - - : } + + : }