diff --git a/.changeset/integration-icon-unification.md b/.changeset/integration-icon-unification.md new file mode 100644 index 0000000000..fd3fd263c1 --- /dev/null +++ b/.changeset/integration-icon-unification.md @@ -0,0 +1,12 @@ +--- +"executor": patch +"@executor-js/plugin-mcp": patch +--- + +Show real icons on the integration browse page. The Codex plugin preset cards +rendered a bare-letter avatar: the page fed the preset's authenticated +`executor:` icon path into a raw ``, which can never load it, and never +consulted the preset's public fallback image. Icon rendering is now unified on +one component — explicit icon, then its fallback image, then the favicon +derived from the integration's URL, then a neutral mark — used by the browse +cards, the command palette, and the Codex plugin add screen alike. diff --git a/e2e/local/codex-plugins.test.ts b/e2e/local/codex-plugins.test.ts index 423e9a13ed..89597666f6 100644 --- a/e2e/local/codex-plugins.test.ts +++ b/e2e/local/codex-plugins.test.ts @@ -28,7 +28,7 @@ import { composePluginApi } from "@executor-js/api/server"; import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; import { scenario } from "../src/scenario"; -import { Cli, RunDir } from "../src/services"; +import { Browser, Cli, RunDir, Target } from "../src/services"; import { withLocalServer } from "./local-server"; const api = composePluginApi([mcpHttpPlugin()] as const); @@ -109,7 +109,10 @@ scenario( { timeout: 300_000 }, Effect.gen(function* () { const cli = yield* Cli; + const browser = yield* Browser; const runDir = yield* RunDir; + const target = yield* Target; + const identity = yield* target.newIdentity(); const codexHome = makeCodexHome(); yield* withLocalServer( @@ -142,6 +145,25 @@ scenario( CODEX_HOME: codexHome, }); } + + yield* browser.session(identity, async ({ page, step }) => { + await step("Find Computer Use on the integration browse page", async () => { + await page.goto(server.url, { waitUntil: "domcontentloaded" }); + await page.getByRole("link", { name: "Add integration" }).click(); + await page + .getByRole("textbox", { name: "Search integrations, or paste a URL" }) + .fill("Computer Use"); + + const card = page.getByTestId("preset-mcp-codex-computer-use"); + await card.getByText("Computer Use MCP").waitFor({ timeout: 30_000 }); + await card + .locator( + 'img[src="https://learn.chatgpt.com/images/codex/icons/computer-use-plugin-icon.png"]', + ) + .waitFor({ timeout: 30_000 }); + }); + }); + // Curated entries carry the app-server bridge recipe: `codex // app-server`, the server name the bridge calls tools on, and the // preset it came from — that last one is what lets a macOS refusal diff --git a/packages/plugins/mcp/src/react/CodexPluginAdd.tsx b/packages/plugins/mcp/src/react/CodexPluginAdd.tsx index 957e1d00f0..dbef98116a 100644 --- a/packages/plugins/mcp/src/react/CodexPluginAdd.tsx +++ b/packages/plugins/mcp/src/react/CodexPluginAdd.tsx @@ -5,6 +5,7 @@ import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import { Button } from "@executor-js/react/components/button"; import { FloatActions } from "@executor-js/react/components/float-actions"; +import { IntegrationFavicon } from "@executor-js/react/components/integration-favicon"; import { integrationsOptimisticAtom } from "@executor-js/react/api/atoms"; import { integrationWriteKeys } from "@executor-js/react/api/reactivity-keys"; import { addIntegrationErrorMessage } from "@executor-js/react/lib/integration-add"; @@ -150,10 +151,11 @@ export default function CodexPluginAdd(props: { {/* The plugin's own icon comes from the local Codex install; without one the card still identifies its provider rather than showing a gap, which matters most on the machines that have no install. */} -
diff --git a/packages/plugins/mcp/src/sdk/presets.ts b/packages/plugins/mcp/src/sdk/presets.ts index 71f1f3f981..47bbef07a1 100644 --- a/packages/plugins/mcp/src/sdk/presets.ts +++ b/packages/plugins/mcp/src/sdk/presets.ts @@ -37,7 +37,7 @@ export type McpPreset = McpRemotePreset | McpStdioPreset; // use", …). `command` is deliberately empty: the real spawn recipe is // machine-specific and comes from the server-side scanner // (`codex-plugins.ts`); picking one of these opens the focused Codex add -// screen. The icon uses the `executor:` scheme (see preset-icon.tsx): the +// screen. The icon uses the `executor:` scheme (see integration-favicon.tsx): the // plugin's own icon is a machine-local file, so it is served by the local API // and resolved with the auth header — a static URL cannot reach it. const codexPluginPresets: readonly McpStdioPreset[] = CURATED_CODEX_PLUGINS.map((plugin) => ({ diff --git a/packages/react/src/components/command-palette.tsx b/packages/react/src/components/command-palette.tsx index 585ceed1f2..7a62570d3f 100644 --- a/packages/react/src/components/command-palette.tsx +++ b/packages/react/src/components/command-palette.tsx @@ -6,7 +6,6 @@ import { PlusIcon } from "lucide-react"; import { trackEvent } from "../api/analytics"; import type { Integration } from "@executor-js/sdk/shared"; import { IntegrationFavicon, integrationPresetIconUrl } from "./integration-favicon"; -import { PresetIcon } from "./preset-icon"; import { integrationsOptimisticAtom } from "../api/atoms"; import { useIntegrationPlugins } from "@executor-js/sdk/client"; import { @@ -196,16 +195,10 @@ export function CommandPalette(props: { open: boolean; onOpenChange: (open: bool value={`preset ${e.presetName} ${e.presetSummary ?? ""} ${e.pluginLabel}`} onSelect={() => goToPreset(e.pluginKey, e.presetId, e.presetUrl)} > - - } + {e.presetName} {e.pluginLabel} diff --git a/packages/react/src/components/integration-favicon.test.tsx b/packages/react/src/components/integration-favicon.test.tsx index e9e79cc663..d2df3b51b5 100644 --- a/packages/react/src/components/integration-favicon.test.tsx +++ b/packages/react/src/components/integration-favicon.test.tsx @@ -33,6 +33,26 @@ describe("IntegrationFavicon", () => { expect(integrationFaviconSrc({ url, size: 20, failedSrcs: [primary ?? ""] })).toBeNull(); }); + it("tries the explicit fallback before the integration-derived favicon", () => { + const icon = "executor:/mcp/codex-plugins/computer-use/icon"; + const fallbackSrc = "https://integrations.sh/logo/openai.com"; + const url = "https://example.com/mcp"; + + expect(integrationFaviconSrc({ icon, fallbackSrc, url, size: 20 })).toBe(icon); + expect(integrationFaviconSrc({ icon, fallbackSrc, url, size: 20, failedSrcs: [icon] })).toBe( + fallbackSrc, + ); + expect( + integrationFaviconSrc({ + icon, + fallbackSrc, + url, + size: 20, + failedSrcs: [icon, fallbackSrc], + }), + ).toBe("https://integrations.sh/logo/example.com?sz=40"); + }); + it("uses the Executor favicon for the built-in executor integration", () => { expect(integrationLocalIconUrl("executor")).toBe("/favicon-32.png"); expect(integrationLocalIconUrl("openapi")).toBeNull(); diff --git a/packages/react/src/components/integration-favicon.tsx b/packages/react/src/components/integration-favicon.tsx index 4412599531..e448b34b01 100644 --- a/packages/react/src/components/integration-favicon.tsx +++ b/packages/react/src/components/integration-favicon.tsx @@ -1,9 +1,47 @@ import { BoxIcon } from "lucide-react"; import { useEffect, useState } from "react"; import type { IntegrationPlugin } from "@executor-js/sdk/client"; +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; import { getDomain } from "tldts"; -import { EXECUTOR_ICON_SCHEME, resolveExecutorIcon } from "./preset-icon"; +import { + getExecutorApiBaseUrl, + getExecutorServerAuthorizationHeader, +} from "../api/server-connection"; +import { cn } from "../lib/utils"; + +// --------------------------------------------------------------------------- + +const EXECUTOR_ICON_SCHEME = "executor:"; + +const IconResponse = Schema.Struct({ icon: Schema.NullOr(Schema.String) }); +const decodeIconResponse = Schema.decodeUnknownOption(IconResponse); + +const resolvedExecutorIcons = new Map>(); + +/** Resolves an authenticated local icon path once per browser session. */ +const resolveExecutorIcon = (path: string): Promise => { + const cached = resolvedExecutorIcons.get(path); + if (cached) return cached; + const authorization = getExecutorServerAuthorizationHeader(); + const request = Effect.runPromise( + Effect.tryPromise(async () => { + const response = await fetch(`${getExecutorApiBaseUrl()}${path}`, { + headers: authorization === null ? {} : { authorization }, + }); + if (!response.ok) return null; + const body: unknown = await response.json(); + return Option.match(decodeIconResponse(body), { + onNone: () => null, + onSome: ({ icon }) => icon, + }); + }).pipe(Effect.orElseSucceed(() => null)), + ); + resolvedExecutorIcons.set(path, request); + return request; +}; // --------------------------------------------------------------------------- // IntegrationFavicon — renders a small favicon derived from an integration URL. @@ -128,13 +166,15 @@ export function integrationPresetIconUrl( } // Resolution cascade for the rendered favicon: first non-null, non-failed of an -// explicit preset icon, the bundled local icon for a known integration id, then -// the integrations.sh logo proxy derived from the integration URL (which owns -// its own upstream fallbacks). The built-in executor integration has no preset -// icon and no URL, so it resolves ONLY through the integrationId branch: callers -// that drop integrationId fall through to the neutral BoxIcon placeholder. +// explicit preset icon, its explicit fallback image, the bundled local icon for +// a known integration id, then the integrations.sh logo proxy derived from the +// integration URL (which owns its own upstream fallbacks). The built-in executor +// integration has no preset icon and no URL, so it resolves ONLY through the +// integrationId branch: callers that drop integrationId fall through to the +// neutral BoxIcon placeholder. export function integrationFaviconSrc(args: { icon?: string | null; + fallbackSrc?: string | null; integrationId?: string; url?: string; size: number; @@ -144,6 +184,7 @@ export function integrationFaviconSrc(args: { return ( [ args.icon ?? null, + args.fallbackSrc ?? null, integrationLocalIconUrl(args.integrationId), integrationFaviconUrl(args.url, args.size), ].find((candidate) => candidate !== null && !failedSrcs.includes(candidate)) ?? null @@ -152,21 +193,32 @@ export function integrationFaviconSrc(args: { export function IntegrationFavicon({ icon, + fallbackSrc, integrationId, url, size = 16, + className, }: { icon?: string | null; + fallbackSrc?: string | null; integrationId?: string; url?: string; size?: number; + className?: string; }) { const [failedSrcs, setFailedSrcs] = useState([]); // `executor:`-scheme icons (served by the local API behind the bearer gate, // e.g. a Codex plugin's own icon) resolve asynchronously to a data URI; a // null resolution marks the candidate failed so the cascade continues. const [executorIcons, setExecutorIcons] = useState>>({}); - const cascadeSrc = integrationFaviconSrc({ icon, integrationId, url, size, failedSrcs }); + const cascadeSrc = integrationFaviconSrc({ + icon, + fallbackSrc, + integrationId, + url, + size, + failedSrcs, + }); const isExecutorSrc = cascadeSrc?.startsWith(EXECUTOR_ICON_SCHEME) ?? false; useEffect(() => { @@ -194,7 +246,7 @@ export function IntegrationFavicon({ return ( ); @@ -214,7 +266,7 @@ export function IntegrationFavicon({ current.includes(failedCandidate) ? current : [...current, failedCandidate], ) } - className="shrink-0 rounded-sm" + className={cn("shrink-0 rounded-sm object-contain", className)} style={{ width: size, height: size }} /> ); diff --git a/packages/react/src/components/preset-icon.tsx b/packages/react/src/components/preset-icon.tsx deleted file mode 100644 index ee34890720..0000000000 --- a/packages/react/src/components/preset-icon.tsx +++ /dev/null @@ -1,84 +0,0 @@ -// --------------------------------------------------------------------------- -// Preset icon — a preset's `icon` is usually a plain image URL. A preset whose -// icon can only come from the local server (e.g. a Codex plugin's own icon, -// read off this machine at runtime) uses the `executor:` scheme instead: the -// path after the scheme is fetched from the executor API with the server auth -// header and must answer `{ icon: string | null }` (a data URI). The -// indirection exists because local auth is deliberately bearer-header-only — -// an cannot authenticate itself. -// --------------------------------------------------------------------------- - -import { useEffect, useState } from "react"; -import * as Effect from "effect/Effect"; -import * as Option from "effect/Option"; -import * as Schema from "effect/Schema"; - -import { - getExecutorApiBaseUrl, - getExecutorServerAuthorizationHeader, -} from "../api/server-connection"; - -export const EXECUTOR_ICON_SCHEME = "executor:"; - -const IconResponse = Schema.Struct({ icon: Schema.NullOr(Schema.String) }); -const decodeIconResponse = Schema.decodeUnknownOption(IconResponse); - -const resolved = new Map>(); - -/** Resolve an `executor:`-scheme icon path to its data URI (or null). Shared - * by every surface that renders preset icons, including IntegrationFavicon's - * cascade. Results are memoized per path for the session; any failure is - * "no icon", never an error. */ -export const resolveExecutorIcon = (path: string): Promise => { - const cached = resolved.get(path); - if (cached) return cached; - const authorization = getExecutorServerAuthorizationHeader(); - const request = Effect.runPromise( - Effect.tryPromise(async () => { - const response = await fetch(`${getExecutorApiBaseUrl()}${path}`, { - headers: authorization === null ? {} : { authorization }, - }); - if (!response.ok) return null; - const body: unknown = await response.json(); - return Option.match(decodeIconResponse(body), { - onNone: () => null, - onSome: ({ icon }) => icon, - }); - }).pipe(Effect.orElseSucceed(() => null)), - ); - resolved.set(path, request); - return request; -}; - -/** Renders a preset icon, resolving `executor:` scheme icons through the - * authenticated API. - * - * `fallbackSrc` is a plain image URL to use when the machine-local icon is - * unavailable — a Codex plugin card still shows its provider's mark on a - * machine where Codex is not installed, which is exactly when the card is - * most in need of explaining itself. `fallback` is the last resort, for when - * there is no image of any kind. */ -export function PresetIcon(props: { - readonly icon?: string; - readonly fallbackSrc?: string; - readonly className?: string; - readonly fallback?: React.ReactNode; -}) { - const isExecutorIcon = props.icon?.startsWith(EXECUTOR_ICON_SCHEME) ?? false; - const [fetched, setFetched] = useState(null); - - useEffect(() => { - if (!isExecutorIcon || props.icon === undefined) return; - let live = true; - void resolveExecutorIcon(props.icon.slice(EXECUTOR_ICON_SCHEME.length)).then((icon) => { - if (live) setFetched(icon); - }); - return () => { - live = false; - }; - }, [isExecutorIcon, props.icon]); - - const src = (isExecutorIcon ? fetched : (props.icon ?? null)) ?? props.fallbackSrc ?? null; - if (src === null) return <>{props.fallback ?? null}; - return ; -} diff --git a/packages/react/src/pages/integration-browse.tsx b/packages/react/src/pages/integration-browse.tsx index 83b133e28f..6f6bedf046 100644 --- a/packages/react/src/pages/integration-browse.tsx +++ b/packages/react/src/pages/integration-browse.tsx @@ -21,7 +21,7 @@ import { Button } from "../components/button"; import { Input } from "../components/input"; import { PageHeader } from "../components/page"; import { - integrationFaviconUrl, + IntegrationFavicon, integrationInferredUrl, integrationPresetIconUrl, } from "../components/integration-favicon"; @@ -29,7 +29,6 @@ import { Skeleton } from "../components/skeleton"; import { useExecutorDocumentTitle } from "../lib/document-title"; import { availableCatalogKinds, - catalogLogoUrl, filterCatalogEntries, resolveConnectTarget, useCatalogBrowse, @@ -168,7 +167,10 @@ interface Row { * distinguishable. Omitted when the title is already the domain. */ readonly domain?: string; readonly description?: string; - readonly iconUrl?: string; + readonly icon?: string; + readonly fallbackSrc?: string; + readonly integrationId?: string; + readonly url?: string; readonly onSelect: () => void; readonly added: boolean; /** Namespace of the installed integration this row matched — the View @@ -214,36 +216,6 @@ function LoadMoreSentinel(props: { readonly onVisible: () => void }) { // Row // --------------------------------------------------------------------------- -/** Registry logos 404 for plenty of hosts; a broken-image glyph in a list of - * brand marks looks like a bug, so failures fall back to a neutral mark. */ -function RowIcon(props: { readonly src?: string; readonly alt: string }) { - const [failed, setFailed] = useState(false); - if (!props.src || failed) { - return ( - - {props.alt.charAt(0).toUpperCase()} - - ); - } - return ( - setFailed(true)} - className="size-5 object-contain" - /> - ); -} - function ResultCard(props: { readonly row: Row }) { const { row } = props; return ( @@ -253,7 +225,13 @@ function ResultCard(props: { readonly row: Row }) { >
- + {row.added ? ( // The add happened HERE — the card is the receipt. View jumps to @@ -725,7 +703,8 @@ export function IntegrationBrowsePage() { title, kindKey, ...(entry.preset.summary ? { description: entry.preset.summary } : {}), - ...(entry.preset.icon ? { iconUrl: entry.preset.icon } : {}), + ...(entry.preset.icon ? { icon: entry.preset.icon } : {}), + ...(entry.preset.fallbackIcon ? { fallbackSrc: entry.preset.fallbackIcon } : {}), onSelect: () => pickPreset(entry), // The rendered title is also what the add flow derives the namespace // from, so the card recognises an add made through it even when the @@ -774,8 +753,8 @@ export function IntegrationBrowsePage() { kindKey: surface.kind, domain: entry.domain, ...(description ? { description } : {}), - iconUrl: - (known && "icon" in known ? known.icon : undefined) ?? catalogLogoUrl(entry.domain, 10), + ...(known && "icon" in known && known.icon ? { icon: known.icon } : {}), + url: `https://${entry.domain}`, onSelect: () => void pickCatalogEntry(entry, surface.kind, title, rowKey, known ?? undefined), added: @@ -854,38 +833,34 @@ export function IntegrationBrowsePage() { String(row.slug).includes(text)), ) .map((row): Row => { - // The SAME icon cascade the sidebar runs — preset icon by exact - // identity, else the logo proxy from the integration's own URL. Two - // resolvers for one integration is how the sidebar and the picker - // ended up disagreeing about what DeepWiki looks like. - const iconUrl = - integrationPresetIconUrl( - { - id: String(row.slug), - kind: row.kind, - name: row.name, - ...(row.displayUrl ? { url: row.displayUrl } : {}), - }, - integrationPlugins, - ) ?? - integrationFaviconUrl( - row.displayUrl ?? - integrationInferredUrl({ id: String(row.slug), name: row.name }) ?? - undefined, - 10, - ); + const integrationId = String(row.slug); + const url = + row.displayUrl ?? + integrationInferredUrl({ id: integrationId, name: row.name }) ?? + undefined; return { - key: `installed-${String(row.slug)}`, - testId: `installed-${String(row.slug)}`, + key: `installed-${integrationId}`, + testId: `installed-${integrationId}`, title: row.name, kindKey: row.kind, ...(row.description && row.description !== row.name ? { description: tidyDescription(row.description) } : {}), - ...(iconUrl ? { iconUrl } : {}), + icon: + integrationPresetIconUrl( + { + id: integrationId, + kind: row.kind, + name: row.name, + ...(row.displayUrl ? { url: row.displayUrl } : {}), + }, + integrationPlugins, + ) ?? undefined, + integrationId, + url, onSelect: () => {}, added: true, - viewSlug: String(row.slug), + viewSlug: integrationId, busy: false, }; });