mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 17:33:51 +02:00
feat(connections): name the four setup states once and derive the card verb
Step 3 of the connector simplification. The gallery card and the connect screen were each deciding "what kind of connector is this" from raw method flags, which is how a card comes to offer Connect for something that then demands a pasted key. `connectionSetupStateForMethod` resolves one of instant/authorize/paste/ register, and the card's verb is a function of it. Both follow the instance's ownership availability rather than catalog order, so GitHub reads Add key until an operator configures the cloud connector and Connect afterwards. Two catalog realities the plan had wrong are now pinned by tests: a provider-generated URL (Zapier, Arcade, Executor) is not instant because the operator brings the endpoint, and a templated URL (Shopify) is not either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a3cdba956f
commit
e0fce1623d
5 files changed
+263
-4
No files matched your search
@@ -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<string, ConnectionSetupState | null>();
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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));
|
||||
}
|
||||
@@ -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";
|
||||
|
||||
@@ -243,10 +243,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<HTMLButtonElement>('button[aria-label="Connect GitHub"]')!.click());
|
||||
await act(() => void container.querySelector<HTMLButtonElement>('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("restores the GitHub intent chooser when enabled and hides cached chat rows immediately when disabled", async () => {
|
||||
listGalleryMock.mockResolvedValue({ apps: [getAppStoreDefinition("github")] });
|
||||
chatListMock.mockResolvedValue([{ id: "endpoint-1", provider: "github", status: "active", assignedAgentName: "Chat agent", botLabel: "Chat bot", assignedAgentId: "agent-1" }]);
|
||||
@@ -260,7 +278,7 @@ describe("Connectors landing page", () => {
|
||||
expect(container.querySelector('[data-app-slug="telegram"]')).toBeNull();
|
||||
expect(container.textContent).not.toContain("Chat agent");
|
||||
expect(container.querySelector('a[href*="/apps/chat/"]')).toBeNull();
|
||||
await act(() => void container.querySelector<HTMLButtonElement>('button[aria-label="Connect GitHub"]')!.click());
|
||||
await act(() => void container.querySelector<HTMLButtonElement>('button[aria-label="Add key GitHub"]')!.click());
|
||||
expect(navigateMock).toHaveBeenLastCalledWith("/apps/connect?source=github");
|
||||
});
|
||||
|
||||
|
||||
@@ -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";
|
||||
@@ -250,7 +254,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,
|
||||
|
||||
Reference in new issue
Block a user