diff --git a/frontend/src/api/composio.ts b/frontend/src/api/composio.ts index 9abf543df..b08663d9a 100644 --- a/frontend/src/api/composio.ts +++ b/frontend/src/api/composio.ts @@ -24,6 +24,35 @@ import type { OpenCompanyClient } from "./client"; */ export type ComposioCredentialSource = "attested" | "static" | "none"; +/** + * One provider in the catalog the host offers, with the backend's own display + * metadata (issue #600). + * + * Every field but `slug` is best-effort and may be empty — a manifest + * allowlist, a degraded fallback, and a backend predating Composio's dynamic + * catalog all yield slug-only entries. The console fills those in from + * `@/lib/composio-catalog`, which is why that local typography table survives + * rather than being deleted in favour of the backend's names. + */ +export interface ComposioToolkitEntry { + /** Toolkit slug, e.g. `googlecalendar`. The key every host call is made with. */ + slug: string; + /** Human-readable name, e.g. `Google Calendar`. Empty when unpublished. */ + name: string; + /** One-line description. Empty when unpublished. Searched alongside the name. */ + description: string; + /** Composio-hosted logo URL, or `null` when unpublished. */ + logo: string | null; + /** + * Composio's own free-form category names, e.g. `["productivity", "email"]`. + * + * Forwarded verbatim by the host and bucketed here by substring — which is + * what means a Composio integration added tomorrow lands in the right group + * with no code change on either side of the wire. + */ + categories: string[]; +} + /** The company's Composio status. Never carries the token. */ export interface ComposioStatus { /** Whether the `composio` feature is compiled into this build at all. */ @@ -54,6 +83,20 @@ export interface ComposioStatus { * "other provider" field is for. */ effectiveToolkits: string[]; + /** + * The same providers as {@link effectiveToolkits}, in the same order, each + * carrying whatever display metadata the backend published for it (issue + * #600). + * + * This is what makes the panel browsable. Before it, the host reduced every + * catalog entry to a bare slug one layer before the console, so 123 providers + * could only be a flat list: there was nothing to group by, nothing to brand + * with, and nothing to search but the slug. + * + * Additive rather than a replacement — {@link effectiveToolkits} is still the + * slug contract, and it is still all an authorize call needs. + */ + effectiveCatalog: ComposioToolkitEntry[]; /** * Where {@link effectiveToolkits} came from (issue #397). * diff --git a/frontend/src/lib/composio-catalog.ts b/frontend/src/lib/composio-catalog.ts index 4f5f568df..13880c5f1 100644 --- a/frontend/src/lib/composio-catalog.ts +++ b/frontend/src/lib/composio-catalog.ts @@ -1,26 +1,42 @@ -// Turning the host's Composio status into the provider rows the console renders -// (issue #397). +// Turning the host's Composio status into the provider tiles the console renders +// (issues #397, #556, #600). // -// The host now answers open mode with the backend's live catalog — roughly a -// hundred toolkits — rather than a hardcoded eight. A hundred rows is its own -// usability problem, and dumping them unsorted and unfiltered would trade one -// broken page for another. These helpers own the two decisions that fixes: +// ## What changed, and why the shape of this file changed with it // -// 1. **Order**, so the answer to "where is Gmail" is "at the top", not "row -// thirty-one". Connected providers first (an operator scanning this panel is -// usually checking what is live), then the handful everyone reaches for, -// then the long tail alphabetically. -// 2. **Reach**, so the tail is discoverable. The list collapses to a preview -// with an explicit "show all N" and a search box that matches on both slug -// and display name. Before this, reaching a provider outside the eight meant -// knowing its Composio slug and typing it into a free-text field — which is -// a fine escape hatch and a terrible discovery mechanism. +// #397 taught the host to serve the backend's live catalog instead of a +// hardcoded eight, and #556 un-capped it — so this surface went from 8 providers +// to 123. It kept rendering them as one flat vertical list, twelve at a time +// behind a "show all" button, because a flat list was all the data allowed: the +// host reduced every catalog entry to a bare slug, and there is nothing to group +// 123 slugs by. +// +// #600 widened that wire (`effectiveCatalog`), so these helpers now own four +// decisions instead of two: +// +// 1. **Category**, so 123 providers are eight browsable buckets. Derived from +// Composio's OWN `categories[]` strings by substring, never from a list +// maintained here — which is the whole point: a Composio integration added +// tomorrow lands in the right bucket with no code change. A slug-only entry +// (manifest list, degraded fallback, backend predating the dynamic catalog) +// falls through to a slug/name keyword heuristic rather than vanishing. +// 2. **Order**, so the answer to "where is Gmail" is "at the top", not "row +// thirty-one". Connected first — an operator scanning this panel is usually +// checking what is live — then the handful everyone reaches for, then the +// tail alphabetically. +// 3. **Reach**, so the tail is discoverable: search over slug, display name AND +// description, composed with the category filter rather than replacing it. +// Someone who does not know a provider's name can still find it by what it +// does. +// 4. **Typography and branding**, so a tile reads as a product rather than a +// slug. The backend's name/description/logo win wherever it published them; +// the local tables below fill the gaps. // // Pure functions on purpose: `vitest.config.ts` scopes the unit runner to // helpers with no document and no host, and this is exactly that. What the -// component does with the rows belongs in `test/e2e`. +// component does with the rows — the grid, the chips, the images — belongs in +// `test/e2e`. -import type { ComposioStatus } from "@/api/composio"; +import type { ComposioStatus, ComposioToolkitEntry } from "@/api/composio"; /** Friendly display labels for the common toolkits; slug-cased fallback otherwise. */ const TOOLKIT_LABELS: Record = { @@ -57,23 +73,245 @@ const TOOLKIT_LABELS: Record = { }; /** - * A display label for a toolkit slug. + * Title-case a slug the label table does not cover: `capsule_crm` → `Capsule + * Crm`, `digital-ocean` → `Digital Ocean`. + * + * Splitting on `_` and `-` matters more than it looks. Composio slugs are + * routinely compound, and the previous rendering — upper-case the first letter + * and stop — turned `capsule_crm` into `Capsule_crm`, which reads as a database + * column rather than a product. + */ +function prettifySlug(slug: string): string { + return slug + .split(/[_-]+/) + .filter(Boolean) + .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) + .join(" "); +} + +/** + * A display label for a toolkit slug, with no catalog entry to hand. * * The table is a nicety, not a catalog — it exists so `googlecalendar` reads as * "Google Calendar" rather than "Googlecalendar". A slug missing from it is * title-cased and still renders; nothing here can hide a provider the host sent. * That is the distinction #397 turns on: the *list* comes from the backend, and * only its *typography* is local. + * + * Kept as a slug-only entry point because the sign-in-by-slug path has nothing + * else — the operator typed a slug the catalog does not carry. Everywhere a + * catalog entry exists, {@link providerLabel} prefers the backend's own name. */ export function toolkitLabel(slug: string): string { const key = slug.trim().toLowerCase(); - return TOOLKIT_LABELS[key] ?? (key ? key.charAt(0).toUpperCase() + key.slice(1) : slug); + return TOOLKIT_LABELS[key] ?? (key ? prettifySlug(key) : slug); +} + +/** + * The display label for a catalog entry: the backend's published name when it + * has one, else local typography. + * + * The backend wins deliberately. It is describing the provider it will actually + * connect you to, and it learns about a rename before this file does. + */ +export function providerLabel(entry: ComposioToolkitEntry): string { + return entry.name.trim() || toolkitLabel(entry.slug); +} + +/** + * Composio's own logo CDN, keyed by slug. + * + * Used only when the entry published no `logo` — which is every slug-only + * entry, i.e. every manifest list and every degraded fallback. The URL is a + * guess by construction, so the tile that renders it must tolerate a 404 and + * fall back to a monogram rather than showing a broken image. + */ +export function composioLogoUrl(slug: string): string { + return `https://logos.composio.dev/api/${slug}`; +} + +/** The buckets the category chips offer, in the order they are shown. */ +export type ProviderCategory = + | "All" + | "Chat" + | "Productivity" + | "Platform" + | "Social" + | "Tools & Automation"; + +/** + * Chip order. Fixed rather than derived so the chips do not reshuffle when a + * company connects a provider or the backend adds one — a filter row that moves + * under the cursor is worse than a filter row in an imperfect order. + */ +export const PROVIDER_CATEGORY_ORDER: readonly ProviderCategory[] = [ + "All", + "Chat", + "Productivity", + "Platform", + "Social", + "Tools & Automation", +]; + +/** + * Map Composio's catalog category strings onto the fixed buckets above. + * + * Composio's category names are free-form (`"productivity"`, `"crm"`, + * `"developer-tools"`, `"project management"`), so this matches on substrings + * and returns the first hit. `null` when nothing matches, so the caller can fall + * through to the slug/name heuristic. + * + * **This is the piece that makes the feature maintainable**, and it is why the + * host forwards `categories[]` verbatim instead of bucketing server-side: 123 + * providers bucket themselves, and provider number 124 does too, with no edit + * here. A hand-maintained slug→category table would be stale the week it + * shipped — which is precisely the trap `TOOLKIT_LABELS` above is allowed to + * fall into only because its failure mode is cosmetic. + * + * ## This function has a twin. Edit both. + * + * The other copy is `mapComposioCategory` in + * `app/src/components/composio/toolkitMeta.tsx` in **tinyhumansai/openhuman**. + * The two drive the same Composio catalog off the same free-form strings, and + * nothing mechanical detects a divergence: edit one and both consoles keep + * looking correct in isolation while bucketing the same provider differently. + * + * There is no shared package to hoist this into, so the guard is social and + * deliberately cheap — this notice, the matching one on the OpenHuman side, and + * `mapComposioCategory keeps the buckets its OpenHuman twin produces` in + * `test/unit/composio-catalog.test.ts`, which pins the substring table + * case-by-case. That test is the diff a divergence has to survive: change the + * table without changing it and the suite says so. + */ +export function mapComposioCategory(categories: readonly string[]): ProviderCategory | null { + if (categories.length === 0) return null; + const haystack = categories.join(" ").toLowerCase(); + const has = (...needles: string[]) => needles.some((n) => haystack.includes(n)); + + if (has("chat", "messaging", "communication")) return "Chat"; + if (has("social", "marketing")) return "Social"; + if ( + has( + "productivity", + "document", + "calendar", + "scheduling", + "project management", + "project-management", + "note", + "task", + "storage", + "email", + ) + ) { + return "Productivity"; + } + if (has("crm", "developer", "devtool", "analytics", "payment", "finance", "database", "cloud")) { + return "Platform"; + } + return null; +} + +const CHAT_KEYWORDS = ["discord", "slack", "teams", "webex", "whatsapp", "dialpad", "lark", "feishu"]; +const SOCIAL_KEYWORDS = ["facebook", "instagram", "linkedin", "reddit", "youtube", "twitter", "x_"]; +const PRODUCTIVITY_KEYWORDS = [ + "gmail", + "calendar", + "drive", + "docs", + "doc", + "sheets", + "slides", + "tasks", + "todoist", + "trello", + "notion", + "box", + "dropbox", + "sharepoint", + "one_drive", + "onedrive", + "outlook", + "miro", + "mural", + "monday", + "clickup", + "linear", + "jira", + "confluence", + "asana", + "basecamp", + "wrike", + "cal", + "calendly", + "typeform", + "excel", + "figma", + "google", +]; +const PLATFORM_KEYWORDS = [ + "github", + "gitlab", + "bitbucket", + "digital_ocean", + "contentful", + "supabase", + "convex", + "prisma", + "sentry", + "stripe", + "salesforce", + "hubspot", + "quickbooks", + "zendesk", + "zoho", +]; + +/** + * Last-resort bucketing from the slug and name alone. + * + * Only reached when the backend published no categories — a manifest allowlist, + * a degraded fallback, or a backend predating the dynamic catalog. Those lists + * are short and weighted towards exactly these providers, so a keyword pass + * covers most of them; anything it misses lands in "Tools & Automation", which + * is a real bucket rather than a hole. + */ +function guessCategory(slug: string, name: string): ProviderCategory { + const key = `${slug} ${name}`.toLowerCase(); + if (CHAT_KEYWORDS.some((k) => key.includes(k))) return "Chat"; + if (SOCIAL_KEYWORDS.some((k) => key.includes(k))) return "Social"; + if (PRODUCTIVITY_KEYWORDS.some((k) => key.includes(k))) return "Productivity"; + if (PLATFORM_KEYWORDS.some((k) => key.includes(k))) return "Platform"; + return "Tools & Automation"; +} + +/** + * A short "what you are authorising" hint, derived from the bucket. + * + * Deliberately vague, and honest about it: Composio decides the real scopes at + * consent time and does not publish them here. A per-provider scope list would + * be a claim this console cannot back, so this says the shape of the access + * rather than pretending to enumerate it. + */ +export function permissionHint(category: ProviderCategory): string { + switch (category) { + case "Chat": + return "Messages, channels, and communication data"; + case "Social": + return "Posts, profiles, and social content"; + case "Productivity": + return "Docs, files, tasks, and workspace data"; + case "Platform": + return "Repos, records, tickets, and system data"; + default: + return "Connected account data"; + } } /** * The providers a company reaches for first, in the order they are surfaced. * - * Purely an ordering hint for the rendered list — it never adds a row, never + * Purely an ordering hint for the rendered tiles — it never adds a tile, never * removes one, and is not what the host offers. A curated slug absent from the * host's list is simply absent. */ @@ -88,15 +326,18 @@ export const CURATED_TOOLKITS: readonly string[] = [ "discord", ]; -/** How many rows show before the operator asks for the rest. */ -export const PREVIEW_COUNT = 12; - -/** One provider row as the section renders it. */ +/** One provider tile as the section renders it. */ export interface ProviderRow { /** The Composio toolkit slug — the key every host call is made with. */ slug: string; - /** What the operator reads. */ + /** What the operator reads. The backend's name when it published one. */ label: string; + /** One line about what the provider is for. Empty when nothing is known. */ + description: string; + /** Logo to render, published or derived. May 404 — the tile must cope. */ + logoUrl: string; + /** Which chip this tile lives under. */ + category: ProviderCategory; /** Whether the company has at least one active connection for it. */ connected: boolean; /** Whether it is one of the {@link CURATED_TOOLKITS}. */ @@ -104,32 +345,43 @@ export interface ProviderRow { } /** - * Build the ordered provider rows. + * Build the ordered provider tiles. * - * `effective` is the host's answer (manifest list, backend catalog, or flagged + * `catalog` is the host's answer (manifest list, backend catalog, or flagged * fallback — this function does not care which, and must not: re-deciding here * what the host already decided is how the two drifted apart in the first * place). `extra` carries slugs the operator connected through the free-text - * field this session, so they keep a row instead of vanishing. `connected` maps + * field this session, so they keep a tile instead of vanishing. `connected` maps * lowercased slug to connected state. * * Order: connected, then curated, then the rest alphabetically. Duplicates * collapse; blank slugs are dropped. */ export function buildProviderRows( - effective: readonly string[], + catalog: readonly ComposioToolkitEntry[], extra: readonly string[], connected: Readonly>, ): ProviderRow[] { const seen = new Set(); const rows: ProviderRow[] = []; - for (const raw of [...effective, ...extra]) { - const slug = raw.trim().toLowerCase(); + const entries: ComposioToolkitEntry[] = [ + ...catalog, + // A slug typed into the sign-in-by-slug field has no catalog entry by + // definition — it is the escape hatch for a provider the catalog omitted. + // It still gets a tile, rendered entirely from local metadata. + ...extra.map((slug) => ({ slug, name: "", description: "", logo: null, categories: [] })), + ]; + for (const entry of entries) { + const slug = entry.slug.trim().toLowerCase(); if (!slug || seen.has(slug)) continue; seen.add(slug); + const label = providerLabel({ ...entry, slug }); rows.push({ slug, - label: toolkitLabel(slug), + label, + description: entry.description.trim(), + logoUrl: entry.logo?.trim() || composioLogoUrl(slug), + category: mapComposioCategory(entry.categories) ?? guessCategory(slug, label), connected: connected[slug] === true, curated: CURATED_TOOLKITS.includes(slug), }); @@ -143,36 +395,72 @@ export function buildProviderRows( } /** - * Narrow rows to a search query, matched case-insensitively against both the - * slug and the display label. + * The categories actually present in `rows`, in {@link PROVIDER_CATEGORY_ORDER}, + * always led by `All`. * - * Both halves matter: an operator who knows the product types "google calendar" - * and an operator who knows Composio types `googlecalendar`, and neither should - * come up empty. + * Only offering buckets that have something in them is the difference between a + * filter and a set of traps: a chip that reliably yields an empty grid teaches + * the operator to distrust the whole row. A manifest list of two providers gets + * two chips plus `All`, not six. + */ +export function availableCategories(rows: readonly ProviderRow[]): ProviderCategory[] { + const present = new Set(rows.map((row) => row.category)); + return PROVIDER_CATEGORY_ORDER.filter((c) => c === "All" || present.has(c)); +} + +/** + * Narrow rows to a category. `All` is the identity. + * + * Composes with {@link filterProviderRows} rather than replacing it — the two + * are `AND`, so an operator can search inside a bucket. Search that silently + * cleared the chip would be the more common design and the more annoying one: + * it throws away half of what the operator already told us. + */ +export function filterByCategory( + rows: readonly ProviderRow[], + category: ProviderCategory, +): ProviderRow[] { + if (category === "All") return [...rows]; + return rows.filter((row) => row.category === category); +} + +/** + * Narrow rows to a search query, matched case-insensitively against the slug, + * the display label, and the description. + * + * All three matter. An operator who knows the product types "google calendar"; + * one who knows Composio types `googlecalendar`; and one who knows neither + * types "invoices" and should still reach Stripe. That third case is new with + * #600 — before it, there was no description on the wire to match against. */ export function filterProviderRows(rows: readonly ProviderRow[], query: string): ProviderRow[] { const q = query.trim().toLowerCase(); if (!q) return [...rows]; - return rows.filter((row) => row.slug.includes(q) || row.label.toLowerCase().includes(q)); + return rows.filter( + (row) => + row.slug.includes(q) || + row.label.toLowerCase().includes(q) || + row.description.toLowerCase().includes(q), + ); } /** - * The rows to actually render, and whether anything is being held back. + * The tiles to actually render: the category filter and the search composed, in + * that order. * - * A search shows every match — someone who typed a query wants the answer, not - * the first twelve answers. Without one, a long list collapses to - * {@link PREVIEW_COUNT} unless the operator has expanded it. + * There is no preview cut here any more, and that is the point of #600. The old + * list collapsed to twelve rows behind a "Show all 123 providers" button + * because 123 full-width rows were unreadable — the cut was a workaround for the + * layout, not a feature. A dense tile grid shows all of them at a glance, so the + * button that stood between the operator and the catalog is gone rather than + * relabelled. */ export function visibleProviderRows( rows: readonly ProviderRow[], + category: ProviderCategory, query: string, - expanded: boolean, -): { visible: ProviderRow[]; hidden: number } { - const matched = filterProviderRows(rows, query); - if (query.trim() || expanded || matched.length <= PREVIEW_COUNT) { - return { visible: matched, hidden: 0 }; - } - return { visible: matched.slice(0, PREVIEW_COUNT), hidden: matched.length - PREVIEW_COUNT }; +): ProviderRow[] { + return filterProviderRows(filterByCategory(rows, category), query); } /** @@ -189,7 +477,9 @@ export function visibleProviderRows( * seeing exactly what it chose, and telling it the list "may be incomplete" * would be false. */ -export function catalogWarning(status: Pick): string | null { +export function catalogWarning( + status: Pick, +): string | null { if (status.catalogSource !== "fallback") return null; return ( status.catalogNotice ?? diff --git a/frontend/src/views/connections/ComposioSection.tsx b/frontend/src/views/connections/ComposioSection.tsx index 3330ab5a0..016f22864 100644 --- a/frontend/src/views/connections/ComposioSection.tsx +++ b/frontend/src/views/connections/ComposioSection.tsx @@ -2,7 +2,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { AlertTriangle, Check, - ChevronDown, KeyRound, Loader2, LogIn, @@ -29,13 +28,55 @@ import { Card, CardContent } from "@/components/ui/card"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Skeleton } from "@/components/ui/skeleton"; +import { cn } from "@/lib/utils"; import { + availableCategories, buildProviderRows, catalogWarning, + permissionHint, toolkitLabel, visibleProviderRows, + type ProviderCategory, + type ProviderRow, } from "@/lib/composio-catalog"; +/** + * A provider's branded logo, degrading to a monogram. + * + * The URL is best-effort by construction: the backend publishes one for most + * catalog entries, and every other tile falls back to a slug-derived guess at + * Composio's logo CDN. A guess that 404s must look like a plain tile, never a + * broken-image glyph — so the error is caught and swapped rather than left to + * the browser. + */ +function ProviderLogo({ row }: { row: ProviderRow }) { + const [failed, setFailed] = useState(false); + // A company can repoint at a different backend, which re-keys the logo. Reset + // on URL change so one dead image does not poison the slot for good. + useEffect(() => setFailed(false), [row.logoUrl]); + + if (failed) { + return ( + + ); + } + return ( + setFailed(true)} + /> + ); +} + interface Props { client: OpenCompanyClient; company: string | null; @@ -98,8 +139,9 @@ export function ComposioSection({ client, company, canManage }: Props) { // catalog — roughly a hundred toolkits — so finding one by scrolling stopped // being reasonable (issue #397). const [query, setQuery] = useState(""); - // Whether the operator asked to see past the preview. - const [expanded, setExpanded] = useState(false); + // The selected category chip. Composed with the search rather than replacing + // it, so an operator can look for "invoices" inside Platform (issue #600). + const [category, setCategory] = useState("All"); const requestGeneration = useRef(0); const pollTimers = useRef>({}); @@ -156,7 +198,7 @@ export function ComposioSection({ client, company, canManage }: Props) { setOtherToolkit(""); setExtraToolkits([]); setQuery(""); - setExpanded(false); + setCategory("All"); setLoad("loading"); void refresh(); }, [refresh]); @@ -237,16 +279,32 @@ export function ComposioSection({ client, company, canManage }: Props) { } // Ordered once per status/connection change: connected first, then the - // handful everyone reaches for, then the tail alphabetically. Ordering and - // filtering live in `@/lib/composio-catalog` so they are testable without a - // document — see `vitest.config.ts` on where the line sits. + // handful everyone reaches for, then the tail alphabetically. Ordering, + // bucketing and filtering live in `@/lib/composio-catalog` so they are + // testable without a document — see `vitest.config.ts` on where the line sits. + // + // Built from `effectiveCatalog`, not `effectiveToolkits`: the slug list is + // still the wire contract every host call uses, but it is the catalog entries + // that carry the name, description, logo and categories this grid is made of + // (issue #600). const rows = useMemo( - () => buildProviderRows(status?.effectiveToolkits ?? [], extraToolkits, connected), - [status?.effectiveToolkits, extraToolkits, connected], + () => buildProviderRows(status?.effectiveCatalog ?? [], extraToolkits, connected), + [status?.effectiveCatalog, extraToolkits, connected], + ); + const categories = useMemo(() => availableCategories(rows), [rows]); + const visible = useMemo( + () => visibleProviderRows(rows, category, query), + [rows, category, query], ); - const { visible, hidden } = visibleProviderRows(rows, query, expanded); const degraded = status ? catalogWarning(status) : null; + // A chip can go away under the operator — a company narrows its manifest, or + // a refresh returns a shorter catalog. Falling back to All beats leaving them + // staring at an empty grid filtered by a bucket that no longer exists. + useEffect(() => { + if (!categories.includes(category)) setCategory("All"); + }, [categories, category]); + if (load === "unavailable") return null; const attested = status?.credentialSource === "attested"; @@ -336,7 +394,8 @@ export function ComposioSection({ client, company, canManage }: Props) { openMode && (

This company allows any provider Composio - offers — {rows.length} in total, most-used first. Search to narrow them. + offers — {rows.length} in total, connected first. Filter by category or + search by name, slug, or what a provider does.

) )} @@ -347,55 +406,158 @@ export function ComposioSection({ client, company, canManage }: Props) { aria-label="Search providers" autoComplete="off" className="pl-7" - placeholder={`Search ${rows.length} providers…`} + placeholder={`Search ${rows.length} providers by name or what they do…`} value={query} onChange={(e) => setQuery(e.target.value)} /> )} -
    + {categories.length > 2 && ( + // Chips only earn their row when there is more than one real + // bucket to choose between — `availableCategories` always + // includes "All", so two means one bucket, which is not a + // choice. A two-provider manifest list gets the grid without + // a filter that can only ever do nothing. +
    + {categories.map((c) => ( + + ))} +
    + )} + {/* + A dense tile grid, not rows (issue #600). 123 full-width rows + are unreadable at any scroll depth, which is why the old list + hid all but twelve behind a "show all" button — the cut was a + workaround for the layout. Compact branded tiles fit the whole + catalog on a screen or two, so the button is gone rather than + relabelled. + */} +
      {visible.map((row) => { const isSigningIn = signingIn === row.slug; - return ( -
    • - {row.label} - {row.connected ? ( - - connected + // The whole tile is the affordance. An 8.5rem tile has no + // room for a label AND a button, and a tile that looks + // clickable but is not would be worse than either. + const actionable = canManage && !row.connected; + // Named `state`, not `status` — `status` is the component's + // ComposioStatus, and shadowing it here would be a quiet + // trap for the next edit. + const state = row.connected + ? "connected" + : isSigningIn + ? "signing in" + : "not connected"; + const shell = cn( + "flex size-full flex-col items-start justify-between gap-1 rounded-lg border p-2.5 text-left", + row.connected + ? "border-emerald-500/30 bg-emerald-500/5" + : "border-border bg-card", + ); + const body = ( + <> +
      + + {row.connected ? ( + + ) : isSigningIn ? ( + + ) : actionable ? ( + + ) : null} +
      +
      + + {row.label} - ) : !canManage ? ( - not connected - ) : ( -
      + + ); + return ( +
    • + {actionable ? ( + + > + {body} + + ) : ( + // Connected, or a viewer who cannot manage: there is + // nothing to click. Rendered as a div rather than a + // disabled button so it stays in the reading order — + // "Gmail, connected" is exactly what a member opened + // this panel to learn, and a disabled button is + // unfocusable. +
      + {body} +
      )}
    • ); })}
    - {visible.length === 0 && query.trim() !== "" && ( + {visible.length === 0 && (

    - No provider matches “{query.trim()}”. Composio's slug may differ from the - product name — try connecting it by slug below. + {query.trim() !== "" ? ( + <> + No provider matches “{query.trim()}” + {category !== "All" && <> in {category}}. Composio's slug may differ + from the product name — try another category, or connect it by slug below. + + ) : ( + <>No provider in {category}. + )}

    )} - {hidden > 0 && ( - - )} {openMode && canManage && (
    diff --git a/frontend/test/unit/composio-catalog.test.ts b/frontend/test/unit/composio-catalog.test.ts index 4284267bb..c427a6f9c 100644 --- a/frontend/test/unit/composio-catalog.test.ts +++ b/frontend/test/unit/composio-catalog.test.ts @@ -1,18 +1,35 @@ import { describe, expect, it } from "vitest"; +import type { ComposioToolkitEntry } from "@/api/composio"; import { CURATED_TOOLKITS, - PREVIEW_COUNT, + availableCategories, buildProviderRows, catalogWarning, + filterByCategory, filterProviderRows, + mapComposioCategory, + permissionHint, + providerLabel, toolkitLabel, visibleProviderRows, } from "@/lib/composio-catalog"; -/** A hundred-slug catalog — the shape the backend actually returns. */ -function hundredSlugs(): string[] { - return Array.from({ length: 100 }, (_, i) => `provider${String(i).padStart(3, "0")}`); +/** A catalog entry, defaulting every field the backend may leave unpublished. */ +function entry(slug: string, over: Partial = {}): ComposioToolkitEntry { + return { slug, name: "", description: "", logo: null, categories: [], ...over }; +} + +/** Slug-only entries — a manifest list, a fallback, or a pre-catalog backend. */ +function slugs(list: readonly string[]): ComposioToolkitEntry[] { + return list.map((s) => entry(s)); +} + +/** A hundred-provider catalog — the shape the backend actually returns. */ +function hundredEntries(): ComposioToolkitEntry[] { + return Array.from({ length: 100 }, (_, i) => + entry(`provider${String(i).padStart(3, "0")}`, { categories: ["productivity"] }), + ); } describe("toolkitLabel", () => { @@ -29,20 +46,181 @@ describe("toolkitLabel", () => { expect(toolkitLabel("obscureprovider")).toBe("Obscureprovider"); expect(toolkitLabel(" MixedCase ")).toBe("Mixedcase"); }); + + it("splits a compound slug into words", () => { + // `Capsule_crm` reads as a database column, not a product. Composio slugs + // are routinely compound, so this is the common case, not the exotic one. + expect(toolkitLabel("capsule_crm")).toBe("Capsule Crm"); + expect(toolkitLabel("digital-ocean")).toBe("Digital Ocean"); + }); +}); + +describe("providerLabel", () => { + it("prefers the backend's published name over local typography", () => { + // The backend is describing the provider it will actually connect you to, + // and it learns about a rename before this repo does. + expect(providerLabel(entry("googlecalendar", { name: "Google Calendar (Workspace)" }))).toBe( + "Google Calendar (Workspace)", + ); + }); + + it("falls back to the local label when the backend published none", () => { + expect(providerLabel(entry("googlecalendar"))).toBe("Google Calendar"); + expect(providerLabel(entry("googlecalendar", { name: " " }))).toBe("Google Calendar"); + }); +}); + +describe("mapComposioCategory", () => { + it("buckets from Composio's own free-form category strings", () => { + // The point of the whole design: these strings come off the wire, so a + // provider Composio adds tomorrow buckets itself with no edit here. + expect(mapComposioCategory(["messaging"])).toBe("Chat"); + expect(mapComposioCategory(["developer-tools"])).toBe("Platform"); + expect(mapComposioCategory(["project management"])).toBe("Productivity"); + expect(mapComposioCategory(["marketing"])).toBe("Social"); + }); + + it("is case-insensitive and reads any of several categories", () => { + expect(mapComposioCategory(["Sales", "CRM"])).toBe("Platform"); + }); + + it("returns null when nothing matches, so the caller can guess", () => { + // Null rather than a default bucket: the caller has a slug/name heuristic + // that is strictly better than "Tools & Automation", and swallowing the + // miss here would hide it. + expect(mapComposioCategory(["quantum-widgets"])).toBeNull(); + expect(mapComposioCategory([])).toBeNull(); + }); + + it("keeps the buckets its OpenHuman twin produces", () => { + // The drift guard. This function is a second copy of + // `mapComposioCategory` in `app/src/components/composio/toolkitMeta.tsx` + // in tinyhumansai/openhuman, and nothing mechanical can compare the two + // across repositories — edit one and both consoles keep looking correct in + // isolation while bucketing the same provider differently. + // + // So the substring table is pinned case-by-case here. It is not testing + // that the code does what the code does: it is the diff a divergence has to + // survive. Anyone editing the table has to edit this list too, and that is + // the moment they are told a twin exists. + const table: Array<[string, string | null]> = [ + // Chat wins over everything, including a string that also reads social. + ["chat", "Chat"], + ["messaging", "Chat"], + ["communication", "Chat"], + // Social is tested BEFORE productivity, so `marketing` is Social even + // though a marketing tool is arguably productivity. + ["social", "Social"], + ["marketing", "Social"], + ["productivity", "Productivity"], + ["document", "Productivity"], + ["calendar", "Productivity"], + ["scheduling", "Productivity"], + ["project management", "Productivity"], + ["project-management", "Productivity"], + ["note", "Productivity"], + ["task", "Productivity"], + ["storage", "Productivity"], + ["email", "Productivity"], + ["crm", "Platform"], + ["developer", "Platform"], + ["devtool", "Platform"], + ["analytics", "Platform"], + ["payment", "Platform"], + ["finance", "Platform"], + ["database", "Platform"], + ["cloud", "Platform"], + // Not in the table on either side. + ["quantum-widgets", null], + ]; + for (const [category, expected] of table) { + expect(mapComposioCategory([category]), `category ${category}`).toBe(expected); + } + }); + + it("orders its buckets so the first hit wins, not the last", () => { + // Both copies test Chat, then Social, then Productivity, then Platform, + // and return on the first hit. An entry carrying several categories + // therefore depends on that order — reordering the branches on one side + // only is the subtlest way the twins can drift. + expect(mapComposioCategory(["email", "messaging"])).toBe("Chat"); + expect(mapComposioCategory(["crm", "marketing"])).toBe("Social"); + expect(mapComposioCategory(["analytics", "calendar"])).toBe("Productivity"); + }); }); describe("buildProviderRows", () => { - it("renders every slug the host sent, whatever it is", () => { - const rows = buildProviderRows(hundredSlugs(), [], {}); - expect(rows).toHaveLength(100); + it("renders every provider the host sent, whatever it is", () => { + expect(buildProviderRows(hundredEntries(), [], {})).toHaveLength(100); }); - it("puts connected first, then the common ones, then the tail alphabetically", () => { - const rows = buildProviderRows( - ["zendesk", "gmail", "airtable", "github", "notion"], + it("carries the backend's display metadata onto the tile", () => { + // The regression test for #600 on the console side. Every field here was on + // the wire and thrown away by the host, which is why the panel could only + // ever be a flat list of slugs. + const [row] = buildProviderRows( + [ + entry("hubspot", { + name: "HubSpot", + description: "CRM and marketing automation.", + logo: "https://logos.composio.dev/api/hubspot", + categories: ["crm"], + }), + ], [], - { notion: true }, + {}, ); + expect(row).toMatchObject({ + slug: "hubspot", + label: "HubSpot", + description: "CRM and marketing automation.", + logoUrl: "https://logos.composio.dev/api/hubspot", + category: "Platform", + }); + }); + + it("derives a logo for an entry that published none", () => { + // Slug-only entries still get branded tiles. The URL is a guess, so the + // tile handles a 404 — but a guess that usually works beats a grid of grey + // squares. + expect(buildProviderRows(slugs(["notion"]), [], {})[0].logoUrl).toBe( + "https://logos.composio.dev/api/notion", + ); + }); + + it("guesses a category from the slug when the backend published none", () => { + // The fallback path — a manifest allowlist, a degraded fallback, or a + // backend predating the dynamic catalog. Without it every such tile would + // land in one bucket and the chips would be useless exactly where the + // catalog is thinnest. + const rows = buildProviderRows(slugs(["slack", "gmail", "github", "quantumwidgets"]), [], {}); + const by = Object.fromEntries(rows.map((r) => [r.slug, r.category])); + expect(by).toEqual({ + slack: "Chat", + gmail: "Productivity", + github: "Platform", + quantumwidgets: "Tools & Automation", + }); + }); + + it("never drops a provider whose category Composio has just invented", () => { + // The property worth pinning explicitly rather than inferring from the two + // tests above: Composio can add a category string tomorrow that neither + // `mapComposioCategory` nor the keyword heuristic knows, and that provider + // must still get a tile. "Tools & Automation" is the floor, and it is a + // real bucket that `availableCategories` will offer — not a hole a provider + // can fall through and silently leave the grid. + const rows = buildProviderRows([entry("newthing", { categories: ["quantum-widgets"] })], [], {}); + expect(rows.map((r) => r.slug)).toEqual(["newthing"]); + expect(rows[0].category).toBe("Tools & Automation"); + expect(availableCategories(rows)).toContain("Tools & Automation"); + expect(visibleProviderRows(rows, "All", "")).toHaveLength(1); + }); + + it("puts connected first, then the common ones, then the tail alphabetically", () => { + const rows = buildProviderRows(slugs(["zendesk", "gmail", "airtable", "github", "notion"]), [], { + notion: true, + }); expect(rows.map((r) => r.slug)).toEqual([ // connected first — an operator scanning the panel is checking what is live "notion", @@ -56,18 +234,44 @@ describe("buildProviderRows", () => { }); it("keeps session-connected extras and collapses duplicates and blanks", () => { - const rows = buildProviderRows(["gmail", "GMAIL", "", " "], ["hubspot", "gmail"], {}); + const rows = buildProviderRows(slugs(["gmail", "GMAIL", "", " "]), ["hubspot", "gmail"], {}); expect(rows.map((r) => r.slug)).toEqual(["gmail", "hubspot"]); }); it("marks connection state case-insensitively", () => { - const rows = buildProviderRows(["Gmail"], [], { gmail: true }); + const rows = buildProviderRows(slugs(["Gmail"]), [], { gmail: true }); expect(rows[0]).toMatchObject({ slug: "gmail", connected: true, curated: true }); }); }); +describe("availableCategories", () => { + it("offers only buckets that have something in them", () => { + // A chip that reliably yields an empty grid teaches the operator to + // distrust the whole filter row. + const rows = buildProviderRows(slugs(["slack", "gmail"]), [], {}); + expect(availableCategories(rows)).toEqual(["All", "Chat", "Productivity"]); + }); + + it("always leads with All, in a fixed order", () => { + // Fixed rather than derived: a filter row that reshuffles when a company + // connects a provider moves under the operator's cursor. + const rows = buildProviderRows(slugs(["github", "slack"]), [], {}); + expect(availableCategories(rows)).toEqual(["All", "Chat", "Platform"]); + }); +}); + describe("filterProviderRows", () => { - const rows = buildProviderRows(["googlecalendar", "gmail", "hubspot", "zendesk"], [], {}); + const rows = buildProviderRows( + [ + entry("googlecalendar"), + entry("gmail"), + entry("hubspot"), + entry("zendesk"), + entry("stripe", { name: "Stripe", description: "Payments, invoices and subscriptions." }), + ], + [], + {}, + ); it("matches on the product name an operator would type", () => { expect(filterProviderRows(rows, "google cal").map((r) => r.slug)).toEqual(["googlecalendar"]); @@ -79,6 +283,12 @@ describe("filterProviderRows", () => { expect(filterProviderRows(rows, "hubspot").map((r) => r.slug)).toEqual(["hubspot"]); }); + it("matches on the description — what a provider does, not what it is called", () => { + // New with #600: there was no description on the wire to search before. + // An operator who does not know Stripe by name can still reach it. + expect(filterProviderRows(rows, "invoices").map((r) => r.slug)).toEqual(["stripe"]); + }); + it("is case-insensitive and returns nothing for a genuine miss", () => { expect(filterProviderRows(rows, "ZENDESK").map((r) => r.slug)).toEqual(["zendesk"]); expect(filterProviderRows(rows, "nothing-like-this")).toEqual([]); @@ -89,30 +299,47 @@ describe("filterProviderRows", () => { }); }); -describe("visibleProviderRows", () => { - const rows = buildProviderRows(hundredSlugs(), [], {}); +describe("filterByCategory", () => { + const rows = buildProviderRows(slugs(["slack", "gmail", "github"]), [], {}); + + it("narrows to one bucket", () => { + expect(filterByCategory(rows, "Chat").map((r) => r.slug)).toEqual(["slack"]); + }); - it("collapses a hundred rows to a preview and reports what is held back", () => { - // The reason this exists: the host now serves the real catalog, and - // rendering a hundred unfiltered rows would trade one broken page for - // another. - const { visible, hidden } = visibleProviderRows(rows, "", false); - expect(visible).toHaveLength(PREVIEW_COUNT); - expect(hidden).toBe(100 - PREVIEW_COUNT); + it("All is the identity", () => { + expect(filterByCategory(rows, "All")).toHaveLength(rows.length); + }); +}); + +describe("visibleProviderRows", () => { + it("shows the whole catalog — there is no preview cut any more", () => { + // The heart of #600. The old helper collapsed to twelve rows behind a "Show + // all 123 providers" button; the cut was a workaround for a flat list being + // unreadable, not a feature. A grid shows all of them, so a test that + // asserted a preview would now be pinning the bug. + expect(visibleProviderRows(buildProviderRows(hundredEntries(), [], {}), "All", "")).toHaveLength( + 100, + ); }); - it("shows everything once the operator expands", () => { - const { visible, hidden } = visibleProviderRows(rows, "", true); - expect(visible).toHaveLength(100); - expect(hidden).toBe(0); + it("composes the category filter and the search with AND", () => { + // Search must not silently clear the chip: that throws away half of what + // the operator already told us. + const rows = buildProviderRows( + [entry("slack", { categories: ["messaging"] }), entry("gmail", { categories: ["email"] })], + [], + {}, + ); + expect(visibleProviderRows(rows, "Chat", "slack").map((r) => r.slug)).toEqual(["slack"]); + expect(visibleProviderRows(rows, "Chat", "gmail")).toEqual([]); }); - it("a search reaches past the preview — the tail must be discoverable", () => { - // provider0NN — 10 matches, all beyond the twelve-row preview for most of - // them. A search that only looked at the preview would make the long tail - // unreachable by name, leaving the slug field as the only route to it. - const { visible, hidden } = visibleProviderRows(rows, "provider09", false); - expect(visible.map((r) => r.slug)).toEqual([ + it("a search reaches the whole tail, not just the top of it", () => { + // provider0NN — 10 matches, all deep in the list. A search that only looked + // at a preview would make the long tail unreachable by name, leaving the + // slug field as the only route to it. + const rows = buildProviderRows(hundredEntries(), [], {}); + expect(visibleProviderRows(rows, "All", "provider09").map((r) => r.slug)).toEqual([ "provider090", "provider091", "provider092", @@ -124,14 +351,20 @@ describe("visibleProviderRows", () => { "provider098", "provider099", ]); - expect(hidden).toBe(0); }); - it("a short list is never collapsed", () => { - const short = buildProviderRows(CURATED_TOOLKITS, [], {}); - const { visible, hidden } = visibleProviderRows(short, "", false); - expect(visible).toHaveLength(short.length); - expect(hidden).toBe(0); + it("a short list renders whole", () => { + const short = buildProviderRows(slugs(CURATED_TOOLKITS), [], {}); + expect(visibleProviderRows(short, "All", "")).toHaveLength(short.length); + }); +}); + +describe("permissionHint", () => { + it("describes the shape of the access, not a scope list", () => { + // Composio decides the real scopes at consent time and does not publish + // them here. Enumerating them would be a claim this console cannot back. + expect(permissionHint("Chat")).toContain("communication"); + expect(permissionHint("Tools & Automation")).toBe("Connected account data"); }); }); diff --git a/src/company/composio.rs b/src/company/composio.rs index 73c75e8eb..ca3bf2b93 100644 --- a/src/company/composio.rs +++ b/src/company/composio.rs @@ -10,6 +10,8 @@ //! closed), never a borrowed identity. Only the backend URL may be overridden //! from the environment. +use serde::Serialize; + use crate::Result; use crate::ports::SecretStore; use crate::ports::types::{CompanyId, SecretValue}; @@ -76,6 +78,70 @@ pub async fn token_configured(company: &CompanyId, secrets: &dyn SecretStore) -> .unwrap_or(false)) } +/// One provider in the catalog the console renders, carrying the backend's own +/// display metadata rather than a bare slug (issue #600). +/// +/// ## Why this lives here and not in the harness +/// +/// It is produced by `harness::composio::list_catalog_toolkits` and consumed by +/// the always-compiled status route, and the harness compiles only under the +/// `openhuman` feature. Same reason [`TOKEN_KEY`] and +/// [`backend_url_or_default`] live here: the console plane must keep working in +/// a default build that links none of the live tools. +/// +/// ## Why it is not `composio_catalog::CatalogToolkit` +/// +/// That type describes the same backend entry for an *agent*, and it drops the +/// logo URL on purpose — a URL a model can never act on costs tokens to no end. +/// The logo and the categories are the entire point of this one: they are what +/// let 123 providers be a browsable grid instead of 123 stacked rows. It also +/// carries no connected flag, because the console learns that from +/// `GET …/composio/connections` — live per-company state, not catalog data. +/// +/// Serialized straight into the status DTO, so these field names are the +/// console's wire contract. +#[derive(Clone, Debug, Default, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct CatalogEntry { + /// Toolkit slug, e.g. `googlecalendar`. The key every host call is made + /// with, and the only field the backend always publishes. + pub slug: String, + /// Human-readable name, e.g. `Google Calendar`. Empty when the backend + /// published none — the console then falls back to its own typography. + pub name: String, + /// One-line description. Empty when unpublished. The console searches it + /// alongside the name, so an operator who knows what a provider *does* can + /// find it without knowing what it is called. + pub description: String, + /// Composio-hosted logo URL. `None` when unpublished. + pub logo: Option, + /// Composio's own category names, e.g. `["productivity", "email"]`. + /// + /// Forwarded **verbatim** and uninterpreted. The console buckets them by + /// substring, and it does so precisely because that means a Composio + /// integration added tomorrow lands in the right group with no code change + /// on either side of this wire. + pub categories: Vec, +} + +impl CatalogEntry { + /// An entry for a provider the backend published a slug and nothing else + /// for. + /// + /// Three real callers, so "no metadata" is a first-class state rather than + /// a reason to drop the provider: a manifest allowlist (hand-written slugs, + /// and the catalog is deliberately never consulted for it), the fallback + /// list (which exists *because* the metadata could not be fetched), and a + /// backend predating the dynamic catalog (which sends no `catalog[]` at + /// all). The console renders all three with its own typography. + pub fn from_slug(slug: impl Into) -> Self { + Self { + slug: slug.into(), + ..Self::default() + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/harness/composio.rs b/src/harness/composio.rs index cd0a3cec4..393e4d136 100644 --- a/src/harness/composio.rs +++ b/src/harness/composio.rs @@ -228,6 +228,7 @@ mod live { use async_trait::async_trait; use serde_json::{Value, json}; + use crate::company::composio::CatalogEntry; use crate::harness::composio_catalog as catalog; use crate::harness::mcp_probe::{redact, scrub}; use crate::metering::record_oauth_call; @@ -454,7 +455,23 @@ mod live { /// all; their plain slug allowlist is used instead. Slugs are trimmed, /// lowercased, de-duplicated and sorted for a stable render order. Any /// upstream error is scrubbed of the tenant bearer before it bubbles. - pub async fn list_catalog_toolkits(config: &TenantComposio) -> Result> { + /// + /// ## Why this returns entries rather than slugs (issue #600) + /// + /// It used to return `Vec`, and that one `.map(|e| e.slug)` was the + /// whole of #600. The backend publishes `name`, `logo`, `description` and + /// `categories` on every entry and states plainly that it assembles them so + /// the frontend can read them straight from there — and this function threw + /// five of the six fields away one layer before the console, which then had + /// nothing to group by, nothing to brand with, and nothing to search but the + /// slug. A hundred-and-twenty-three-item flat list was the honest rendering + /// of what it was handed. + /// + /// Nothing about *admission* changed. The agent-side gate is + /// [`toolkit_allowed`], which takes slugs and never consulted this function; + /// the console's slug list is still derived from these entries. This widens + /// what is *described*, not what is permitted. + pub async fn list_catalog_toolkits(config: &TenantComposio) -> Result> { tracing::debug!("[composio] ops list_catalog_toolkits"); let (client, secrets) = live_call(config).await?; let resp = match client.list_toolkits().await { @@ -462,22 +479,59 @@ mod live { Err(err) => return Err(anyhow::anyhow!(scrub(&format!("{err}"), &secrets))), }; let normalize = |slug: &str| slug.trim().to_ascii_lowercase(); - let mut slugs: std::collections::BTreeSet = resp - .catalog - .iter() - .filter(|entry| entry.enabled.unwrap_or(false)) - .map(|entry| normalize(&entry.slug)) - .filter(|slug| !slug.is_empty()) - .collect(); - if slugs.is_empty() { - slugs = resp + // A `BTreeMap` keyed on the normalized slug keeps the de-duplication and + // the stable sort the slug set gave us, while carrying the metadata that + // is the entire point of the widening. + // + // `or_insert_with`, NOT `collect()` into the map: collecting keeps the + // LAST value for a repeated key, and a duplicate catalog entry is + // typically the degenerate one — the mock's `Gmail (dup)` carries no + // description, and collecting would let it silently blank the real + // Gmail's. First entry wins, which is also what the `BTreeSet` + // this replaced effectively did. + let mut entries: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + for entry in resp.catalog.iter().filter(|e| e.enabled.unwrap_or(false)) { + let slug = normalize(&entry.slug); + if slug.is_empty() { + continue; + } + entries.entry(slug.clone()).or_insert_with(|| CatalogEntry { + slug, + name: entry.name.trim().to_string(), + description: entry + .description + .as_deref() + .map(str::trim) + .unwrap_or_default() + .to_string(), + logo: entry + .logo + .as_deref() + .map(str::trim) + .filter(|logo| !logo.is_empty()) + .map(str::to_string), + categories: entry + .categories + .iter() + .map(|c| c.trim().to_string()) + .filter(|c| !c.is_empty()) + .collect(), + }); + } + if entries.is_empty() { + // A backend predating the dynamic catalog. Slugs are all it has, so + // slugs are all the console gets — rendered with local typography + // rather than dropped. + entries = resp .toolkits .iter() .map(|slug| normalize(slug)) .filter(|slug| !slug.is_empty()) + .map(|slug| (slug.clone(), CatalogEntry::from_slug(slug))) .collect(); } - Ok(slugs.into_iter().collect()) + Ok(entries.into_values().collect()) } // ── composio_list_toolkits ────────────────────────────────────────── @@ -1256,6 +1310,8 @@ mod ops_helper_tests { use std::net::SocketAddr; + use crate::company::composio::CatalogEntry; + use axum::Router; use axum::routing::{get, post}; use serde_json::{Value, json}; @@ -1289,14 +1345,32 @@ mod ops_helper_tests { /// entries carry an `enabled` gate. `zendesk` is present but not connectable /// and must not be advertised; the casing and whitespace on `HubSpot` must /// normalise. + /// + /// Entries carry the display metadata (`logo`, `description`, `categories`) + /// the backend actually publishes — issue #600 is that all of it was dropped + /// on the way through, so a mock that omitted it could not have caught the + /// bug. async fn toolkits_handler() -> axum::Json { axum::Json(json!({ "success": true, "data": { "toolkits": ["gmail", "slack"], "catalog": [ - { "slug": " HubSpot ", "name": "HubSpot", "enabled": true }, - { "slug": "gmail", "name": "Gmail", "enabled": true }, + { + "slug": " HubSpot ", + "name": "HubSpot", + "enabled": true, + "logo": " https://logos.composio.dev/api/hubspot ", + "description": " CRM and marketing automation. ", + "categories": ["crm", " marketing ", ""] + }, + { + "slug": "gmail", + "name": "Gmail", + "enabled": true, + "description": "Send and read email.", + "categories": ["email"] + }, { "slug": "zendesk", "name": "Zendesk", "enabled": false }, { "slug": "gmail", "name": "Gmail (dup)", "enabled": true } ] @@ -1389,12 +1463,61 @@ mod ops_helper_tests { .await .expect("catalog fetch"); assert_eq!( - catalog, - vec!["gmail".to_string(), "hubspot".to_string()], + catalog.iter().map(|e| e.slug.as_str()).collect::>(), + vec!["gmail", "hubspot"], "connectable entries only, normalised, de-duplicated and sorted" ); } + /// Issue #600: the display metadata the backend publishes reaches the + /// caller instead of being reduced to a slug. + /// + /// This is the regression test for the defect itself. Every field asserted + /// here was present in the response and discarded by a single + /// `.map(|entry| entry.slug)`, which is why the console had nothing to + /// group by, nothing to brand with, and nothing to search but the slug. + #[tokio::test] + async fn list_catalog_toolkits_carries_the_display_metadata() { + let url = spawn_backend().await; + let catalog = list_catalog_toolkits(&config(&url, Vec::new())) + .await + .expect("catalog fetch"); + + let hubspot = catalog + .iter() + .find(|e| e.slug == "hubspot") + .expect("hubspot is connectable"); + assert_eq!(hubspot.name, "HubSpot"); + assert_eq!(hubspot.description, "CRM and marketing automation."); + assert_eq!( + hubspot.logo.as_deref(), + Some("https://logos.composio.dev/api/hubspot"), + "the logo URL is what lets a tile be branded rather than a text row" + ); + assert_eq!( + hubspot.categories, + vec!["crm".to_string(), "marketing".to_string()], + "categories are trimmed and emptied-out entries dropped, but otherwise \ + forwarded verbatim — the console buckets them, not this layer" + ); + + let gmail = catalog + .iter() + .find(|e| e.slug == "gmail") + .expect("gmail is connectable"); + assert_eq!(gmail.description, "Send and read email."); + assert_eq!( + gmail.logo, None, + "an unpublished logo is None, not an empty string the console would \ + render as a broken image" + ); + assert_eq!( + gmail.name, "Gmail", + "the FIRST entry for a slug wins, matching the de-duplication the slug \ + set used to do — not the later `Gmail (dup)`" + ); + } + /// A backend predating the dynamic catalog sends no `catalog[]`. Its plain /// slug allowlist is used rather than reporting an empty catalog — which the /// console would (correctly) render as a degraded fallback. @@ -1404,7 +1527,15 @@ mod ops_helper_tests { let catalog = list_catalog_toolkits(&config(&url, Vec::new())) .await .expect("catalog fetch"); - assert_eq!(catalog, vec!["gmail".to_string(), "notion".to_string()]); + assert_eq!( + catalog, + vec![ + CatalogEntry::from_slug("gmail"), + CatalogEntry::from_slug("notion"), + ], + "slug-only entries: the backend published nothing else, and the console \ + renders these with its own typography rather than dropping them" + ); } /// An unreachable backend is an error, never a quietly-empty catalog — the diff --git a/src/server/ops/composio.rs b/src/server/ops/composio.rs index 0a263c47e..e98d62ba1 100644 --- a/src/server/ops/composio.rs +++ b/src/server/ops/composio.rs @@ -66,7 +66,9 @@ use axum::routing::{get, post, put}; use serde::{Deserialize, Serialize}; use crate::AppState; -use crate::company::composio::{backend_url_or_default, store_token, token_configured}; +use crate::company::composio::{ + CatalogEntry, backend_url_or_default, store_token, token_configured, +}; use crate::company::credentials::{CredentialSource, TinyhumansTokenSource}; use crate::company::runtime::CompanyRuntime; use crate::ports::types::CompanyEvent; @@ -112,7 +114,12 @@ async fn effective_toolkits( ( false, OpenModeToolkits { - toolkits: manifest.to_vec(), + // A manifest allowlist is slugs and nothing else — the company + // wrote it by hand, and the catalog is deliberately not + // consulted here (it must not be able to widen a list that + // narrowed on purpose). So these entries carry no metadata, and + // the console falls back to its own typography for them. + toolkits: manifest.iter().map(CatalogEntry::from_slug).collect(), source: CatalogSource::Manifest, notice: None, }, @@ -155,7 +162,7 @@ fn catalog_cache_key(runtime: &CompanyRuntime) -> String { /// `Err` is a plain-language reason the console can show — never a bare /// fall-through to a short list that would look authoritative. #[cfg(feature = "composio")] -async fn fetch_catalog(runtime: &CompanyRuntime) -> Result, String> { +async fn fetch_catalog(runtime: &CompanyRuntime) -> Result, String> { // No credential of any tier means there is nothing to dial the backend // with. Say that, rather than spending the timeout to discover it. let config = resolve_tenant(runtime).await.map_err(|_| { @@ -179,7 +186,7 @@ async fn fetch_catalog(runtime: &CompanyRuntime) -> Result, String> /// The status route still answers (reporting `inBuild:false`), and it says so /// rather than presenting the fallback as the backend's list. #[cfg(not(feature = "composio"))] -async fn fetch_catalog(_runtime: &CompanyRuntime) -> Result, String> { +async fn fetch_catalog(_runtime: &CompanyRuntime) -> Result, String> { Err("Composio is not compiled into this build".to_string()) } @@ -240,6 +247,26 @@ struct ComposioStatusDto { /// mode this is still not a hard limit: any slug the backend permits can be /// authorized by typing it. effective_toolkits: Vec, + /// The same providers as [`Self::effective_toolkits`], in the same order, + /// carrying whatever display metadata the backend published for each — + /// name, description, logo URL, and Composio's own category names (issue + /// #600). + /// + /// **Additive, and deliberately so.** The slug list above is the contract + /// every existing consumer reads and the only thing an authorize call + /// needs; this is the render model beside it. Replacing the slug list with + /// this one would have bought nothing and broken that contract. + /// + /// Empty metadata is a real state, not a bug: a manifest allowlist, a + /// fallback list, and a backend predating the dynamic catalog all yield + /// slug-only entries, and the console renders those with its own + /// typography. + /// + /// The categories are forwarded **verbatim**, uninterpreted. The console + /// buckets them by substring, which is what lets a Composio integration + /// added tomorrow land in the right group with no change on either side of + /// this wire. + effective_catalog: Vec, /// Where [`Self::effective_toolkits`] came from — `manifest`, `backend`, or /// `fallback` (issue #397). /// @@ -349,7 +376,8 @@ async fn effective_status(runtime: &CompanyRuntime) -> Result Result Vec { + (0..100) + .map(|i| CatalogEntry { + slug: format!("provider{i:03}"), + name: format!("Provider {i:03}"), + description: format!("Does provider-{i:03} things."), + logo: Some(format!("https://logos.example.test/provider{i:03}")), + categories: vec!["productivity".to_string()], + }) + .collect() + } + + /// Just the slugs of [`hundred_entries`], for asserting on the slug list + /// the wire has always carried. fn hundred_slugs() -> Vec { - (0..100).map(|i| format!("provider{i:03}")).collect() + hundred_entries().into_iter().map(|e| e.slug).collect() } /// The heart of the reopened issue: in open mode the console is offered the @@ -778,7 +823,7 @@ mod tests { "[company]\nname = \"Catalog Co\"\n[policy]\nmode = \"full\"\n[tools]\nallow = [\"composio\"]\n", ) .await; - let catalog = hundred_slugs(); + let catalog = hundred_entries(); composio_toolkits::cache().store( &super::catalog_cache_key(runtime_of(&state, "catalogco").as_ref()), Ok(catalog.clone()), @@ -800,7 +845,7 @@ mod tests { ); assert_eq!( dto["effectiveToolkits"], - json!(catalog), + json!(hundred_slugs()), "open mode must serve what the backend permits, verbatim" ); assert!( @@ -829,7 +874,7 @@ mod tests { .await; composio_toolkits::cache().store( &super::catalog_cache_key(runtime_of(&state, "narrowco").as_ref()), - Ok(hundred_slugs()), + Ok(hundred_entries()), std::time::Instant::now(), ); @@ -909,7 +954,7 @@ mod tests { ) .await; let key = super::catalog_cache_key(runtime_of(&state, "rotateco").as_ref()); - composio_toolkits::cache().store(&key, Ok(hundred_slugs()), std::time::Instant::now()); + composio_toolkits::cache().store(&key, Ok(hundred_entries()), std::time::Instant::now()); let (status, resp, raw) = send_for( &state, @@ -1063,6 +1108,7 @@ mod tests { toolkits: vec!["gmail".to_string()], open_mode: false, effective_toolkits: vec!["gmail".to_string()], + effective_catalog: vec![CatalogEntry::from_slug("gmail")], catalog_source: CatalogSource::Manifest, catalog_notice: None, }; @@ -1080,6 +1126,7 @@ mod tests { "toolkits", "openMode", "effectiveToolkits", + "effectiveCatalog", "catalogSource", "catalogNotice" ], diff --git a/src/server/ops/composio_toolkits.rs b/src/server/ops/composio_toolkits.rs index 6ee118f93..191a97a59 100644 --- a/src/server/ops/composio_toolkits.rs +++ b/src/server/ops/composio_toolkits.rs @@ -41,6 +41,8 @@ use std::time::{Duration, Instant}; use serde::Serialize; +use crate::company::composio::CatalogEntry; + /// The last-resort provider list, used only when the real catalog cannot be /// fetched — and always accompanied by [`CatalogSource::Fallback`] plus a /// notice, so it is never mistaken for the backend's answer. @@ -114,10 +116,13 @@ pub(crate) enum CatalogSource { } /// The toolkits open mode offers, and how honest the answer is. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq)] pub(crate) struct OpenModeToolkits { - /// The slugs to offer as provider rows. - pub toolkits: Vec, + /// The providers to offer, with whatever display metadata the backend + /// published for each (issue #600). A manifest or fallback list carries + /// slugs only; a fetched catalog carries names, logos, descriptions and + /// categories too. + pub toolkits: Vec, /// Where they came from. pub source: CatalogSource, /// Plain-language reason the list is a fallback, for the console to show the @@ -128,7 +133,7 @@ pub(crate) struct OpenModeToolkits { impl OpenModeToolkits { /// The good case: the backend answered. - pub(crate) fn from_backend(toolkits: Vec) -> Self { + pub(crate) fn from_backend(toolkits: Vec) -> Self { Self { toolkits, source: CatalogSource::Backend, @@ -138,9 +143,17 @@ impl OpenModeToolkits { /// The honest degradation: the built-in list, marked as such, with the /// reason attached. + /// + /// Slug-only entries, and unavoidably so: a fallback exists precisely + /// because the metadata could not be fetched. The console renders these + /// with its own typography, which is why that fallback survives #600 rather + /// than being deleted in favour of the backend's names. pub(crate) fn degraded(reason: &str) -> Self { Self { - toolkits: FALLBACK_TOOLKITS.iter().map(|s| (*s).to_string()).collect(), + toolkits: FALLBACK_TOOLKITS + .iter() + .map(|s| CatalogEntry::from_slug(*s)) + .collect(), source: CatalogSource::Fallback, notice: Some(format!( "Composio's provider catalog could not be fetched ({}), so this is a built-in \ @@ -152,12 +165,21 @@ impl OpenModeToolkits { } /// Turn a cached-or-fresh fetch outcome into the rendered answer. - pub(crate) fn from_outcome(outcome: Result, String>) -> Self { + pub(crate) fn from_outcome(outcome: Result, String>) -> Self { match outcome { Ok(toolkits) => Self::from_backend(toolkits), Err(reason) => Self::degraded(&reason), } } + + /// Just the slugs, in render order — the wire field the console has always + /// had and every existing consumer still reads. + /// + /// Kept as a derived view rather than a second stored list so the two can + /// never disagree about which providers are on offer. + pub(crate) fn slugs(&self) -> Vec { + self.toolkits.iter().map(|t| t.slug.clone()).collect() + } } /// Keep an upstream reason to one readable clause. @@ -181,7 +203,7 @@ fn bound_reason(reason: &str) -> String { /// One cached fetch outcome and when it was recorded. struct CacheEntry { at: Instant, - outcome: Result, String>, + outcome: Result, String>, } impl CacheEntry { @@ -212,14 +234,18 @@ impl CatalogCache { /// The cached outcome for `key`, if one was recorded within its TTL of /// `now`. An expired entry reads as a miss and is left for the next /// [`Self::store`] to overwrite. - pub(crate) fn lookup(&self, key: &str, now: Instant) -> Option, String>> { + pub(crate) fn lookup( + &self, + key: &str, + now: Instant, + ) -> Option, String>> { let entries = self.entries.lock().ok()?; let entry = entries.get(key)?; (now.saturating_duration_since(entry.at) < entry.ttl()).then(|| entry.outcome.clone()) } /// Record an outcome as observed at `at`. - pub(crate) fn store(&self, key: &str, outcome: Result, String>, at: Instant) { + pub(crate) fn store(&self, key: &str, outcome: Result, String>, at: Instant) { if let Ok(mut entries) = self.entries.lock() { entries.insert(key.to_string(), CacheEntry { at, outcome }); } @@ -255,8 +281,10 @@ pub(crate) fn cache_key(company: &crate::ports::types::CompanyId, backend_url: & mod tests { use super::*; - fn slugs(list: &[&str]) -> Vec { - list.iter().map(|s| (*s).to_string()).collect() + /// Slug-only catalog entries — what a manifest list, a fallback list, or a + /// backend predating the dynamic catalog yields. + fn slugs(list: &[&str]) -> Vec { + list.iter().map(|s| CatalogEntry::from_slug(*s)).collect() } /// A fetched catalog is served as the backend's answer, with nothing