From 66970c529c4af59e6f8735e6501013275eda4574 Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 13 Aug 2026 14:48:10 +0530 Subject: [PATCH 1/2] fix(console): stop offering the inert native OAuth catalog (#822) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Connections page listed the native OAuth catalog as something you can connect, and the route it offered was not broken — which is what made it worth removing. `POST …/connections/{provider}/start` completes a real handshake against a provider application the operator registered, and the callback stores `oauth/{provider}` — read by no agent tool anywhere under `src/harness/` (#396). So a self-hoster could do everything the page asked, watch the tile go green, and give their agents nothing. #599 removed the Connect buttons that failed; this is the one that succeeded and bought nothing. - `connectRoute` loses its `native` arm entirely — the variant is gone from `ConnectRoute`, not merely deprioritised, so no tier and reach combination can reach it. A `static` host takes the Composio route where it has one and reports `unavailable` where it does not. - `buildGridProviders` stops appending `CONNECTION_PROVIDERS` tiles the backend catalog does not carry. It keeps its metadata role; a provider now appears because the host offers it, not because the console has a logo. - The tail it appends instead is what the host reports as **connected**, so retracting the offer cannot hide a credential the company already stored: the tile, its `via: ["native"]` and its Disconnect all survive. `providerId` gained a host-spelling fallback so that Disconnect can still name a connected provider the console has no tile for. - An honest empty state for a host whose catalog really is empty, since the native fallback used to paper over it. The host routes are untouched: this is the console declining to offer a path, not the path being removed. Settling #396 makes the offer honest again and reinstating it is one arm in `connectRoute`. Verified against a live host with the hatch genuinely open (a local stand-in provider behind `OPENCOMPANY_OAUTH_SLACK_AUTHORIZE_URL`/`_TOKEN_URL`): the pre-fix bundle offered Connect on Slack, completed a real handshake and stored the credential; the post-fix bundle over the same data dir drops the five console-only tiles, offers no Connect, and still shows and releases that connection. `npm run typecheck`, `typecheck:e2e`, 603 unit tests and 127 Playwright specs pass. Co-Authored-By: Claude Opus 5 (1M context) --- docs/modules/server/README.md | 36 +++- frontend/src/lib/connections.ts | 110 +++++++----- frontend/src/lib/provider-grid.ts | 125 +++++++++++--- frontend/src/tour/state.ts | 10 ++ frontend/src/views/ConnectionsView.tsx | 33 +--- .../views/connections/ProvidersSection.tsx | 28 ++- .../connections-native-not-offered.spec.ts | 163 ++++++++++++++++++ frontend/test/unit/connection-route.test.ts | 44 ++++- frontend/test/unit/provider-grid.test.ts | 90 +++++++++- 9 files changed, 521 insertions(+), 118 deletions(-) create mode 100644 frontend/test/e2e/connections-native-not-offered.spec.ts diff --git a/docs/modules/server/README.md b/docs/modules/server/README.md index cc231d8e3..fa16407d7 100644 --- a/docs/modules/server/README.md +++ b/docs/modules/server/README.md @@ -286,12 +286,28 @@ have their own focused page: [pausing-workflows.md](pausing-workflows.md). `ops::connections` (feature `oauth`) runs OAuth with **this host's own provider application** — a client id/secret an operator registered themselves and handed -to the process as `OPENCOMPANY_OAUTH__ID` / `_SECRET`. That is the -only way a standalone checkout can complete a handshake, and it is supported for -exactly that reason. It is a hatch, not a deployment mode — the same framing -`ops::composio` uses for its BYO token. A hosted tenant is injected no -`OPENCOMPANY_OAUTH_*` variable at all, so on that host `provider_config` -resolves nothing and a local Connect can only fail. +to the process as `OPENCOMPANY_OAUTH__ID` / `_SECRET`. It is a hatch, +not a deployment mode — the same framing `ops::composio` uses for its BYO token. +A hosted tenant is injected no `OPENCOMPANY_OAUTH_*` variable at all, so on that +host `provider_config` resolves nothing and a local Connect can only fail. + +**The console no longer offers this route** (issue #822). The routes below are +live and unchanged; what changed is that nothing invites an operator down them. +The reason is #396: `oauth_key(provider)` — `"oauth/{provider}"` — is written by +the callback and read by *no agent tool*, zero occurrences under `src/harness/`. +So the hatch worked and conferred nothing, and a self-hoster could register a +provider application, complete a real handshake, see the tile turn green, and +give their agents no ability whatsoever. `frontend/src/lib/connections.ts`'s +`connectRoute` therefore answers `composio`, `managed` or `unavailable` and never +`native`, and `provider-grid.ts` builds the grid from the backend's Composio +catalog rather than from the console's own provider metadata. + +Two things this deliberately does **not** do. It does not remove the routes — +settling #396 by wiring the credential into the harness makes the offer honest +again, and reinstating it is one arm in `connectRoute`. And it does not hide a +credential already stored: a provider `GET …/connections` reports connected keeps +its tile, its `via: ["native"]` and its Disconnect, whether or not the Composio +catalog carries it. The read plane says which it is. `ops::connections_read::connect_route` answers one question per provider — *can a Connect click possibly succeed here, and by @@ -299,10 +315,16 @@ which route?* — as a `credentialSource` tier, stored-wins: | Tier | When | Console | | --- | --- | --- | -| `static` | a token is already stored for this provider (BYO override), **or** this host registered its own provider app *and* has a state signing secret (the hatch) | Connect button, as today | +| `static` | a token is already stored for this provider (BYO override), **or** this host registered its own provider app *and* has a state signing secret (the hatch) | no local Connect since #822 — Composio's if it has one, else "not available here" | | `attested` | no stored token, and the pod carries a platform-**projected** identity (`TINYHUMANS_TOKEN_FILE` naming a file that exists) | "Managed by the platform", no local Connect | | `none` | neither | read-only "not available on this host" | +The tier is still the honest answer to *can a Connect click possibly succeed +here* — it is what the route itself decides by, and `start` still refuses on +`none`. What #822 changed is that the console stopped acting on `static`: the +question it renders is no longer "could this succeed" but "would this confer +anything", and for the native hatch the answer is no until #396 is settled. + **The hatch also needs `OPENCOMPANY_OAUTH_STATE_SECRET`** (issue #318). The `state` nonce binds an in-flight authorization to one company, provider and expiry, and the callback verifies it before exchanging the code — it is the diff --git a/frontend/src/lib/connections.ts b/frontend/src/lib/connections.ts index 77083a91a..1f5fe524d 100644 --- a/frontend/src/lib/connections.ts +++ b/frontend/src/lib/connections.ts @@ -11,9 +11,7 @@ // // What `CONNECTION_PROVIDERS` still owns is what only the console can know: // which ids the host's `well_known` table can run a native handshake for, the -// Composio slug each maps to, and the brand colours. It also supplies the tiles -// on a host with no Composio at all, where the backend catalog comes back empty -// and these eleven are the whole page. +// Composio slug each maps to, and the brand colours. // // ## Two routes, one tile (issue #599) // @@ -31,6 +29,31 @@ // 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. +// +// ## …and now one route offered, not two (issue #822) +// +// The native arm is gone from that decision. It was the half of #599 that +// *could* succeed and still bought the operator nothing: `oauth/{provider}` is +// written by the callback and read by no agent tool — zero occurrences under +// `src/harness/` (#396) — so a self-hoster could register a provider +// application, complete a real handshake, watch the tile go green, and have +// given their agents no ability whatsoever. A Connect that 400s is a bad +// button; a Connect that succeeds and confers nothing is a false promise, and +// the page should not be making it while #396 is unsettled. +// +// So `connectRoute` answers `composio`, `managed` or `unavailable`, and a host +// whose only route is the native hatch gets the same honest "not available +// here" a host with no route at all gets. Two things deliberately survive: +// +// - **A credential already stored stays visible.** Removing the offer must not +// hide a token the company has, so a natively connected provider keeps its +// tile, its `via: ["native"]` and its Disconnect (`lib/provider-grid.ts`). +// Retracting the invitation is this issue; releasing what was accepted under +// it is the operator's call. +// - **The host route itself.** `POST …/connections/{provider}/start` and the +// callback are untouched — this is the console declining to offer a path, +// not the path being removed. Fixing #396 makes the offer honest again, and +// reinstating it is then one arm in this function. export type ConnectionCategory = | "Communication" @@ -46,12 +69,13 @@ export interface ConnectionProvider { * `[[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. + * It is also what `disconnectConnection(id)` is called with, and what the + * host's `well_known` table keys the native hatch by + * (`src/server/ops/connections.rs`; today `slack`, `github`, `google`, + * `gmail`). The console no longer *offers* that hatch (issue #822), so this + * is now the id of a connection already held rather than of one on offer — + * which is why the ids stay aligned to `well_known` even though nothing here + * starts a handshake any more. */ id: string; /** @@ -326,16 +350,18 @@ export function composioCanAuthorize(reach: ComposioReach | null, toolkit: strin /** * 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. + * - `unavailable` — no route this console will offer can succeed, so the tile + * says so instead of offering a button. + * + * There is deliberately no `native` arm (issue #822). It existed, it worked, + * and what it bought was a credential no agent reads (#396) — see the note at + * the top of this file. A host whose only hatch is that one now lands on + * `unavailable`, which is what the tile already renders without an action. */ export type ConnectRoute = - | { kind: "native" } | { kind: "composio"; toolkit: string } | { kind: "managed" } | { kind: "unavailable" }; @@ -346,38 +372,45 @@ export type ConnectRoute = * * 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 + * 1. **Composio, when it can authorize this toolkit.** The hosted path, the only + * one on a tenant — which is injected no `OPENCOMPANY_OAUTH_*` variable at + * all — and now the only one this console offers anywhere. It is also the + * only one that makes a connection a *capability*: `src/harness/composio.rs` + * turns it into tools the agents receive. + * 2. **`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. + * 3. **Otherwise unavailable.** 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 — and, since #822, where a provider + * lands whose *only* route is the native hatch. * - * Step 4 is the bug #599 reports. The grid renders every catalog tile, but + * Step 3 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` is no longer read (issue #822) + * + * It used to take precedence over everything: a host that registered its own + * provider application, or a company that stored its own token, got the native + * hatch and Composio was not allowed to override the operator's deliberate + * configuration. What that reasoning missed is that the hatch confers nothing — + * the stored `oauth/{provider}` secret is read by no agent tool (#396) — so the + * arm preserved an operator's configuration by handing them a green tile and no + * capability. With it gone, such a host takes the Composio route when it has + * one and reports `unavailable` when it does not. + * + * ## One tier name appears 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. + * `attested` is named because it answers a question about the *local* host: no + * local handshake can ever complete on it. 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( // Only the toolkit slug is read, and the merged grid (issue #582) routes tiles @@ -388,7 +421,6 @@ export function connectRoute( 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 }; } diff --git a/frontend/src/lib/provider-grid.ts b/frontend/src/lib/provider-grid.ts index 8a4c3ae10..e21af0783 100644 --- a/frontend/src/lib/provider-grid.ts +++ b/frontend/src/lib/provider-grid.ts @@ -30,15 +30,33 @@ // - **What is connected** comes from `GET …/connections` alone. It is the only // route that reconciles the native `oauth/{provider}` catalog with Composio, // and it is the answer an agent's tool belt is built from. -// - **What can be connected** comes from the backend's Composio catalog, plus -// the local `CONNECTION_PROVIDERS` tiles for the native-only hatch. +// - **What can be connected** comes from the backend's Composio catalog. // - **How to connect it** is `connectRoute`, unchanged and still the single // rule the tile renders and the click calls. // // `CONNECTION_PROVIDERS` is consequently no longer a *list* — the backend -// catalog is. It is metadata for the native route: which ids the host's -// `well_known` table can run an OAuth handshake for, and the brand colours for -// tiles the backend catalog does not cover. +// catalog is. It is metadata: which ids the host's `well_known` table keys, the +// Composio slug each maps to, and the brand colours. +// +// ## The native catalog stops being offerable (issue #822) +// +// It was still half a list here. Every `CONNECTION_PROVIDERS` tile the backend +// catalog did not cover was appended anyway — five of the eleven against the +// host's built-in starter list, all eleven against a host that answers no +// catalog at all — and each of them offered a Connect that #396 says confers +// nothing: `oauth/{provider}` is written by the callback and read by no agent +// tool. An operator on a self-hosted instance could register a provider +// application, complete a real handshake, see the tile go green, and have given +// their agents nothing. `connectRoute` no longer offers that route, and this +// file no longer offers the tile: a provider appears because the backend catalog +// carries it, not because the console has a logo for it. +// +// What is appended instead is narrower and load-bearing: **a provider the host +// reports as connected**, catalog or no catalog. Retracting an offer must not +// hide a credential the company already holds — the tile, its `via` and its +// Disconnect are how an operator sees and releases one. So the tail went from +// "everything we have metadata for" to "everything that is actually connected", +// which is the same union stated honestly: what can be connected, plus what is. import type { ComposioToolkitEntry } from "@/api/composio"; import type { ConnectionState } from "@/api/types"; @@ -58,16 +76,22 @@ import { /** One tile in the merged grid: what to draw, and what a click does. */ export interface GridProvider extends ProviderRow { /** - * The id the *host* knows this provider by — what `startConnection` and - * `disconnectConnection` are called with, and what a manifest declares. + * The id the *host* knows this provider by — what `disconnectConnection` is + * called with, and what a manifest declares. * * Distinct from {@link ProviderRow.slug}, which is Composio's spelling and * what `POST …/composio/authorize` takes. They differ for every hyphenated * tile (`google-calendar` / `googlecalendar`) and outright for `x` / * `twitter`, so collapsing them into one field would silently send one - * namespace's key to the other's route. Falls back to the slug for a provider - * the local catalog has no tile for — which is most of them, and is correct: - * a provider with no native metadata has no native route to name. + * namespace's key to the other's route. + * + * Resolved local metadata first, then **the host's own spelling from + * `GET …/connections`**, then the slug. That middle step is what keeps + * Disconnect working for a natively connected provider the console has no + * tile for (issue #822): its row is now the only reason it has a tile at all, + * and `DELETE …/connections/{provider}` has to name it the way the host does. + * The slug fallback remains correct for the rest — a provider nothing local + * and nothing connected names has no host id to give. */ providerId: string; /** How this tile's button behaves — rendered from and acted on identically. */ @@ -100,6 +124,20 @@ function nativeTileFor(slug: string): ConnectionProvider | undefined { return CONNECTION_PROVIDERS.find((p) => toolkitSlug(p.toolkit) === slug); } +/** + * The local tile a host row names, matched in *either* spelling. + * + * `GET …/connections` keys a row by whichever namespace produced it, so `x` and + * `twitter` are the same tile and only the local metadata knows it. Both the + * status fold and the connected tail need that alias, and needed it identically. + */ +function tileForHostProvider(provider: string): ConnectionProvider | undefined { + const key = toolkitSlug(provider); + return CONNECTION_PROVIDERS.find( + (p) => toolkitSlug(p.id) === key || toolkitSlug(p.toolkit) === key, + ); +} + /** * Collapse the host's connection rows into `slug -> connected`. * @@ -118,9 +156,7 @@ function connectedBySlug( const key = toolkitSlug(state.provider); // Fold the alias too, so a `twitter` row also marks the `x` tile connected // for a console that keys the tile by its id rather than its slug. - const tile = CONNECTION_PROVIDERS.find( - (p) => toolkitSlug(p.id) === key || toolkitSlug(p.toolkit) === key, - ); + const tile = tileForHostProvider(state.provider); for (const alias of new Set([key, tile ? toolkitSlug(tile.toolkit) : key])) { out[alias] = out[alias] === true || state.connected; } @@ -128,6 +164,25 @@ function connectedBySlug( return out; } +/** + * The host's own spelling for each provider it reported, keyed by normalized + * slug. + * + * `GET …/connections` answers under the manifest's spelling (`google-calendar`) + * or Composio's (`googlecalendar`) depending on which namespace the row came + * from, and `DELETE …/connections/{provider}` only accepts the former. First row + * wins; a raw id and its normalized form are the same string in every case where + * they differ only in case. + */ +function hostIdBySlug(states: Readonly>): Record { + const out: Record = {}; + for (const state of Object.values(states)) { + const key = toolkitSlug(state.provider); + if (out[key] === undefined) out[key] = state.provider; + } + return out; +} + /** * Every host row that speaks about one tile, most-informative first. * @@ -158,10 +213,14 @@ function statesFor( * `GET …/connections`, the sole authority on connected. `reach` and * `platformManaged` feed `connectRoute`. * - * The tail is the point of the union: `CONNECTION_PROVIDERS` tiles whose toolkit - * the catalog did not offer are appended anyway. A host with Composio switched - * off returns an empty catalog, and without this the page would render nothing - * at all rather than the eleven native tiles a self-hoster can genuinely use. + * The tail is the point of the union, and since #822 it is a *connected* tail: + * a provider the host reports as connected gets a tile whether or not the + * catalog offers one. It used to be every `CONNECTION_PROVIDERS` tile the + * catalog missed, which is how a host with Composio switched off came to offer + * eleven Connects for a route that stores a credential no agent reads (#396). + * Dropping the offer must not drop the record: a company that connected Slack + * through the hatch keeps its tile, its `via: ["native"]` and its Disconnect, + * on a page that no longer invites anyone else to do the same. */ export function buildGridProviders( catalog: readonly ComposioToolkitEntry[], @@ -171,25 +230,35 @@ export function buildGridProviders( platformManaged: boolean, ): GridProvider[] { const offered = new Set(catalog.map((entry) => toolkitSlug(entry.slug))); - const nativeOnly: ComposioToolkitEntry[] = CONNECTION_PROVIDERS.filter( - (p) => !offered.has(toolkitSlug(p.toolkit)), - ).map((p) => ({ - slug: p.toolkit, - name: p.name, - description: p.description, - logo: null, - categories: [], - })); + const connectedOnly: ComposioToolkitEntry[] = []; + for (const state of Object.values(states)) { + if (!state.connected) continue; + // Local metadata decides the tile's spelling where there is any, so the two + // rows a split alias produces (`x` and `twitter`) land on one tile rather + // than two — the same fold `connectedBySlug` performs for the status. + const tile = tileForHostProvider(state.provider); + const slug = tile ? toolkitSlug(tile.toolkit) : toolkitSlug(state.provider); + if (!slug || offered.has(slug)) continue; + offered.add(slug); + connectedOnly.push({ + slug, + name: tile?.name ?? "", + description: tile?.description ?? "", + logo: null, + categories: [], + }); + } const rows = buildProviderRows( - [...catalog, ...nativeOnly], + [...catalog, ...connectedOnly], extra, connectedBySlug(states), ); + const hostIds = hostIdBySlug(states); return rows.map((row) => { const tile = nativeTileFor(row.slug); - const providerId = tile?.id ?? row.slug; + const providerId = tile?.id ?? hostIds[row.slug] ?? row.slug; const matched = statesFor({ slug: row.slug, providerId }, states); const state = matched[0]; // A tile the host said nothing about still inherits the instance-level diff --git a/frontend/src/tour/state.ts b/frontend/src/tour/state.ts index 2a7ef3ebf..9b2185acc 100644 --- a/frontend/src/tour/state.ts +++ b/frontend/src/tour/state.ts @@ -115,6 +115,16 @@ export function setActiveTourStop(view: string | null): void { * * A **no-op when no tour is running** — that is what lets a caller arm * unconditionally without first asking whether it is inside onboarding. + * + * **Nothing in the console calls this today** (issue #822). Its one caller was + * `ConnectionsView`'s native connect, which set `window.location.href` to the + * host's authorize URL; that offer is gone, and the Composio sign-in it left + * behind opens a tab rather than handing the document away. The read half stays + * wired — `TourController` still honours a marker on mount, and the host still + * redirects an in-flight handshake back to `/connections?connected=…`, so a + * marker written by an older bundle is still consumed. Kept, rather than deleted + * with its caller, because the next surface that navigates the document away + * mid-tour needs exactly this and reconstructing it is the harder half. */ export function armTourResume(scope: LocalScope): void { if (activeStopView === null) return; diff --git a/frontend/src/views/ConnectionsView.tsx b/frontend/src/views/ConnectionsView.tsx index 177e5e959..5e825874e 100644 --- a/frontend/src/views/ConnectionsView.tsx +++ b/frontend/src/views/ConnectionsView.tsx @@ -16,7 +16,6 @@ import { Badge } from "@/components/ui/badge"; import { catalogWarning } from "@/lib/composio-catalog"; import { type ComposioReach } from "@/lib/connections"; import { buildGridProviders, type GridProvider } from "@/lib/provider-grid"; -import { armTourResume } from "@/tour/state"; import { InferenceSection } from "@/views/connections/InferenceSection"; import { McpServersSection } from "@/views/connections/McpServersSection"; import { CompanyCredentialCard } from "@/views/connections/CompanyCredentialCard"; @@ -24,7 +23,6 @@ import { ComposioSection } from "@/views/connections/ComposioSection"; import { ProvidersSection } from "@/views/connections/ProvidersSection"; import { RepositoriesCard } from "@/views/connections/RepositoriesCard"; import { ChannelsSection } from "./connections/ChannelsSection"; -import { useLocalScope } from "@/connections/ConnectionContext"; interface Props { client: OpenCompanyClient; @@ -44,8 +42,6 @@ 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. - const scope = useLocalScope(); const [load, setLoad] = useState("loading"); const [states, setStates] = useState>({}); const [busy, setBusy] = useState(null); @@ -103,9 +99,10 @@ 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. + // Composio status drives the only route this page offers (issue #822). A host + // without the feature, without the grant, or without a credential simply + // leaves `reach` null, and `connectRoute` falls back to managed/unavailable — + // there is no native arm to fall back to any more. useEffect(() => { let live = true; setReachSettled(false); @@ -168,20 +165,6 @@ export function ConnectionsView({ client, company }: Props) { }; }, [client, company]); - /** The self-hosted hatch: navigate the document to the host's authorize URL. */ - async function connectNative(p: GridProvider) { - const { url } = await client.startConnection(p.providerId, 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 @@ -252,12 +235,10 @@ export function ConnectionsView({ client, company }: Props) { await connectComposio(p, p.route.toolkit); return; } - if (p.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. + // would be a rendering bug rather than an operator action. Composio is + // the only route left: the native hatch stopped being offered with #822, + // because what it stores is read by no agent tool (#396). setBusy(null); } catch { toast.error(`Couldn't start the ${p.label} connection.`); diff --git a/frontend/src/views/connections/ProvidersSection.tsx b/frontend/src/views/connections/ProvidersSection.tsx index 4393594fa..868a54c62 100644 --- a/frontend/src/views/connections/ProvidersSection.tsx +++ b/frontend/src/views/connections/ProvidersSection.tsx @@ -216,7 +216,26 @@ export function ProvidersSection({ ))} - {visible.length === 0 && ( + {providers.length === 0 && ( + // The honest empty state (issue #822). This grid used to fall back + // to eleven hardcoded tiles whenever the backend offered no catalog, + // so a host with Composio switched off looked like a page full of + // connectable providers — and every one of those Connects stored a + // credential no agent reads (#396). With the fallback gone, a host + // with no catalog has nothing to show, and saying why beats a bare + // "No provider in All." +

+ This host has no providers to offer yet. They come from Composio, + which runs the sign-in and turns the result into tools your agents + actually receive + {canManage + ? " — set the company's credential above to see its catalog here." + : " — ask an admin to set the company's credential."}{" "} + Anything this company has already connected still appears here. +

+ )} + + {visible.length === 0 && providers.length > 0 && (

{query.trim() !== "" ? ( <> @@ -309,9 +328,10 @@ function ProviderTile({ }) { // `managed` and `unavailable` render no action at all — that is the whole // point of routing the tile (issue #599): a button that could only 400 is - // never drawn. - const connectable = - canManage && !row.connected && (row.route.kind === "composio" || row.route.kind === "native"); + // never drawn. Composio is the only remaining kind that draws one, since the + // native hatch stopped being offered (issue #822) — a Connect that succeeds + // and confers nothing is no better than one that fails. + const connectable = canManage && !row.connected && row.route.kind === "composio"; const state = row.connected ? row.via.length > 0 ? `connected via ${row.via.join(" + ")}` diff --git a/frontend/test/e2e/connections-native-not-offered.spec.ts b/frontend/test/e2e/connections-native-not-offered.spec.ts new file mode 100644 index 000000000..6b270a3e1 --- /dev/null +++ b/frontend/test/e2e/connections-native-not-offered.spec.ts @@ -0,0 +1,163 @@ +import { expect, test } from "@playwright/test"; + +/** + * Issue #822 — the Connections page stops offering the native OAuth catalog. + * + * The route it offered was not broken, which is what made it worth removing. + * `POST …/connections/{provider}/start` completes a real handshake against a + * provider application the operator registered themselves, and the callback + * stores `oauth/{provider}` — which is read by no agent tool anywhere under + * `src/harness/` (#396). So a self-hoster could do everything the page asked, + * watch the tile go green, and have given their agents nothing. #599 removed the + * Connect buttons that *failed*; this is the one that succeeded and bought + * nothing. + * + * ## Which tiles this actually removes, on this host + * + * Worth being precise, because the harness is not the empty-catalog case it + * looks like. `GET …/composio` answers `200` even in a build carrying no + * `composio` feature: `inBuild: false`, `catalogSource: "fallback"`, and a + * built-in starter list of eight slugs. So the grid here is eight backend tiles + * plus — before this change — the five `CONNECTION_PROVIDERS` entries that list + * does not carry (Dropbox, Stripe, HubSpot, X, LinkedIn), appended from console + * metadata alone. Those five are what goes, and they are exactly the issue's + * "a provider the backend catalog does not carry no longer appears solely + * because the console has a logo for it". + * + * The eight stay, because the host offers them — and they say "not available + * here", because this build cannot authorize any of them. + * + * ## What is stubbed, and why only that + * + * Two host answers this harness cannot produce are stubbed at the wire, each + * for a reason that is about the host rather than about convenience: + * + * - **A stored native credential.** Making this host hold one needs a + * registered provider application and a live provider to hand back a code. + * The console's rendering of the resulting row is what is under test. + * - **A host with no catalog at all.** The fallback list above means the + * backend always answers with something; a host predating that route (or one + * whose probe times out) leaves `effectiveCatalog` empty, and that is the + * case this change turns from "eleven tiles" into "none". + * + * Everything else — session, status, route, rendering — is the live host. + * `test/unit/provider-grid.test.ts` pins the same rules on the merge itself. + * + * Drives a running host (see `playwright.config.ts` — the harness brings it up, + * there is no `webServer`). CI does not run Playwright. + */ + +type Page = import("@playwright/test").Page; + +/** The tiles this console used to append from its own metadata alone. */ +const CONSOLE_ONLY = ["dropbox", "stripe", "hubspot", "twitter", "linkedin"]; + +/** + * Open the page with the first-run tour out of the way. + * + * The welcome dialog is a Radix dialog, so while it is open every other element + * is `aria-hidden` and invisible to `getByRole`; it also mounts a beat after the + * navigation resolves, so an immediate `isVisible()` check races it and wins. + */ +async function openConnections(page: Page): Promise { + await page.goto("/#/settings/connections"); + const skip = page.getByRole("button", { name: "Skip for now" }); + await skip + .waitFor({ state: "visible", timeout: 10_000 }) + .then(() => skip.click()) + .catch(() => { + /* already dismissed in this context — nothing to close */ + }); + await expect(skip).toBeHidden({ timeout: 10_000 }); + await expect(page.getByRole("heading", { name: "Providers" })).toBeVisible({ timeout: 30_000 }); +} + +test("a provider the backend does not offer is no longer listed", async ({ page }) => { + await openConnections(page); + + for (const slug of CONSOLE_ONLY) { + await expect( + page.getByTestId(`provider-${slug}`), + `${slug} is still listed, and only the console's own metadata says it exists`, + ).toHaveCount(0); + } + + // The host's own eight are untouched — this removes an invented list, not the + // page. Slack is the one that matters most: it is in the backend's list AND + // is one of the four ids `well_known` can run a native handshake for, so if + // anything were going to keep a native offer alive it would be this tile. + await expect(page.getByTestId("provider-slack")).toBeVisible(); + await expect(page.getByTestId("provider-slack")).toContainText(/not available here/i); +}); + +test("no Connect is offered anywhere on a host that cannot authorize one", async ({ page }) => { + await openConnections(page); + + // The tile IS the button when a tile is connectable, so this is the + // affordance in its rendered form. This host has no Composio credential, and + // the native hatch is no longer an offer — so there is nothing to click, which + // before #822 was true of every tile EXCEPT one the operator had configured + // `OPENCOMPANY_OAUTH_*` for. + await expect( + page.locator("[data-testid^='provider-'] button[aria-label^='Connect ']"), + ).toHaveCount(0); +}); + +test("a provider already connected natively keeps its tile and its Disconnect", async ({ + page, +}) => { + // The host's own answer, with one connected native row added — see the header + // for why this row cannot come from the harness itself. + await page.route("**/connections", async (route) => { + if (route.request().method() !== "GET") return route.fallback(); + const response = await route.fetch(); + const rows = response.ok() ? await response.json() : []; + await route.fulfill({ + json: [ + ...(Array.isArray(rows) ? rows : []), + { + provider: "slack", + connected: true, + via: ["native"], + credentialSource: "static", + account: "acme-workspace", + }, + ], + }); + }); + + await openConnections(page); + + const tile = page.getByTestId("provider-slack"); + await expect(tile, "removing the offer must not hide a credential the company holds").toBeVisible(); + await expect(tile).toContainText("acme-workspace"); + + // Releasable, and by the only route that releases anything here: + // `DELETE …/connections/{provider}` blanks this host's own secret, which is + // exactly what a `via: ["native"]` row is. + await expect(tile.getByRole("button", { name: "Disconnect Slack" })).toBeVisible(); + + // But still not an invitation: a connected tile is not a Connect affordance, + // and nothing else on the page became one either. + await expect( + page.locator("[data-testid^='provider-'] button[aria-label^='Connect ']"), + ).toHaveCount(0); +}); + +test("a host with no catalog says why the grid is empty", async ({ page }) => { + // The case the native fallback used to paper over. Before #822 this rendered + // eleven Connect-able tiles from console metadata; the honest answer is none, + // and "No provider in All." would read as a broken filter rather than as a + // host that has nothing to offer. + await page.route("**/composio", async (route) => { + if (route.request().method() !== "GET") return route.fallback(); + await route.fulfill({ status: 404, json: { error: "not_found" } }); + }); + + await openConnections(page); + + await expect(page.locator("[data-testid^='provider-']")).toHaveCount(0); + const empty = page.getByTestId("providers-empty"); + await expect(empty).toBeVisible(); + await expect(empty).toContainText(/composio/i); +}); diff --git a/frontend/test/unit/connection-route.test.ts b/frontend/test/unit/connection-route.test.ts index 4ec4223d2..25f157205 100644 --- a/frontend/test/unit/connection-route.test.ts +++ b/frontend/test/unit/connection-route.test.ts @@ -162,18 +162,50 @@ describe("connectRoute", () => { } }); - 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. + it("no longer offers the self-hosted hatch, whose credential no agent reads", () => { + // Issue #822. Both assertions used to read `{ kind: "native" }`, on the + // reasoning that `static` is a deliberate act by the operator — a + // registered provider application, or a token this company stored — and + // that preferring Composio would take away the hatch they configured. + // + // What that missed is what the hatch confers: nothing. `oauth/{provider}` + // is written by the callback and read by no agent tool — zero occurrences + // under `src/harness/` (#396) — so the arm preserved the operator's + // configuration by handing them a green tile and no capability. A Connect + // that 400s is a bad button; one that succeeds and buys nothing is a false + // promise, and #599 only fixed the first kind. + // + // A host that also reaches Composio now takes the route that does confer + // something... expect(connectRoute(tile("github"), { credentialSource: "static" }, OPEN)).toEqual({ - kind: "native", + kind: "composio", + toolkit: "github", }); + // ...and a host whose only route was the hatch says so, which is what the + // tile already renders without an action. expect(connectRoute(tile("github"), { credentialSource: "static" }, null)).toEqual({ - kind: "native", + kind: "unavailable", }); }); + it("returns no native route for any tile, under any host shape", () => { + // The regression guard for #822 as a whole: the arm is gone, not merely + // deprioritised, so no combination of tier and reach can reach it. `static` + // is the tier that used to, and it is in the sweep. + const tiers = [undefined, "static", "attested", "company", "none"] as const; + for (const provider of CONNECTION_PROVIDERS) { + for (const tier of tiers) { + for (const reach of [OPEN, null]) { + const state = tier === undefined ? undefined : { credentialSource: tier }; + expect( + connectRoute(provider, state, reach).kind, + `${provider.id} still routes natively for ${tier ?? "no"} state`, + ).not.toBe("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. diff --git a/frontend/test/unit/provider-grid.test.ts b/frontend/test/unit/provider-grid.test.ts index 5bf82a371..cc63edd01 100644 --- a/frontend/test/unit/provider-grid.test.ts +++ b/frontend/test/unit/provider-grid.test.ts @@ -104,19 +104,93 @@ describe("buildGridProviders", () => { expect(tile.providerId).toBe("google-calendar"); }); - it("keeps every local tile when the host offers no Composio catalog", () => { - // A self-hoster with Composio switched off gets an empty catalog. Without - // the union the page would render nothing at all rather than the tiles they - // can genuinely connect natively. - const providers = buildGridProviders([], [], {}, null, false); + it("offers no tile at all when the host's catalog is empty", () => { + // Issue #822, and the assertion this replaces said the opposite: every one + // of the eleven local tiles survived an empty catalog, so a self-hoster with + // Composio switched off saw a full page of Connect buttons. Each of those + // completed a real handshake and stored `oauth/{provider}` — which no agent + // tool reads (#396). The honest render of an inert route is no tile. + expect(buildGridProviders([], [], {}, null, false)).toEqual([]); for (const local of CONNECTION_PROVIDERS) { expect( - providers.some((p) => p.slug === local.toolkit), - `${local.id} lost its tile with no Composio catalog`, - ).toBe(true); + buildGridProviders([], [], {}, OPEN, false).some((p) => p.slug === local.toolkit), + `${local.id} is still offered from local metadata alone`, + ).toBe(false); } }); + it("keeps a natively connected provider listed, with its Disconnect", () => { + // The half that must NOT go with the offer. Retracting an invitation is not + // permission to hide a credential the company already stored: this is the + // only surface that shows it exists and the only one that releases it. + const providers = buildGridProviders( + [], + [], + states({ + provider: "slack", + connected: true, + via: ["native"], + credentialSource: "static", + account: "acme-workspace", + }), + null, + false, + ); + const tile = bySlug(providers, "slack"); + expect(tile.connected).toBe(true); + expect(tile.via).toEqual(["native"]); + expect(tile.canDisconnect).toBe(true); + expect(tile.account).toBe("acme-workspace"); + // And the host's own id, which is what `DELETE …/connections/{provider}` + // takes — the tile exists because of that row, so it must name it. + expect(tile.providerId).toBe("slack"); + }); + + it("lists a connected provider the console has no metadata for", () => { + // The tail is now "what is connected", not "what we have a logo for", so a + // provider only the manifest ever named still gets a tile — and a Disconnect + // addressed the way the host spells it, not the way the slug normalizes. + const providers = buildGridProviders( + [], + [], + states({ provider: "zoom-pro", connected: true, via: ["native"] }), + null, + false, + ); + const tile = bySlug(providers, "zoompro"); + expect(tile.connected).toBe(true); + expect(tile.canDisconnect).toBe(true); + expect(tile.providerId).toBe("zoom-pro"); + }); + + it("does not list a provider the host merely answered about", () => { + // A disconnected row is the host saying "not connected", which is the same + // information as the tile's absence — and listing it would put the offer + // back under a different name. + expect( + buildGridProviders([], [], states({ provider: "slack", connected: false }), null, false), + ).toEqual([]); + }); + + it("folds a connected alias into one tile rather than appending a second", () => { + // The `x` / `twitter` split, now that a connected row can create a tile of + // its own: both rows describe one provider, and the local metadata is what + // says so. Two tiles here would reintroduce the duplicate #582 removed. + const providers = buildGridProviders( + [], + [], + states( + { provider: "x", connected: false }, + { provider: "twitter", connected: true, via: ["composio"] }, + ), + null, + false, + ); + expect(providers).toHaveLength(1); + expect(providers[0].slug).toBe("twitter"); + expect(providers[0].providerId).toBe("x"); + }); + it("does not duplicate a local tile the backend catalog also offers", () => { const providers = buildGridProviders([entry("gmail", { name: "Gmail" })], [], {}, OPEN, false); expect(providers.filter((p) => p.slug === "gmail")).toHaveLength(1); From 88d6a14055f22d470cf71e8c9958593d88aef80f Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 13 Aug 2026 17:13:45 +0530 Subject: [PATCH 2/2] docs(server): split connections out of the module README (#822) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's `assert-md-line-cap.sh` went red: this branch's edits took `docs/modules/server/README.md` to 504 lines. The 500 cap is not arbitrary — CLAUDE.md's remedy is to split the topic into a focused file and link it from the folder's README, which is what `workflow-routes.md`, `pausing-workflows.md` and `authority.md` already are. Connections is the natural seam and the topic this branch edits: the hatch and why the console stopped offering it, the `credentialSource` tiers, the single status behind the one provider grid, and the two disconnect routes are one subject spanning three sections. Moved verbatim into `connections.md` with the headings promoted a level; the README keeps a pointer in the same shape the other three splits use, and lands at 374 lines — headroom rather than one line under the wire. Also corrects `ConnectionCredentialSource`'s `static` doc, which still read "Connect works". The handshake does; the console stopped offering it. Co-Authored-By: Claude Opus 5 (1M context) --- docs/modules/server/README.md | 142 ++-------------------------- docs/modules/server/connections.md | 143 +++++++++++++++++++++++++++++ frontend/src/api/types.ts | 6 +- 3 files changed, 154 insertions(+), 137 deletions(-) create mode 100644 docs/modules/server/connections.md diff --git a/docs/modules/server/README.md b/docs/modules/server/README.md index a5057be0e..bf1c72ea9 100644 --- a/docs/modules/server/README.md +++ b/docs/modules/server/README.md @@ -282,142 +282,12 @@ The pause switch, what it does **not** stop, why it lives in `disabled_workflows` rather than the manifest, and the create/edit disarm rule have their own focused page: [pausing-workflows.md](pausing-workflows.md). -### Connections: hosted versus the self-hosted hatch - -`ops::connections` (feature `oauth`) runs OAuth with **this host's own provider -application** — a client id/secret an operator registered themselves and handed -to the process as `OPENCOMPANY_OAUTH__ID` / `_SECRET`. It is a hatch, -not a deployment mode — the same framing `ops::composio` uses for its BYO token. -A hosted tenant is injected no `OPENCOMPANY_OAUTH_*` variable at all, so on that -host `provider_config` resolves nothing and a local Connect can only fail. - -**The console no longer offers this route** (issue #822). The routes below are -live and unchanged; what changed is that nothing invites an operator down them. -The reason is #396: `oauth_key(provider)` — `"oauth/{provider}"` — is written by -the callback and read by *no agent tool*, zero occurrences under `src/harness/`. -So the hatch worked and conferred nothing, and a self-hoster could register a -provider application, complete a real handshake, see the tile turn green, and -give their agents no ability whatsoever. `frontend/src/lib/connections.ts`'s -`connectRoute` therefore answers `composio`, `managed` or `unavailable` and never -`native`, and `provider-grid.ts` builds the grid from the backend's Composio -catalog rather than from the console's own provider metadata. - -Two things this deliberately does **not** do. It does not remove the routes — -settling #396 by wiring the credential into the harness makes the offer honest -again, and reinstating it is one arm in `connectRoute`. And it does not hide a -credential already stored: a provider `GET …/connections` reports connected keeps -its tile, its `via: ["native"]` and its Disconnect, whether or not the Composio -catalog carries it. - -The read plane says which it is. `ops::connections_read::connect_route` answers -one question per provider — *can a Connect click possibly succeed here, and by -which route?* — as a `credentialSource` tier, stored-wins: - -| Tier | When | Console | -| --- | --- | --- | -| `static` | a token is already stored for this provider (BYO override), **or** this host registered its own provider app *and* has a state signing secret (the hatch) | no local Connect since #822 — Composio's if it has one, else "not available here" | -| `attested` | no stored token, and the pod carries a platform-**projected** identity (`TINYHUMANS_TOKEN_FILE` naming a file that exists) | "Managed by the platform", no local Connect | -| `none` | neither | read-only "not available on this host" | - -The tier is still the honest answer to *can a Connect click possibly succeed -here* — it is what the route itself decides by, and `start` still refuses on -`none`. What #822 changed is that the console stopped acting on `static`: the -question it renders is no longer "could this succeed" but "would this confer -anything", and for the native hatch the answer is no until #396 is settled. - -**The hatch also needs `OPENCOMPANY_OAUTH_STATE_SECRET`** (issue #318). The -`state` nonce binds an in-flight authorization to one company, provider and -expiry, and the callback verifies it before exchanging the code — it is the -flow's CSRF defence. That signing key used to fall back to a literal baked into -this repository, which made the value public, identical across every -unconfigured deployment, and constructible rather than obtainable: verifying it -proved only that it was well-formed. There is now **no default**. A host with a -registered provider application but no secret reports `none` rather than -offering a button whose check is void, `start` refuses with a message naming the -variable, and the process logs the misconfiguration once — a tile has no room to -name a variable, and an operator reads logs. Whitespace-only counts as unset, so -an empty shell expansion gets the closed door rather than a secret of `" "`. - -`attested` deliberately requires the projected-file tier, not -`TinyhumansTokenSource::from_env` as a whole: that resolver also accepts a -long-lived `TINYHUMANS_API_KEY`, which a self-hoster commonly sets to buy -inference. Accepting it here would tell such an operator their working Connect -button is platform-managed and take it away. Both the REST route and the GraphQL -`Company.connections` resolver project the field through the same -`connect_route_from_env`, so the two read shapes cannot drift. - -**Provider mapping to the platform backend.** Its registered OAuth providers are -`notion`, `google`, `gmail`, `github`, `twitter`, `discord` and `instagram`. Two -consequences for the console catalog: `gmail` is a registered provider *name* but -not a separate provider application — it is Google's app requested with the Gmail -skill scopes, so a Gmail connect and a Google connect share one grant (which is -why the backend merges scopes incrementally rather than replacing them). And -there is **no Slack provider** at all (the backend's only Slack credential is an -internal alerting bot), so Slack has no hosted route except Composio, which runs -its own OAuth. - -### One connection status, for one console list (issue #582) - -`GET …/connections` is the **only** answer to "what is connected". It reconciles -the native `oauth/{provider}` catalog with a live Composio probe into one row per -provider, marking which namespaces reported it in `via` — and the console renders -exactly one provider grid from it (`frontend/src/lib/provider-grid.ts`). - -That took removing a gate. `composio_view` used to discard the Composio half -unless the company explicitly granted the `composio` tool namespace, while -`GET …/composio/connections` — which the console's *other* provider list read — -never consulted the grant. The two lists therefore disagreed by construction, not -by timing: 13 of the 21 shipped companies grant no `composio`, so for most of -them one screen said both "connected" and "not connected" about the same account, -and the second list's Connect button was actionable. - -The grant governs whether **agents receive Composio tools**. It never governed -whether a handshake completes — `resolve_tenant` reads the credential and the -toolkit allowlist and nothing else, so the sign-in the gate hid worked perfectly -well from the surface that ignored it. It is now reported (`granted` on -`GET …/composio`) and stated as a caveat next to the connected badge, rather than -silently deciding what the page may show. - -Two consequences worth knowing: - -- The probe now runs for any company holding a credential, not only granting - ones. It stays bounded by `COMPOSIO_PROBE_TIMEOUT` and still degrades to - `unverified` rather than to a confident "not connected". -- `reconcile` is split from `project_connections` purely as a test seam: the - probe is a network call with no injection point, and the removed gate lived on - the far side of it, which is how it survived unasserted. - -### Releasing a connection: two routes, not interchangeable (issue #404) - -There are two disconnects and they act on different things: - -- `POST …/connections/{provider}/disconnect` blanks this host's own - `oauth/{provider}` secret and best-effort revokes it upstream. It has never - touched Composio. -- `DELETE …/composio/connections/{connection_id}` revokes **one connected - account** at Composio. Addressed by connection id, because a company can hold - two accounts for one toolkit and exactly one of them is being released. - -Until the second route existed the console sent every Disconnect to the first, -so a Composio-connected provider answered `200`, reported success, and was still -connected on the next refresh. The console now routes by what the provider is -actually connected through (`disconnectRouteFor` in -`frontend/src/lib/provider-grid.ts`), preferring the Composio account when a -provider is connected through both — the native secret is inert until #396 -lands, so blanking it would release nothing an agent can feel. - -`GET …/composio/connections` carries the accounts a revoke is addressed to: -`accounts[]` per toolkit, each with Composio's verbatim `status`, its -`createdAt` when Composio published one, and the account label when the provider -did. `toolkit` and `connected` keep their previous meaning, so the callers that -read only those two are untouched. - -**Which account an agent acts as is not decided here.** `composio_execute` posts -`{tool, arguments}` and no connection id (`src/harness/composio.rs`), so -Composio resolves it for the entity — nothing in this codebase selects, orders -or defaults an account, and the console's provider detail view says so rather -than marking one as the default. Changing that means sending a connection id on -execute, which is a harness change, not a console one. +### Connections: hosted versus the self-hosted hatch (issues #396, #404, #582, #822) + +The self-hosted OAuth hatch and why the console stopped offering it, the +`credentialSource` tiers, the single connection status behind the console's one +provider grid, and the two non-interchangeable disconnect routes have their own +focused page: [connections.md](connections.md). ## tiny.place A2A inbound + discovery (`tinyplace` feature) diff --git a/docs/modules/server/connections.md b/docs/modules/server/connections.md new file mode 100644 index 000000000..2da8173ae --- /dev/null +++ b/docs/modules/server/connections.md @@ -0,0 +1,143 @@ +# Connections: hosted, the self-hosted hatch, and releasing one + +Split out of [README.md](README.md) when that file reached the 500-line cap. +The write routes are `ops::connections` (feature `oauth`) and `ops::composio`; +the read plane is `ops::connections_read`. + +## Hosted versus the self-hosted hatch + +`ops::connections` (feature `oauth`) runs OAuth with **this host's own provider +application** — a client id/secret an operator registered themselves and handed +to the process as `OPENCOMPANY_OAUTH__ID` / `_SECRET`. It is a hatch, +not a deployment mode — the same framing `ops::composio` uses for its BYO token. +A hosted tenant is injected no `OPENCOMPANY_OAUTH_*` variable at all, so on that +host `provider_config` resolves nothing and a local Connect can only fail. + +**The console no longer offers this route** (issue #822). The routes below are +live and unchanged; what changed is that nothing invites an operator down them. +The reason is #396: `oauth_key(provider)` — `"oauth/{provider}"` — is written by +the callback and read by *no agent tool*, zero occurrences under `src/harness/`. +So the hatch worked and conferred nothing, and a self-hoster could register a +provider application, complete a real handshake, see the tile turn green, and +give their agents no ability whatsoever. `frontend/src/lib/connections.ts`'s +`connectRoute` therefore answers `composio`, `managed` or `unavailable` and never +`native`, and `provider-grid.ts` builds the grid from the backend's Composio +catalog rather than from the console's own provider metadata. + +Two things this deliberately does **not** do. It does not remove the routes — +settling #396 by wiring the credential into the harness makes the offer honest +again, and reinstating it is one arm in `connectRoute`. And it does not hide a +credential already stored: a provider `GET …/connections` reports connected keeps +its tile, its `via: ["native"]` and its Disconnect, whether or not the Composio +catalog carries it. + +The read plane says which it is. `ops::connections_read::connect_route` answers +one question per provider — *can a Connect click possibly succeed here, and by +which route?* — as a `credentialSource` tier, stored-wins: + +| Tier | When | Console | +| --- | --- | --- | +| `static` | a token is already stored for this provider (BYO override), **or** this host registered its own provider app *and* has a state signing secret (the hatch) | no local Connect since #822 — Composio's if it has one, else "not available here" | +| `attested` | no stored token, and the pod carries a platform-**projected** identity (`TINYHUMANS_TOKEN_FILE` naming a file that exists) | "Managed by the platform", no local Connect | +| `none` | neither | read-only "not available on this host" | + +The tier is still the honest answer to *can a Connect click possibly succeed +here* — it is what the route itself decides by, and `start` still refuses on +`none`. What #822 changed is that the console stopped acting on `static`: the +question it renders is no longer "could this succeed" but "would this confer +anything", and for the native hatch the answer is no until #396 is settled. + +**The hatch also needs `OPENCOMPANY_OAUTH_STATE_SECRET`** (issue #318). The +`state` nonce binds an in-flight authorization to one company, provider and +expiry, and the callback verifies it before exchanging the code — it is the +flow's CSRF defence. That signing key used to fall back to a literal baked into +this repository, which made the value public, identical across every +unconfigured deployment, and constructible rather than obtainable: verifying it +proved only that it was well-formed. There is now **no default**. A host with a +registered provider application but no secret reports `none` rather than +offering a button whose check is void, `start` refuses with a message naming the +variable, and the process logs the misconfiguration once — a tile has no room to +name a variable, and an operator reads logs. Whitespace-only counts as unset, so +an empty shell expansion gets the closed door rather than a secret of `" "`. + +`attested` deliberately requires the projected-file tier, not +`TinyhumansTokenSource::from_env` as a whole: that resolver also accepts a +long-lived `TINYHUMANS_API_KEY`, which a self-hoster commonly sets to buy +inference. Accepting it here would tell such an operator their working Connect +button is platform-managed and take it away. Both the REST route and the GraphQL +`Company.connections` resolver project the field through the same +`connect_route_from_env`, so the two read shapes cannot drift. + +**Provider mapping to the platform backend.** Its registered OAuth providers are +`notion`, `google`, `gmail`, `github`, `twitter`, `discord` and `instagram`. Two +consequences for the console catalog: `gmail` is a registered provider *name* but +not a separate provider application — it is Google's app requested with the Gmail +skill scopes, so a Gmail connect and a Google connect share one grant (which is +why the backend merges scopes incrementally rather than replacing them). And +there is **no Slack provider** at all (the backend's only Slack credential is an +internal alerting bot), so Slack has no hosted route except Composio, which runs +its own OAuth. + +## One connection status, for one console list (issue #582) + +`GET …/connections` is the **only** answer to "what is connected". It reconciles +the native `oauth/{provider}` catalog with a live Composio probe into one row per +provider, marking which namespaces reported it in `via` — and the console renders +exactly one provider grid from it (`frontend/src/lib/provider-grid.ts`). + +That took removing a gate. `composio_view` used to discard the Composio half +unless the company explicitly granted the `composio` tool namespace, while +`GET …/composio/connections` — which the console's *other* provider list read — +never consulted the grant. The two lists therefore disagreed by construction, not +by timing: 13 of the 21 shipped companies grant no `composio`, so for most of +them one screen said both "connected" and "not connected" about the same account, +and the second list's Connect button was actionable. + +The grant governs whether **agents receive Composio tools**. It never governed +whether a handshake completes — `resolve_tenant` reads the credential and the +toolkit allowlist and nothing else, so the sign-in the gate hid worked perfectly +well from the surface that ignored it. It is now reported (`granted` on +`GET …/composio`) and stated as a caveat next to the connected badge, rather than +silently deciding what the page may show. + +Two consequences worth knowing: + +- The probe now runs for any company holding a credential, not only granting + ones. It stays bounded by `COMPOSIO_PROBE_TIMEOUT` and still degrades to + `unverified` rather than to a confident "not connected". +- `reconcile` is split from `project_connections` purely as a test seam: the + probe is a network call with no injection point, and the removed gate lived on + the far side of it, which is how it survived unasserted. + +## Releasing a connection: two routes, not interchangeable (issue #404) + +There are two disconnects and they act on different things: + +- `POST …/connections/{provider}/disconnect` blanks this host's own + `oauth/{provider}` secret and best-effort revokes it upstream. It has never + touched Composio. +- `DELETE …/composio/connections/{connection_id}` revokes **one connected + account** at Composio. Addressed by connection id, because a company can hold + two accounts for one toolkit and exactly one of them is being released. + +Until the second route existed the console sent every Disconnect to the first, +so a Composio-connected provider answered `200`, reported success, and was still +connected on the next refresh. The console now routes by what the provider is +actually connected through (`disconnectRouteFor` in +`frontend/src/lib/provider-grid.ts`), preferring the Composio account when a +provider is connected through both — the native secret is inert until #396 +lands, so blanking it would release nothing an agent can feel. + +`GET …/composio/connections` carries the accounts a revoke is addressed to: +`accounts[]` per toolkit, each with Composio's verbatim `status`, its +`createdAt` when Composio published one, and the account label when the provider +did. `toolkit` and `connected` keep their previous meaning, so the callers that +read only those two are untouched. + +**Which account an agent acts as is not decided here.** `composio_execute` posts +`{tool, arguments}` and no connection id (`src/harness/composio.rs`), so +Composio resolves it for the entity — nothing in this codebase selects, orders +or defaults an account, and the console's provider detail view says so rather +than marking one as the default. Changing that means sending a connection id on +execute, which is a harness change, not a console one. + diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index f60f79f98..d7cc868db 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -742,7 +742,11 @@ export interface InboxMessageDto { * does **not** route through the company key today, so this value does not * appear on a native-only provider — see `api/credential.ts`. * - `static` — a token this company already stored, or this host's own - * registered provider application (the self-hosted hatch). Connect works. + * registered provider application (the self-hosted hatch). The handshake + * works; the console stopped offering it in issue #822, because what it + * stores is read by no agent tool (#396). So this tier now says what the host + * *could* do, and `connectRoute` (`lib/connections.ts`) routes such a + * provider through Composio or reports it unavailable. * - `none` — neither, so no Connect can succeed on this host. */ export type ConnectionCredentialSource = "attested" | "company" | "static" | "none";