diff --git a/frontend/src/lib/connections.ts b/frontend/src/lib/connections.ts index f45394d8d..22f297b19 100644 --- a/frontend/src/lib/connections.ts +++ b/frontend/src/lib/connections.ts @@ -1,6 +1,23 @@ // The catalog of third-party accounts a company can act through. This is the // console's view of what *can* be connected; whether a host can actually run // the OAuth handshake depends on its `/connections` surface (see the client). +// +// ## Two routes, one tile (issue #599) +// +// A tile can be connected two ways, and which one is live is a property of the +// *host*, not of the tile: +// +// - **Composio** — the hosted path. Composio runs the OAuth itself and the +// resulting connection is a tool belt the agents actually receive. +// - **Native** — this host's own registered provider application, the +// self-hosted hatch documented on `src/server/ops/connections.rs`. Reported +// as `credentialSource: "static"`. +// +// Until #599 every tile hard-routed to native. On a hosted tenant no +// `OPENCOMPANY_OAUTH_*` variable is injected, so all eleven Connect buttons +// 400'd with "provider is not enabled on this host" — a grid of buttons that +// could never succeed. [`connectRoute`] is now the single place that decides, +// and it can answer "neither", so a button that cannot work is never rendered. export type ConnectionCategory = | "Communication" @@ -12,16 +29,32 @@ export type ConnectionCategory = export interface ConnectionProvider { /** - * The provider identity. This string MUST be identical end-to-end: the - * manifest `[[connection]] provider = "…"`, this catalog `id`, and the - * backend `well_known(provider)` key (`src/server/ops/connections.rs`) are the - * same token. A tile whose `id` has no matching backend key can never - * complete OAuth — `startConnection(id)` resolves no `provider_config` and the - * host answers "provider not enabled". Backend keys today: `slack`, `github`, - * `google`, `gmail`. Keep new tiles aligned to an existing key (or add the - * backend key first); do not invent console-only ids. + * The provider identity in *this host's* namespace: the manifest + * `[[connection]] provider = "…"` and the key `GET …/connections` reports + * status under. + * + * It is also the token the native hatch resolves — `startConnection(id)` needs + * a matching `well_known(id)` key in `src/server/ops/connections.rs` (today: + * `slack`, `github`, `google`, `gmail`). An id outside that set has **no + * native route**, which is no longer a dead tile: {@link connectRoute} sends + * it down the Composio path instead, and reports `unavailable` when neither + * route is open rather than rendering a Connect that 400s. */ id: string; + /** + * The Composio toolkit slug this tile authorizes against — the hosted route, + * and the only route on a hosted tenant. + * + * Composio slugs are lowercase and unpunctuated (`googlecalendar`) while ids + * here are hyphenated (`google-calendar`), and a few differ outright (`x` is + * `twitter`). Stated per tile rather than derived, because {@link toolkitSlug} + * normalization alone cannot produce `twitter` from `x`. + * + * Mirrors the backend's `toolkit_slug()` + * (`src/server/ops/connections_read.rs`), which is what reconciles the two + * namespaces into one row per provider. + */ + toolkit: string; name: string; description: string; category: ConnectionCategory; @@ -34,6 +67,7 @@ export interface ConnectionProvider { export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ { id: "gmail", + toolkit: "gmail", name: "Gmail", description: "Send and read email from a connected inbox.", category: "Communication", @@ -42,6 +76,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "slack", + toolkit: "slack", name: "Slack", description: "Post updates and take requests from your workspace.", category: "Communication", @@ -49,12 +84,8 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ glyph: "#", }, { - // DEAD TILE until aligned: backend `well_known` has no `google-calendar` - // key — the closest existing key is `google`. Connecting this tile fails - // ("provider not enabled") until either the id is changed to `google` (or a - // dedicated `google-calendar` key + scopes are added to - // `well_known`/`provider_config` in src/server/ops/connections.rs). id: "google-calendar", + toolkit: "googlecalendar", name: "Google Calendar", description: "Schedule and read events on a shared calendar.", category: "Productivity", @@ -63,6 +94,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "notion", + toolkit: "notion", name: "Notion", description: "Read and write docs and databases.", category: "Productivity", @@ -70,10 +102,8 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ glyph: "N", }, { - // DEAD TILE until aligned: backend `well_known` has no `google-drive` key - // (closest existing key is `google`). See the `google-calendar` note above — - // align the id to a backend key or add the key before this tile can connect. id: "google-drive", + toolkit: "googledrive", name: "Google Drive", description: "Store and retrieve files and deliverables.", category: "Storage", @@ -82,6 +112,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "dropbox", + toolkit: "dropbox", name: "Dropbox", description: "Sync assets and shared folders.", category: "Storage", @@ -90,6 +121,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "github", + toolkit: "github", name: "GitHub", description: "Open issues and pull requests in your repos.", category: "Developer", @@ -98,6 +130,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "stripe", + toolkit: "stripe", name: "Stripe", description: "Create invoices and read payment activity.", category: "Finance", @@ -106,6 +139,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "hubspot", + toolkit: "hubspot", name: "HubSpot", description: "Sync contacts and deals in your CRM.", category: "Finance", @@ -113,7 +147,10 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ glyph: "H", }, { + // Composio still spells this toolkit `twitter`; the tile keeps the current + // product name. Exactly the case a normalization rule cannot derive. id: "x", + toolkit: "twitter", name: "X", description: "Publish posts and read mentions.", category: "Social", @@ -122,6 +159,7 @@ export const CONNECTION_PROVIDERS: ConnectionProvider[] = [ }, { id: "linkedin", + toolkit: "linkedin", name: "LinkedIn", description: "Publish updates and manage your page.", category: "Social", @@ -138,3 +176,168 @@ export const CONNECTION_CATEGORY_ORDER: ConnectionCategory[] = [ "Social", "Storage", ]; + +// --------------------------------------------------------------------------- +// Which route a tile's Connect takes (issue #599) +// --------------------------------------------------------------------------- + +/** + * Normalize a provider id or toolkit slug to one comparable key. + * + * The console spells ids hyphenated (`google-calendar`), Composio spells slugs + * unpunctuated (`googlecalendar`), and `GET …/connections` returns rows keyed + * either way — manifest rows under the manifest's spelling, reconciled Composio + * rows under the Composio slug. Matching raw strings therefore misses a + * genuinely connected provider and leaves its tile showing Connect. + * + * Mirrors `toolkit_slug()` in `src/server/ops/connections_read.rs`; keep the two + * in step. + */ +export function toolkitSlug(value: string): string { + return value.replace(/[^a-zA-Z0-9]/g, "").toLowerCase(); +} + +/** + * The host's status row for a tile, matched across both spellings. + * + * The tile's own id is tried first — a manifest row is keyed that way — then + * normalized keys, so a reconciled Composio row keyed `googlecalendar` still + * finds the `google-calendar` tile. + * + * **A connected row wins over a disconnected one.** The host can emit *two* rows + * for one provider when its id and its Composio slug do not normalize to the + * same key: `toolkit_slug("x")` is `x`, not `twitter`, so a manifest declaring + * `provider = "x"` produces a disconnected `x` row while Composio's connected + * `twitter` state arrives as a separate appended row. Taking the first match + * would report that tile disconnected while the account is in fact connected — + * the same "two surfaces disagreeing" failure #316 set out to end. Connected + * beats not-connected for the same reason the host unions `native || + * composio_connected` within a single row; this extends that union across the + * alias it cannot currently see. + * + * Direct-id precedence still decides when nothing is connected, so a manifest + * row remains the authority on a provider the host answered for. + */ +export function connectionStateFor( + provider: ConnectionProvider, + states: Record, +): T | undefined { + const wanted = new Set([toolkitSlug(provider.id), toolkitSlug(provider.toolkit)]); + const direct = states[provider.id]; + const matches: T[] = []; + if (direct) matches.push(direct); + for (const state of Object.values(states)) { + if (state !== direct && wanted.has(toolkitSlug(state.provider))) matches.push(state); + } + return matches.find((state) => state.connected) ?? matches[0]; +} + +/** What this host offers for Composio, as far as routing a tile is concerned. */ +export interface ComposioReach { + /** Whether the `composio` feature is compiled into this build. */ + inBuild: boolean; + /** Whether the company explicitly grants `composio`. */ + granted: boolean; + /** + * Whether a credential of **any** tier resolves; `none` means there is + * nothing to authorize against. + * + * Deliberately a boolean rather than the tier itself. Which credential the + * host reaches Composio with is the host's business, and the set of tiers + * grows — #586 adds `company` (the company's own TinyHumans key) alongside + * `attested` and `static`. Routing on "is there one at all" means a new tier + * is additive here by construction: a tenant that can only reach Composio + * through its company key gets the same working Connect as an attested pod, + * with no edit to this rule. + */ + hasCredential: boolean; + /** Open mode — the backend's own allowlist governs, so any slug it permits is reachable. */ + openMode: boolean; + /** The toolkits offered as rows; the hard limit when not in open mode. */ + effectiveToolkits: readonly string[]; +} + +/** + * Whether `toolkit` can actually be authorized against Composio on this host. + * + * The allowlist half matters as much as the credential half: outside open mode + * the manifest list is a real limit, so offering a Connect for a toolkit outside + * it would just move the 400 from one backend to the other. In open mode the + * effective list is a *display* list, not a limit — any slug the backend permits + * is reachable — so it is deliberately not consulted (issue #397). + */ +export function composioCanAuthorize(reach: ComposioReach | null, toolkit: string): boolean { + if (!reach || !reach.inBuild || !reach.granted || !reach.hasCredential) return false; + if (reach.openMode) return true; + const wanted = toolkitSlug(toolkit); + return reach.effectiveToolkits.some((slug) => toolkitSlug(slug) === wanted); +} + +/** + * How a tile's Connect should behave. + * + * - `native` — this host has its own registered provider application (or the + * company already stored a token): the self-hosted hatch, unchanged. + * - `composio` — authorize `toolkit` through Composio's hosted OAuth. + * - `managed` — the platform runs connections for this instance and there is no + * Composio route either; nothing to do here. + * - `unavailable` — no route can succeed, so the tile says so instead of + * offering a button that fails. + */ +export type ConnectRoute = + | { kind: "native" } + | { kind: "composio"; toolkit: string } + | { kind: "managed" } + | { kind: "unavailable" }; + +/** + * Decide the route for one tile — the single rule the grid renders *and* acts + * on, so the button shown and the call made can never disagree. + * + * Precedence, and why: + * + * 1. **`static` → native.** The host registered a provider application for this + * provider, or the company stored its own token. Both are deliberate acts by + * the operator; preferring Composio here would quietly take away the hatch + * they configured. This is the self-hosted route staying supported. + * 2. **Composio, when it can authorize this toolkit.** The hosted path, and the + * only one on a tenant — which is injected no `OPENCOMPANY_OAUTH_*` variable + * at all. It is also the route that makes the connection a capability: a + * native connection is recorded against the company but no agent tool reads + * it (see the catalog advisory in `ConnectionsView`). + * 3. **`attested` → managed.** A platform-projected identity, so connections are + * the platform's to run and no local Connect could work. + * 4. **Otherwise unavailable.** Notably this is where an unknown provider lands + * on a host with no Composio: `credentialSource` is `undefined` because the + * manifest never declared it, and a Connect would 400. + * + * Step 4 is the bug #599 reports. The grid renders every catalog tile, but + * `GET …/connections` only answers for providers the manifest declares — so on + * a tenant that declares none, every tile had `state === undefined`, the + * `attested` guard never fired (there was no row to read it from), and all + * eleven fell through to a Connect that 400'd. + * + * ## Only two tier names appear here, on purpose + * + * `static` and `attested` are named because each answers a question about the + * *local* host: `static` means a native handshake can complete here, `attested` + * means no local one ever can. Every other tier — including `company` from + * #586 — is a statement about which credential the host presents to Composio, + * which this function reads through {@link ComposioReach.hasCredential} rather + * than by name. So the combination "the company credential is set but the + * provider is not in `states`" needs no case of its own: `state` is + * `undefined`, `hasCredential` is true, and the tile gets a working Composio + * Connect — which is exactly the outcome #599 is about. + */ +export function connectRoute( + provider: ConnectionProvider, + state: { credentialSource?: string } | undefined, + reach: ComposioReach | null, +): ConnectRoute { + if (state?.credentialSource === "static") return { kind: "native" }; + if (composioCanAuthorize(reach, provider.toolkit)) { + return { kind: "composio", toolkit: provider.toolkit }; + } + if (state?.credentialSource === "attested") return { kind: "managed" }; + return { kind: "unavailable" }; +} diff --git a/frontend/src/views/ConnectionsView.tsx b/frontend/src/views/ConnectionsView.tsx index 1046fa5ec..886f78fbe 100644 --- a/frontend/src/views/ConnectionsView.tsx +++ b/frontend/src/views/ConnectionsView.tsx @@ -1,9 +1,14 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Check, Info, Loader2, Plug, ShieldCheck } from "lucide-react"; import { toast } from "sonner"; import { me as fetchMe } from "@/api/auth"; import type { OpenCompanyClient } from "@/api/client"; +import { + getComposioStatus, + listComposioConnections, + startComposioAuthorize, +} from "@/api/composio"; import type { ConnectionState } from "@/api/types"; import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { Badge } from "@/components/ui/badge"; @@ -13,7 +18,11 @@ import { Skeleton } from "@/components/ui/skeleton"; import { CONNECTION_CATEGORY_ORDER, CONNECTION_PROVIDERS, + connectionStateFor, + connectRoute, + type ComposioReach, type ConnectionProvider, + type ConnectRoute, } from "@/lib/connections"; import { cn } from "@/lib/utils"; import { armTourResume } from "@/tour/state"; @@ -30,6 +39,15 @@ interface Props { type Load = "loading" | "ready" | "unavailable"; +/** + * How long the Composio status probe may take before the grid stops waiting on + * it. Mirrors the host's own `COMPOSIO_PROBE_TIMEOUT` on the connections read + * path (`src/server/ops/connections_read.rs`) — the same argument applies on + * this side of the wire: the answer it contributes is which route a tile takes, + * and a page that paints honestly beats a page that never paints. + */ +const COMPOSIO_PROBE_TIMEOUT_MS = 5_000; + /** Wire the third-party accounts your company can act through. */ export function ConnectionsView({ client, company }: Props) { // Which (connection, company) this subtree's browser-local state belongs to. @@ -47,6 +65,23 @@ export function ConnectionsView({ client, company }: Props) { // particularly poor greeting, since the failure arrives only after they have // pasted a live credential into a form that could never submit it. const [canManage, setCanManage] = useState(false); + // What Composio offers here, or `null` while unknown / not reachable. This is + // the hosted connect route for every tile (issue #599), so the grid cannot + // decide what a Connect does without it. + const [reach, setReach] = useState(null); + // Whether this instance carries a platform-projected identity, as Composio + // reports it. A second witness for the same host-level fact the connection + // rows carry — and the only one available when the manifest declares no + // connections, which is exactly when the #319 guard used to go dark. + const [attested, setAttested] = useState(false); + // Whether the Composio probe has answered. The grid must not paint before it + // has: `refresh()` routinely resolves first, and a tile rendered on a null + // `reach` reads "Not available on this host" — so every tile would flash that + // and then flip to Connect a moment later. + const [reachSettled, setReachSettled] = useState(false); + // Poll timers for Composio sign-ins in flight, keyed by toolkit, so a company + // switch or unmount cannot leave one running. + const pollTimers = useRef>({}); const refresh = useCallback(async () => { try { @@ -64,6 +99,60 @@ export function ConnectionsView({ client, company }: Props) { void refresh(); }, [refresh]); + // Composio status drives the hosted route for every tile. A host without the + // feature, without the grant, or without a credential simply leaves `reach` + // null, and `connectRoute` falls back to the native/managed/unavailable arms. + useEffect(() => { + let live = true; + setReachSettled(false); + void (async () => { + try { + // Bounded: the shared client has no abort or timeout, and the grid now + // waits on this call before painting — so a host that accepts the + // connection and never answers would hold the page on skeletons + // forever. Losing the race is not an error, it is "no Composio route + // we can confirm", which is what a null `reach` already means. + const status = await Promise.race([ + getComposioStatus(client, company), + new Promise((resolve) => window.setTimeout(() => resolve(null), COMPOSIO_PROBE_TIMEOUT_MS)), + ]); + if (!live) return; + if (!status) { + setReach(null); + setAttested(false); + return; + } + setReach({ + inBuild: status.inBuild, + granted: status.granted, + hasCredential: status.credentialSource !== "none", + openMode: status.openMode, + effectiveToolkits: status.effectiveToolkits, + }); + setAttested(status.credentialSource === "attested"); + } catch { + // No Composio surface on this host — not an error for this page. + if (live) { + setReach(null); + setAttested(false); + } + } finally { + if (live) setReachSettled(true); + } + })(); + return () => { + live = false; + }; + }, [client, company]); + + useEffect(() => { + const timers = pollTimers.current; + return () => { + Object.values(timers).forEach((id) => window.clearTimeout(id)); + pollTimers.current = {}; + }; + }, [company]); + useEffect(() => { let live = true; void (async () => { @@ -80,20 +169,94 @@ export function ConnectionsView({ client, company }: Props) { }; }, [client, company]); - async function connect(p: ConnectionProvider) { + /** The self-hosted hatch: navigate the document to the host's authorize URL. */ + async function connectNative(p: ConnectionProvider) { + const { url } = await client.startConnection(p.id, company); + // Unlike the Composio sign-in below (which opens a tab and survives), this + // navigates the whole document away — taking the product tour's + // in-memory step state with it. Arm a resume marker so the operator comes + // back to the stop they left instead of the tour restarting from step 1 + // (issue #300). No-op when no tour is running, and deliberately after the + // start call succeeds: a provider that isn't configured 400s here and + // never navigates, so it must not leave a marker behind. + armTourResume(scope); + window.location.href = url; + } + + /** + * The hosted route: Composio runs the OAuth on its own side, so there is no + * local callback to wait on. Open its connect URL in a tab and poll this + * company's connection list until the toolkit flips, mirroring + * `ComposioSection.signIn`. + * + * `busy` is cleared by the poll rather than by the caller — the connect is not + * finished when the tab opens, and clearing early would offer a second Connect + * for a sign-in already in flight. + */ + async function connectComposio(p: ConnectionProvider, toolkit: string) { + // A sign-in for this toolkit is already polling (it can have been started + // from a different tile sharing the slug). Clear the flag we just set rather + // than leaving this tile spinning on someone else's flow. + if (pollTimers.current[toolkit] !== undefined) { + setBusy((b) => (b === p.id ? null : b)); + return; + } + const { connectUrl } = await startComposioAuthorize(client, company, toolkit); + // `noopener` keeps the Composio tab from reaching back through + // `window.opener` — it is a third-party page carrying an OAuth flow, so it + // stays. The cost is that the handle is ALWAYS null: with `noopener` (or + // `noreferrer`) set, `window.open` returns null on success exactly as it + // does when a popup is blocked. + // + // So a null check here cannot detect a blocked popup — it fires on every + // successful open. This was reviewed as "handle a blocked popup", written + // that way, and caught in manual testing: the tab opened and the operator + // was told it had not. Detecting the block would mean dropping `noopener` + // and trading a real security property for a nicer error, which is the + // wrong trade on a tab we hand an OAuth URL to. `ComposioSection.signIn` + // opens the same URL the same way and likewise does not check. + window.open(connectUrl, "_blank", "noopener,noreferrer"); + toast.message(`Complete ${p.name} sign-in in the new tab.`); + const deadline = Date.now() + 120_000; + const poll = async () => { + delete pollTimers.current[toolkit]; + if (Date.now() > deadline) { + setBusy((b) => (b === p.id ? null : b)); + toast.message(`${p.name} sign-in timed out. Try again if it didn't complete.`); + return; + } + try { + const rows = await listComposioConnections(client, company); + if (rows.some((r) => r.toolkit.toLowerCase() === toolkit.toLowerCase() && r.connected)) { + setBusy((b) => (b === p.id ? null : b)); + toast.success(`Connected ${p.name}.`); + // Re-read the host's reconciled view so the tile flips to Disconnect. + await refresh(); + return; + } + } catch { + // Ignore transient probe errors while the operator finishes sign-in. + } + pollTimers.current[toolkit] = window.setTimeout(() => void poll(), 2_000); + }; + pollTimers.current[toolkit] = window.setTimeout(() => void poll(), 2_000); + } + + async function connect(p: ConnectionProvider, route: ConnectRoute) { if (busy) return; setBusy(p.id); try { - const { url } = await client.startConnection(p.id, company); - // Unlike the MCP sign-in below (which opens a tab and survives), this - // navigates the whole document away — taking the product tour's - // in-memory step state with it. Arm a resume marker so the operator comes - // back to the stop they left instead of the tour restarting from step 1 - // (issue #300). No-op when no tour is running, and deliberately after the - // start call succeeds: a provider that isn't configured 400s here and - // never navigates, so it must not leave a marker behind. - armTourResume(scope); - window.location.href = url; + if (route.kind === "composio") { + await connectComposio(p, route.toolkit); + return; + } + if (route.kind === "native") { + await connectNative(p); + return; + } + // `managed` / `unavailable` render no Connect button, so reaching here + // would be a rendering bug rather than an operator action. + setBusy(null); } catch { toast.error(`Couldn't start the ${p.name} connection.`); setBusy(null); @@ -119,8 +282,30 @@ export function ConnectionsView({ client, company }: Props) { // this pod carries a platform-minted identity, so every connection is the // platform's to run. One row reporting it is therefore enough to say so once // at the top, rather than only in per-tile copy (issue #319). + // + // Read from the Composio status as well as the connection rows (issue #599). + // `GET …/connections` only answers for providers the manifest declares, so a + // tenant that declares none produced no rows at all — and this went false on a + // host that is unambiguously platform-managed, which is how eleven tiles ended + // up offering a Connect that could only 400. const platformManaged = - load === "ready" && Object.values(states).some((s) => s.credentialSource === "attested"); + load === "ready" && + (Object.values(states).some((s) => s.credentialSource === "attested") || attested); + + // One decision per tile, made once and used for both the button and the click, + // so what is rendered and what is called can never disagree. + const routes = useMemo(() => { + const out = new Map(); + for (const provider of CONNECTION_PROVIDERS) { + const state = connectionStateFor(provider, states); + // A tile the host said nothing about still inherits the instance-level + // `attested` fact — it is a property of the pod, not of one provider. + const effective = + state ?? (platformManaged ? ({ credentialSource: "attested" } as const) : undefined); + out.set(provider.id, { state, route: connectRoute(provider, effective, reach) }); + } + return out; + }, [states, reach, platformManaged]); return (
@@ -193,7 +378,7 @@ export function ConnectionsView({ client, company }: Props) { )} - {load === "loading" ? ( + {load === "loading" || !reachSettled ? (
{Array.from({ length: 6 }).map((_, i) => ( @@ -209,18 +394,21 @@ export function ConnectionsView({ client, company }: Props) { {category}
- {providers.map((p) => ( - void connect(p)} - onDisconnect={() => void disconnect(p)} - /> - ))} + {providers.map((p) => { + const { state, route } = routes.get(p.id) ?? { route: { kind: "unavailable" as const } }; + return ( + void connect(p, route)} + onDisconnect={() => void disconnect(p)} + /> + ); + })}
); @@ -234,37 +422,32 @@ export function ConnectionsView({ client, company }: Props) { /** * One provider tile. * - * The unconnected foot of the card is a tri-state driven by the host's - * `credentialSource` (issue #319) — the same three tiers the Composio section - * above already reports, worded the same way on purpose: + * The unconnected foot of the card renders whatever {@link connectRoute} + * decided, so the button an operator sees is the call the click makes: * - * - `static` — a Connect can succeed here, either through a token this company - * already holds or through this host's own registered provider application - * (the self-hosted hatch). Unchanged: the Connect button, exactly as today. - * - `attested` — this instance carries a platform identity, so connections are - * the platform's to run and no provider credential is (or should be) present - * locally. A local Connect on such a host could only ever 400, so the button - * is replaced by "Managed by the platform" and that failure becomes - * unreachable from the console by construction. - * - `none` — nothing can complete a handshake here; the tile says so instead of - * offering a button that fails. + * - `native` — this host holds a registered provider application for it (or the + * company already stored a token): the self-hosted hatch. The Connect button, + * exactly as before. + * - `composio` — the hosted route. Also a Connect button, but it opens + * Composio's own OAuth in a tab rather than navigating this document. + * - `managed` — a platform identity runs connections for this instance and + * there is no Composio route; nothing to set up locally. + * - `unavailable` — no route can succeed, so the tile says so rather than + * offering a button that 400s. This is the state issue #599 reports missing: + * every undeclared tile fell through to a Connect that could never work. * - * A host that predates the field sends no `credentialSource`; that falls back - * to the Connect button so an older host is never silently disabled. - * - * `platformManaged` covers the tiles the host said nothing about. `/connections` - * only answers for providers the company's manifest declares, but this grid - * renders the whole catalog — so on a hosted instance the undeclared tiles would - * otherwise keep a Connect button sitting under a banner that says connections - * are the platform's, and clicking it would 400. `attested` is a property of the - * *instance*, not of one provider, so it is safe to apply to every tile. `none` - * is NOT: it is provider-specific, and a host can hold a provider app for a - * provider the manifest never declared, where Connect genuinely works. + * The connected foot offers **Disconnect only when there is something local to + * revoke** — i.e. `via` includes `native`. A Composio-only connection has no + * disconnect route on the host at all (`/composio` exposes status, token, + * authorize and connections, and nothing else), so a Disconnect button there + * would call `…/connections/{id}/disconnect`, blank a secret that was never + * set, report success and change nothing. Naming where the connection lives is + * the honest answer until a Composio disconnect route exists. */ function ConnectionCard({ provider, state, - platformManaged, + route, disabled, busy, onConnect, @@ -272,21 +455,26 @@ function ConnectionCard({ }: { provider: ConnectionProvider; state?: ConnectionState; - platformManaged: boolean; + route: ConnectRoute; disabled: boolean; busy: boolean; onConnect: () => void; onDisconnect: () => void; }) { const connected = Boolean(state?.connected); - const source = state?.credentialSource; - const managedByPlatform = source === "attested" || (platformManaged && source === undefined); - const noRoute = source === "none"; + const managedByPlatform = route.kind === "managed"; + const noRoute = route.kind === "unavailable"; // Which namespace actually backs this connection. `native` alone means the // credential sits in the host's catalog, which no agent tool reads yet — worth // distinguishing from a Composio connection, which is a live capability. const via = state?.via ?? []; const nativeOnly = connected && via.length > 0 && !via.includes("composio"); + // Only a native credential can be revoked from here; see the doc above. + // An empty `via` is "this host predates the field" (it is optional on the + // wire), not "Composio owns it" — withholding Disconnect there would strip + // the control from every connection on an older host, so the affordance is + // withheld only when the host affirmatively named Composio and nothing else. + const canDisconnect = connected && (via.length === 0 || via.includes("native")); const unverified = state?.unverified === true; return ( @@ -319,11 +507,18 @@ function ConnectionCard({
- {connected ? ( + {canDisconnect ? ( + ) : connected ? ( +

+ Connected through Composio — manage it in the Composio section above. +

) : managedByPlatform ? (

new URL(page.url()).searchParams.get("connect_error")) .toBeNull(); - // Connecting is still offered: the failure is a retry, not a terminal state. - await expect(page.getByRole("button", { name: "Connect" }).first()).toBeEnabled(); + // The grid is rendered and usable — the failure was a bounce-back, not a + // terminal state. + await expect(page.getByRole("heading", { name: "Communication" })).toBeVisible(); + + // Issue #599: this used to assert `getByRole("button", { name: "Connect" })` + // was enabled. That button was only there because of the bug — this harness + // company declares no `[[connection]]`, the binary carries no `composio` + // feature and `host.sh` passes no `OPENCOMPANY_OAUTH_*`, so no route could + // complete and clicking it 400'd with "provider is not enabled on this host". + // A tile with no route now says so instead, which is what makes the retry + // offer honest rather than merely present. + await expect(page.getByTestId("connection-unavailable-slack")).toBeVisible(); }); test("an unknown failure code still produces a usable message", async ({ page }) => { diff --git a/frontend/test/unit/connection-route.test.ts b/frontend/test/unit/connection-route.test.ts new file mode 100644 index 000000000..9a2bbf1aa --- /dev/null +++ b/frontend/test/unit/connection-route.test.ts @@ -0,0 +1,237 @@ +// Which route a Connections tile's button takes, and whether it gets one at all +// (issue #599). +// +// The bug these pin: the grid renders every catalog tile, but `GET …/connections` +// only answers for providers the company manifest declares. A hosted tenant +// declares none, so every tile had `state === undefined` — the `attested` guard +// had no row to read itself out of, and all eleven fell through to a Connect +// that 400'd with "provider is not enabled on this host". + +import { describe, expect, it } from "vitest"; + +import { + CONNECTION_PROVIDERS, + composioCanAuthorize, + connectRoute, + connectionStateFor, + toolkitSlug, + type ComposioReach, + type ConnectionProvider, +} from "@/lib/connections"; + +/** The tile for `id`, which must exist — a typo here should fail loudly. */ +function tile(id: string): ConnectionProvider { + const found = CONNECTION_PROVIDERS.find((p) => p.id === id); + if (!found) throw new Error(`no such tile: ${id}`); + return found; +} + +/** A host where Composio is live and everything is reachable (open mode). */ +const OPEN: ComposioReach = { + inBuild: true, + granted: true, + hasCredential: true, + openMode: true, + effectiveToolkits: [], +}; + +describe("toolkitSlug", () => { + // Mirrors `toolkit_slug()` in src/server/ops/connections_read.rs — the two + // namespaces only reconcile into one row if both sides normalize the same way. + it("strips punctuation and case, matching the backend rule", () => { + expect(toolkitSlug("google-calendar")).toBe("googlecalendar"); + expect(toolkitSlug("google-drive")).toBe("googledrive"); + expect(toolkitSlug("GitHub")).toBe("github"); + expect(toolkitSlug("gmail")).toBe("gmail"); + }); +}); + +describe("connectionStateFor", () => { + it("matches a row keyed by the manifest's own provider id", () => { + const state = { provider: "slack", connected: true }; + expect(connectionStateFor(tile("slack"), { slack: state })).toBe(state); + }); + + it("matches a reconciled Composio row across the spelling difference", () => { + // The host appends rows for Composio-connected providers keyed by Composio + // slug. `googlecalendar` and the `google-calendar` tile are one provider; + // a raw `states[p.id]` lookup misses it and the tile keeps saying Connect + // while the account is in fact connected. + const state = { provider: "googlecalendar", connected: true }; + expect(connectionStateFor(tile("google-calendar"), { googlecalendar: state })).toBe(state); + }); + + it("matches when only the tile's toolkit differs outright from its id", () => { + // `x` → `twitter` is the case no normalization rule can derive. + const state = { provider: "twitter", connected: true }; + expect(connectionStateFor(tile("x"), { twitter: state })).toBe(state); + }); + + it("is undefined when no namespace mentions the provider", () => { + expect(connectionStateFor(tile("stripe"), {})).toBeUndefined(); + }); + + it("prefers the manifest row when two rows describe the same tile and none is connected", () => { + // Direct-id precedence. Both rows match the `x` tile; with nothing + // connected the manifest's own row is the authority. + const manifest = { provider: "x", connected: false, account: "from-manifest" }; + const composio = { provider: "twitter", connected: false, account: "from-composio" }; + expect(connectionStateFor(tile("x"), { x: manifest, twitter: composio })).toBe(manifest); + }); + + it("prefers a connected row over a disconnected one for the same tile", () => { + // `toolkit_slug("x")` is `x`, not `twitter`, so the host cannot union these + // two itself: a manifest `provider = "x"` yields a disconnected `x` row + // while Composio's connected `twitter` state arrives as a separate appended + // row. Taking the first match would report the tile disconnected while the + // account is in fact connected. + const manifest = { provider: "x", connected: false }; + const composio = { provider: "twitter", connected: true }; + expect(connectionStateFor(tile("x"), { x: manifest, twitter: composio })).toBe(composio); + }); + + it("applies the same union to a hyphenated id and its Composio slug", () => { + const manifest = { provider: "google-calendar", connected: false }; + const composio = { provider: "googlecalendar", connected: true }; + expect( + connectionStateFor(tile("google-calendar"), { + "google-calendar": manifest, + googlecalendar: composio, + }), + ).toBe(composio); + }); +}); + +describe("composioCanAuthorize", () => { + it("is false without a reachable Composio", () => { + expect(composioCanAuthorize(null, "slack")).toBe(false); + expect(composioCanAuthorize({ ...OPEN, inBuild: false }, "slack")).toBe(false); + expect(composioCanAuthorize({ ...OPEN, granted: false }, "slack")).toBe(false); + // `credentialSource: "none"` — nothing to authorize against. + expect(composioCanAuthorize({ ...OPEN, hasCredential: false }, "slack")).toBe(false); + }); + + it("allows any toolkit in open mode, where the backend allowlist governs", () => { + // Issue #397: an empty manifest list means "allow everything", so the + // effective list is a display list here, not a limit. + expect(composioCanAuthorize(OPEN, "hubspot")).toBe(true); + }); + + it("honours the manifest allowlist as a real limit outside open mode", () => { + const narrow: ComposioReach = { + ...OPEN, + openMode: false, + effectiveToolkits: ["gmail", "googlecalendar"], + }; + expect(composioCanAuthorize(narrow, "googlecalendar")).toBe(true); + // Offering a Connect for a toolkit outside the list would only move the + // 400 from one backend to the other. + expect(composioCanAuthorize(narrow, "stripe")).toBe(false); + }); +}); + +describe("connectRoute", () => { + it("routes every tile through Composio on a tenant that declares no connections", () => { + // The #599 regression guard. No manifest rows at all, so no tile has state. + for (const provider of CONNECTION_PROVIDERS) { + expect(connectRoute(provider, undefined, OPEN)).toEqual({ + kind: "composio", + toolkit: provider.toolkit, + }); + } + }); + + it("never offers a Connect when no route can succeed", () => { + // The shipped behaviour before this fix: a button on every tile, and every + // one of them 400s. `unavailable` is what the operator gets instead. + for (const provider of CONNECTION_PROVIDERS) { + expect(connectRoute(provider, undefined, null)).toEqual({ kind: "unavailable" }); + } + }); + + it("keeps the self-hosted hatch when the host registered its own provider app", () => { + // `static` is a deliberate act by the operator — either a registered + // provider application or a token this company stored. Preferring Composio + // here would quietly take away the hatch they configured. + expect(connectRoute(tile("github"), { credentialSource: "static" }, OPEN)).toEqual({ + kind: "native", + }); + expect(connectRoute(tile("github"), { credentialSource: "static" }, null)).toEqual({ + kind: "native", + }); + }); + + it("prefers Composio over a platform identity that runs no connection here", () => { + // `attested` says the platform owns connections, but Composio is a live + // route on the same host — and the one that actually gives agents tools. + expect(connectRoute(tile("notion"), { credentialSource: "attested" }, OPEN)).toEqual({ + kind: "composio", + toolkit: "notion", + }); + }); + + it("falls back to managed when the platform runs connections and Composio does not", () => { + expect(connectRoute(tile("notion"), { credentialSource: "attested" }, null)).toEqual({ + kind: "managed", + }); + }); + + it("routes on whether a Composio credential exists, not on which tier it is", () => { + // Issue #586 adds a `company` tier (the company's own TinyHumans key) + // alongside `attested` and `static`. `connectRoute` names only the two + // tiers that describe the LOCAL host, so a new Composio tier is additive + // by construction and needs no case here. + // + // This pins the specific combination neither #599 nor #586 covers alone: + // the company credential is set, and the provider is not in `states`. + const companyTier: ComposioReach = { ...OPEN, hasCredential: true }; + expect(connectRoute(tile("stripe"), undefined, companyTier)).toEqual({ + kind: "composio", + toolkit: "stripe", + }); + // And a connection row reporting the new tier is not mistaken for the + // native hatch — only `static` means a local handshake can complete. + expect(connectRoute(tile("stripe"), { credentialSource: "company" }, companyTier)).toEqual({ + kind: "composio", + toolkit: "stripe", + }); + expect(connectRoute(tile("stripe"), { credentialSource: "company" }, null)).toEqual({ + kind: "unavailable", + }); + }); + + it("reports unavailable for a provider the host explicitly has no route for", () => { + expect(connectRoute(tile("stripe"), { credentialSource: "none" }, null)).toEqual({ + kind: "unavailable", + }); + }); + + it("gives the eight ids with no native backend key a working Composio route", () => { + // `well_known()` recognises only slack / google / gmail / github, so these + // could never complete a native handshake no matter how the host is + // configured. Composio is what makes them connectable at all. + for (const id of [ + "google-calendar", + "notion", + "google-drive", + "dropbox", + "stripe", + "hubspot", + "x", + "linkedin", + ]) { + expect(connectRoute(tile(id), undefined, OPEN).kind).toBe("composio"); + } + }); +}); + +describe("the tile catalog", () => { + it("gives every tile a Composio toolkit slug", () => { + for (const provider of CONNECTION_PROVIDERS) { + expect(provider.toolkit, `${provider.id} has no toolkit`).toBeTruthy(); + // The slug is what the host is called with, so it must already be in + // Composio's spelling rather than needing normalization at the call site. + expect(toolkitSlug(provider.toolkit)).toBe(provider.toolkit); + } + }); +}); diff --git a/src/server/ops/connections_read.rs b/src/server/ops/connections_read.rs index b8d80ca1a..bcbbf7db8 100644 --- a/src/server/ops/connections_read.rs +++ b/src/server/ops/connections_read.rs @@ -904,6 +904,61 @@ mod tests { assert_eq!(toolkit_slug("gmail"), "gmail"); } + /// Every `toolkit` the console authorizes with must already be in this + /// normalizer's canonical form (issue #599). + /// + /// The console states its eleven Composio slugs explicitly rather than + /// deriving them — `x` maps to `twitter`, which no normalization rule + /// produces — and its doc calls the table a mirror of [`toolkit_slug`]. A + /// mirror with no reflection test drifts silently: loosen or tighten the + /// rule here and those eleven tiles keep authorizing against slugs this + /// host no longer reconciles rows under, surfacing as "provider not + /// enabled" — the exact symptom #599 fixed. + /// + /// So this reads the real console catalog and feeds it through the real + /// normalizer. It asserts a fixed point (`toolkit_slug(t) == t`) rather + /// than `toolkit_slug(id) == toolkit`, because the latter is false for + /// `x`/`twitter` by design — the alias is the reason the table is explicit. + #[test] + fn console_toolkit_slugs_are_canonical_under_this_normalizer() { + use super::toolkit_slug; + + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/frontend/src/lib/connections.ts" + ); + let source = std::fs::read_to_string(path) + .unwrap_or_else(|err| panic!("read the console connection catalog at {path}: {err}")); + + let slugs: Vec = source + .lines() + .filter_map(|line| line.trim().strip_prefix("toolkit: \"")) + .filter_map(|rest| rest.split('"').next()) + .map(str::to_string) + .collect(); + + // Guard against a silent pass: a renamed field or a reformatted catalog + // would otherwise leave this asserting over an empty list. + assert_eq!( + slugs.len(), + 11, + "expected one `toolkit:` per console tile, found {}: {slugs:?}. If the \ + catalog legitimately changed size, update this count; if the field was \ + renamed, update the parse.", + slugs.len() + ); + + for slug in &slugs { + assert_eq!( + &toolkit_slug(slug), + slug, + "console toolkit {slug:?} is not canonical under toolkit_slug; the \ + console would authorize a slug this host reconciles rows under a \ + different key" + ); + } + } + /// A company with no `[[connection]]` entries returns an empty list (200), /// not a 404 — so the console renders "ready" with an empty catalog rather /// than the "unavailable" fallback.