From c2205a88b4e7b1639bf759edde46e4ae9a67456a Mon Sep 17 00:00:00 2001 From: scotttong Date: Mon, 28 Sep 2026 20:58:37 -0700 Subject: [PATCH] feat(connections): one connect screen per connector with a stated default PAP-659 step 1 (C0 + bucket A) and step 2 (C1 + C2) of the approved plan. - Delete the Access step. Identity and agent reach resolve to a default that is printed above the primary action and repeated on the completion screen, with the same controls one click away in an Advanced disclosure. - A selected connector is a single screen, so its stepper is gone rather than showing one lonely dot. - Always have a default connection method: the ranking in getRecommendedConnectionMethod is now used instead of discarded for multi-method apps, and the alternates plus the capability choice move into the same Advanced disclosure. - The one primary action carries the external-handoff cue. Co-Authored-By: Claude Opus 5 --- pnpm-lock.yaml | 9 +- .../connections/ConnectionSetupFlow.tsx | 429 ++++++++++++------ .../remote-mcp/RemoteMcpConnectionSetup.tsx | 2 +- ui/src/pages/apps/AppsConnect.test.tsx | 188 +++++--- 4 files changed, 400 insertions(+), 228 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4544b619c3b..ff5c230ba95 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -43,7 +43,7 @@ patchedDependencies: hash: gtvuasrdnibltgd6jvdqufvsdy path: patches/acpx@0.12.0.patch acpx@0.13.1: - hash: ikfco27aeue26z63kex2z5d5ce + hash: jxpmkytjzjh3vghha3pybg7k54 path: patches/acpx@0.13.1.patch embedded-postgres@18.1.0-beta.16: hash: 55uhvnotpqyiy37rn3pqpukhei @@ -508,7 +508,7 @@ importers: version: 0.156.0 acpx: specifier: 0.13.1 - version: 0.13.1(patch_hash=ikfco27aeue26z63kex2z5d5ce) + version: 0.13.1(patch_hash=jxpmkytjzjh3vghha3pybg7k54) ajv: specifier: ^8.20.0 version: 8.20.0 @@ -960,7 +960,7 @@ importers: version: 8.18.0 acpx: specifier: 0.13.1 - version: 0.13.1(patch_hash=ikfco27aeue26z63kex2z5d5ce) + version: 0.13.1(patch_hash=jxpmkytjzjh3vghha3pybg7k54) ajv: specifier: ^8.20.0 version: 8.20.0 @@ -7656,7 +7656,6 @@ packages: opencode-ai@1.18.32: resolution: {integrity: sha512-SCrZWdq44y/EoH2+fE4HLcXS+DzpVqHPzmXk3p2RrufYy8LWvpfhRhKtijb5ktvx8r94ScqToqwHxR0BwS65OQ==} - cpu: [arm64, x64] os: [darwin, linux, win32] hasBin: true @@ -13348,7 +13347,7 @@ snapshots: - bare-buffer - react-native-b4a - acpx@0.13.1(patch_hash=ikfco27aeue26z63kex2z5d5ce): + acpx@0.13.1(patch_hash=jxpmkytjzjh3vghha3pybg7k54): dependencies: '@agentclientprotocol/sdk': 1.4.0(zod@4.4.3) commander: 15.0.0 diff --git a/ui/src/features/connections/ConnectionSetupFlow.tsx b/ui/src/features/connections/ConnectionSetupFlow.tsx index e1907963871..386ac193510 100644 --- a/ui/src/features/connections/ConnectionSetupFlow.tsx +++ b/ui/src/features/connections/ConnectionSetupFlow.tsx @@ -102,7 +102,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 +184,6 @@ function oauthCallbackErrorMessage(outcome: string | null, code: string | null): } const ROUTE_STAGE_BY_STEP: Partial> = { - access: "access", key: "setup", success: "complete", }; @@ -196,10 +195,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 +302,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 +390,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( @@ -901,7 +894,7 @@ function StandardConnectionSetupFlow({ resetGenericAuthState(); setCredentials({}); setConnectResult(null); - setStep("access"); + setStep("key"); navigate(withConnectionIntent("/apps/connect?source=zapier", connectionIntentId)); return; } @@ -930,7 +923,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) @@ -1598,10 +1591,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. @@ -1723,9 +1712,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.", @@ -1908,6 +1897,66 @@ 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 connectionDefaults = step === "key" ? ( + + ) : null; + const showCuratedOAuthState = Boolean( automaticOAuthEntry && step === "key" @@ -1947,6 +1996,15 @@ function StandardConnectionSetupFlow({ } : undefined} authorizationHost={authorizationHost} authorizationUrl={authorizationFallbackUrl} + guidance={entry?.slug === "railway" && accessStepMethod ? ( +
+

{accessStepMethod.guidanceMd}

+
    + {accessStepMethod.warnings?.map((warning) =>
  • {warning}
    • )} +
+
+ ) : null} + defaults={connectionDefaults} onOpenAuthorization={openAuthorizationTab} onRetry={async () => { setOAuthError(null); @@ -1985,20 +2043,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); @@ -2013,7 +2067,7 @@ function StandardConnectionSetupFlow({ oauthHandoffAbortRef.current?.abort(); setOAuthPhase("entry"); setOAuthError(null); - setAppStep("access"); + backToGallery(); }} onCancel={() => { oauthHandoffAbortRef.current?.abort(); @@ -2039,6 +2093,7 @@ function StandardConnectionSetupFlow({ error={oauthError} authorizationHost={authorizationHost} authorizationUrl={authorizationFallbackUrl} + defaults={connectionDefaults} onOpenAuthorization={openAuthorizationTab} onRetry={() => { setOAuthError(null); @@ -2071,10 +2126,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, @@ -2088,7 +2139,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" @@ -2147,7 +2179,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} @@ -2190,7 +2224,7 @@ function StandardConnectionSetupFlow({ setInstallAgentIds(new Set(requestedAgentId ? [requestedAgentId] : [])); setInstallChoice(requestedAgentId ? "specific" : "all"); setGrantKind(reconnectGrantKind ?? (requestedAgentId ? "user" : "organization")); - setStep("access"); + setStep("key"); }} /> )} @@ -2202,7 +2236,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 @@ -2549,7 +2547,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, @@ -2578,6 +2578,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; @@ -2640,6 +2644,9 @@ export function OAuthConnectStateScreen({ + {phase === "entry" && guidance ?
{guidance}
: null} + {phase === "entry" || phase === "error" ? defaults : null} + {phase === "error" && recoveryActions && (recoveryActions.installationUrl || recoveryActions.managementUrl) ? (
{recoveryActions.installationUrl ? ( @@ -2666,6 +2673,7 @@ export function OAuthConnectStateScreen({ {phase === "entry" ? resuming ? `Finish with ${serverName}` : `Continue to ${serverName}` : "Try again"} + {phase === "entry" ?
+ {defaults} +
+ {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 advancedMethodSelection = authenticationSelection; + 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; if (isGoogleSheetsRobotMethod(entry, method)) { const parsed = parseGoogleSheetIds(googleSheetsLinks); @@ -3582,6 +3613,8 @@ function KeyStep({
+ {defaults} +
@@ -4220,7 +4260,93 @@ 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, + sentence, + notice, + disabled = false, + ...accessProps +}: Omit[0], "agents" | "agentsLoading" | "submitLabel" | "onBack" | "onContinue" | "pending" | "continuesToProvider" | "hideFooter" | "bare"> & { + companyId: string; + sentence: string; + /** Shown when the resolved default genuinely cannot apply. Never a step. */ + notice?: string | null; + disabled?: boolean; +}) { + const [open, setOpen] = useState(false); + return ( +
+
+

{sentence}

+
+ {notice ?

{notice}

: null} + + + {open ? + +
+ {}} onContinue={() => {}} /> +
+ +
); } @@ -4283,6 +4409,7 @@ export function ConnectionSetupCompletionScreen({ logoUrl, darkLogoUrl, summary, + statedDefault, onDone, }: { appName: string; @@ -4290,6 +4417,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 ( @@ -4301,6 +4433,7 @@ export function ConnectionSetupCompletionScreen({

{appName} is ready.

+ {statedDefault ?

{statedDefault}

: null}
{summary.map((line) => (
diff --git a/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx b/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx index 6fca08e095e..d68e05bd2c9 100644 --- a/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx +++ b/ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx @@ -63,7 +63,7 @@ export function RemoteMcpConnectionSetup({ provider, state: s, actions: a, agent = 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 : a.saveExit} /> + step={currentStep >= 0 && !s.setupComplete ? "key" : "gallery"} activeIndex={currentStep} labels={["Access", "Connect"]} onCancel={busy || s.step === "management" || s.step === "permissions" || s.step === "draft" ? undefined : 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}

} diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index 29bcf5c236f..2a75ee56c4d 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -171,11 +171,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) { @@ -184,11 +228,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 })); }); @@ -579,6 +622,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?"); @@ -678,12 +722,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"); @@ -715,11 +766,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?"); @@ -736,11 +788,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"]')) @@ -749,43 +804,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("A dedicated account for an agent")) ?.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("Connects as a dedicated agent account"); - 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("A dedicated account for an agent")) + ?.getAttribute("aria-checked")).toBe("true"); }); it("uses Cancel to exit while the bottom Back button stays in the wizard", async () => { @@ -824,6 +864,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"); @@ -844,10 +886,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")); @@ -986,6 +1024,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?"); @@ -1011,9 +1050,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 }); @@ -1647,9 +1689,9 @@ 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?"); + // 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 passAccessStep(); expect(container.textContent).toContain("How do you want to connect?"); @@ -1838,7 +1880,7 @@ 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", ); @@ -1995,6 +2037,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"]')); @@ -2041,19 +2085,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(() => {})); @@ -2067,9 +2112,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 () => { @@ -2103,7 +2146,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(); @@ -2269,9 +2312,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(); @@ -2529,7 +2572,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", { @@ -2575,7 +2618,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-after-retry", { @@ -2708,6 +2751,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 }); }); @@ -2798,6 +2842,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?"); @@ -2941,6 +2986,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(); @@ -3051,21 +3097,15 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ); }); - 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 })); }); @@ -3510,10 +3550,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" }, @@ -3526,7 +3566,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 }));