From 5116ce973c755502e70164b159f7f71f501f68c0 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Fri, 21 Aug 2026 15:21:27 +0000 Subject: [PATCH] Project a declared provider icon glyph onto ProviderInfo and draw it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PluginProviderDeclaration.icon accepts a named host glyph ("Zap") or a plugin-relative SVG path. The projection kept only the path form (logoUrl, served by the logo route) and dropped the glyph, so a provider plugin without an SVG asset showed its display name's initial in the picker — a privilege asymmetry the echo canary surfaced, since every first-party plugin ships an asset. ProviderInfo gains an optional `icon: { glyph }` (absent when the declaration named a path or nothing; at most one of icon/logoUrl is set), the same vocabulary an item presentation's icon uses. The registration projects it. The web app resolves it in getProviderIconInfo after logoUrl and before the generic ACP glyph, drawn through the shared icon set so it inherits the text color; an unknown glyph name resolves to nothing and the initial fallback stands. The picker, the providers settings list and the mobile provider picker carry it. The first-party projections are pinned against the plugins' own declarations (four logos, four icon-less ACP agents, no glyph). Server→app wire only (ProviderInfo); nothing between the server and the host daemon changes, so HOST_DAEMON_PROTOCOL_VERSION stays. Co-Authored-By: Claude --- .../settings/ProvidersSettingsSection.tsx | 5 +- .../app/src/hooks/useThreadCreationOptions.ts | 2 +- apps/app/src/lib/provider-icon.test.tsx | 77 +++++++++++++---- apps/app/src/lib/provider-icon.ts | 84 ++++++++++++++++--- .../data/compose/execution-options.test.ts | 39 ++++++++- .../src/data/compose/execution-options.ts | 3 + .../src/screens/pickers/ProviderPicker.tsx | 16 +++- .../providers/plugin-provider-registration.ts | 6 +- .../plugin-provider-registration.test.ts | 61 ++++++++++++++ docs/api_to_audit.md | 13 ++- packages/domain/src/provider-types.ts | 11 +++ 11 files changed, 274 insertions(+), 43 deletions(-) diff --git a/apps/app/src/components/settings/ProvidersSettingsSection.tsx b/apps/app/src/components/settings/ProvidersSettingsSection.tsx index 5902ac462b..211dce3ed5 100644 --- a/apps/app/src/components/settings/ProvidersSettingsSection.tsx +++ b/apps/app/src/components/settings/ProvidersSettingsSection.tsx @@ -62,10 +62,7 @@ export function ProvidersSettingsSection({ ) : ( {providers.map((provider, index) => { - const ProviderIcon = getProviderIconInfo( - provider.id, - provider.logoUrl, - )?.icon; + const ProviderIcon = getProviderIconInfo(provider.id, provider)?.icon; const isDefault = generalSettings.defaultProviderId === provider.id || (generalSettings.defaultProviderId === null && index === 0); diff --git a/apps/app/src/hooks/useThreadCreationOptions.ts b/apps/app/src/hooks/useThreadCreationOptions.ts index 1c565f2f41..a105fa9abc 100644 --- a/apps/app/src/hooks/useThreadCreationOptions.ts +++ b/apps/app/src/hooks/useThreadCreationOptions.ts @@ -440,7 +440,7 @@ export function useThreadCreationOptions( providers.map((p) => ({ value: p.id, label: p.displayName, - icon: getProviderIconInfo(p.id, p.logoUrl ?? null)?.icon, + icon: getProviderIconInfo(p.id, p)?.icon, ...(p.strings?.brandPrefix === undefined ? {} : { brandPrefix: p.strings.brandPrefix }), diff --git a/apps/app/src/lib/provider-icon.test.tsx b/apps/app/src/lib/provider-icon.test.tsx index 8c24363da8..c4018b8611 100644 --- a/apps/app/src/lib/provider-icon.test.tsx +++ b/apps/app/src/lib/provider-icon.test.tsx @@ -35,18 +35,16 @@ afterEach(() => { describe("getProviderIconInfo", () => { it("prefers a configured provider logo over the generic ACP icon", () => { - const iconInfo = getProviderIconInfo( - "acp-do-computer", - "/api/v1/system/providers/acp-do-computer/logo", - ); + const iconInfo = getProviderIconInfo("acp-do-computer", { + logoUrl: "/api/v1/system/providers/acp-do-computer/logo", + }); if (iconInfo === undefined) { throw new Error("Expected configured provider logo icon info"); } expect( - getProviderIconInfo( - "acp-do-computer", - "/api/v1/system/providers/acp-do-computer/logo", - )?.icon, + getProviderIconInfo("acp-do-computer", { + logoUrl: "/api/v1/system/providers/acp-do-computer/logo", + })?.icon, ).toBe(iconInfo.icon); const view = render( @@ -71,10 +69,9 @@ describe("getProviderIconInfo", () => { // resolves to black there, invisible on dark themes. Known ids must keep // their inline React marks even when the server provides a logoUrl. for (const providerId of ["codex", "claude-code", "pi", "acp-opencode"]) { - const iconInfo = getProviderIconInfo( - providerId, - `/api/v1/system/providers/${providerId}/logo`, - ); + const iconInfo = getProviderIconInfo(providerId, { + logoUrl: `/api/v1/system/providers/${providerId}/logo`, + }); if (iconInfo === undefined) { throw new Error(`Expected icon info for ${providerId}`); } @@ -85,11 +82,59 @@ describe("getProviderIconInfo", () => { } }); - it("lets a plugin-registered component win, and falls back when it goes away", () => { - const iconInfo = getProviderIconInfo( - "codex", - "/api/v1/system/providers/codex/logo", + it("draws a declared host glyph for a provider without a logo, and keeps it below a logo", () => { + // `icon: "Zap"` on the declaration: no bytes to serve, so no logoUrl; the + // glyph arrives by name and must render through the shared icon set + // (inline svg, inherits the text color) instead of the initial. + const glyphInfo = getProviderIconInfo("echo-agent", { + logoUrl: null, + icon: { glyph: "Zap" }, + }); + if (glyphInfo === undefined) { + throw new Error("Expected a glyph icon for echo-agent"); + } + expect( + getProviderIconInfo("echo-agent", { logoUrl: null, icon: { glyph: "Zap" } }) + ?.icon, + ).toBe(glyphInfo.icon); + const glyphView = render( + createElement(glyphInfo.icon, { className: "size-4" }), ); + expect(glyphView.container.querySelector("img")).toBeNull(); + expect( + glyphView.container.querySelector('svg[data-icon="Zap"]'), + ).not.toBeNull(); + glyphView.unmount(); + + // A glyph the host does not know resolves to nothing, so the caller's + // fallback (the initial) takes over instead of an empty box. + expect( + getProviderIconInfo("echo-agent", { + logoUrl: null, + icon: { glyph: "NoSuchGlyph" }, + }), + ).toBeUndefined(); + + // A file logo is the richer asset: it wins when both are present. + const bothInfo = getProviderIconInfo("echo-agent", { + logoUrl: "/api/v1/system/providers/echo-agent/logo", + icon: { glyph: "Zap" }, + }); + if (bothInfo === undefined) { + throw new Error("Expected icon info when both forms are present"); + } + const bothView = render(createElement(bothInfo.icon, {})); + expect(bothView.container.querySelector("img")).not.toBeNull(); + bothView.unmount(); + + // Unknown non-ACP provider with neither form: nothing, as before. + expect(getProviderIconInfo("echo-agent", { logoUrl: null })).toBeUndefined(); + }); + + it("lets a plugin-registered component win, and falls back when it goes away", () => { + const iconInfo = getProviderIconInfo("codex", { + logoUrl: "/api/v1/system/providers/codex/logo", + }); if (iconInfo === undefined) { throw new Error("Expected icon info for codex"); } diff --git a/apps/app/src/lib/provider-icon.ts b/apps/app/src/lib/provider-icon.ts index bc4f69b85f..739a5ac8c2 100644 --- a/apps/app/src/lib/provider-icon.ts +++ b/apps/app/src/lib/provider-icon.ts @@ -8,7 +8,7 @@ import { OpenAiIcon } from "@/components/icons/OpenAiIcon"; import { OpencodeIcon } from "@/components/icons/OpencodeIcon"; import { OmpIcon } from "@/components/icons/OmpIcon"; import { PiIcon } from "@/components/icons/PiIcon"; -import { Icon } from "@bb/shared-ui/icon"; +import { Icon, ICON_NAMES, type IconName } from "@bb/shared-ui/icon"; import { getPluginSlotSnapshot, subscribePluginSlots } from "./plugin-slots"; const ACP_ID_PREFIX = "acp-"; @@ -25,6 +25,44 @@ function isAcpProviderId(providerId: string): boolean { const GenericAcpIcon: ComponentType<{ className?: string }> = ({ className }) => createElement(Icon, { name: "Code", className, "aria-hidden": "true" }); +/** + * What a caller knows about a provider's declared icon, straight off its + * `ProviderInfo`: a file logo served by the host (`logoUrl`) or a named host + * glyph (`icon.glyph`). A declaration names at most one. + */ +export interface ProviderIconSource { + logoUrl: string | null; + icon?: { glyph: string }; +} + +function isIconName(name: string): name is IconName { + return (ICON_NAMES as readonly string[]).includes(name); +} + +const declaredGlyphIcons = new Map>(); + +/** + * A provider's declared host glyph, rendered through the shared icon set so + * it inherits the surrounding text color like a vendored brand mark. An + * unknown glyph name (a newer host's vocabulary, a typo) resolves to nothing, + * and the caller's fallback chain continues. + */ +function getDeclaredGlyphIcon( + glyph: string, +): ComponentType<{ className?: string }> | undefined { + if (!isIconName(glyph)) { + return undefined; + } + const cached = declaredGlyphIcons.get(glyph); + if (cached !== undefined) { + return cached; + } + const GlyphIcon: ComponentType<{ className?: string }> = ({ className }) => + createElement(Icon, { name: glyph, className, "aria-hidden": "true" }); + declaredGlyphIcons.set(glyph, GlyphIcon); + return GlyphIcon; +} + // Vendored brand marks for the built-in providers, keyed by provider id. The // first-party provider plugins ship no frontend bundle: registering these // same marks through `app.slots.experimental_providerIcon` cost four JS+CSS @@ -66,7 +104,9 @@ function getConfiguredProviderLogoIcon( return cached; } - const fallbackIcon = resolveStaticProviderIconInfo(providerId, null)?.icon; + const fallbackIcon = resolveStaticProviderIconInfo(providerId, { + logoUrl: null, + })?.icon; const ProviderLogoIcon: ComponentType<{ className?: string }> = ({ className, }) => { @@ -110,10 +150,10 @@ const pluginAwareProviderIcons = new Map< */ function getPluginAwareProviderIcon( providerId: string, - logoUrl: string | null, + source: ProviderIconSource, staticIcon: ComponentType<{ className?: string }> | undefined, ): ComponentType<{ className?: string }> { - const cacheKey = `${providerId}\0${logoUrl ?? ""}`; + const cacheKey = `${providerId}\0${source.logoUrl ?? ""}\0${source.icon?.glyph ?? ""}`; const cached = pluginAwareProviderIcons.get(cacheKey); if (cached !== undefined) { return cached; @@ -152,29 +192,41 @@ function getPluginAwareProviderIcon( * 3. A caller-supplied `logoUrl` (from a server-provided `ProviderInfo`) for * providers without a vendored mark — plugin-registered third parties, and * the right home for static color logos. - * 4. The generic glyph for unrecognized ACP providers. + * 4. The host glyph the provider declared (`ProviderInfo.icon.glyph`, from a + * declaration like `icon: "Zap"`): a plugin without an SVG asset still + * gets a mark instead of its initial. Drawn through the shared icon set, + * so it inherits the text color like a vendored mark. + * 5. The generic glyph for unrecognized ACP providers. * * Returns undefined for unknown non-ACP providers so callers can fall back - * gracefully. + * gracefully (the picker shows the display name's initial). + * + * The second argument is the provider's declared icon source — pass the + * `ProviderInfo` itself, or nothing for surfaces that only know the id. */ export function getProviderIconInfo( providerId: string, - logoUrl: string | null = null, + source: ProviderIconSource | null = null, ): ProviderIconInfo | undefined { - const staticInfo = resolveStaticProviderIconInfo(providerId, logoUrl); + const resolvedSource = source ?? { logoUrl: null }; + const staticInfo = resolveStaticProviderIconInfo(providerId, resolvedSource); const pluginIcon = getRegisteredPluginProviderIcon(providerId); if (staticInfo === undefined && pluginIcon === undefined) { return undefined; } return { - icon: getPluginAwareProviderIcon(providerId, logoUrl, staticInfo?.icon), + icon: getPluginAwareProviderIcon( + providerId, + resolvedSource, + staticInfo?.icon, + ), ariaLabel: staticInfo?.ariaLabel ?? providerId, }; } function resolveStaticProviderIconInfo( providerId: string, - logoUrl: string | null, + source: ProviderIconSource, ): ProviderIconInfo | undefined { const builtInBrand = BUILT_IN_BRAND_ICONS[providerId]; if (builtInBrand !== undefined) { @@ -189,13 +241,21 @@ function resolveStaticProviderIconInfo( } } - if (logoUrl !== null) { + if (source.logoUrl !== null) { return { - icon: getConfiguredProviderLogoIcon(providerId, logoUrl), + icon: getConfiguredProviderLogoIcon(providerId, source.logoUrl), ariaLabel: "Provider logo", }; } + const glyphIcon = + source.icon === undefined + ? undefined + : getDeclaredGlyphIcon(source.icon.glyph); + if (glyphIcon !== undefined) { + return { icon: glyphIcon, ariaLabel: "Provider icon" }; + } + if (isAcpProviderId(providerId)) { return { icon: GenericAcpIcon, ariaLabel: "ACP provider" }; } diff --git a/apps/mobile/src/data/compose/execution-options.test.ts b/apps/mobile/src/data/compose/execution-options.test.ts index 891b7c4afb..458d1e8e99 100644 --- a/apps/mobile/src/data/compose/execution-options.test.ts +++ b/apps/mobile/src/data/compose/execution-options.test.ts @@ -1,8 +1,9 @@ -import type { AvailableModel } from "@bb/domain"; +import type { AvailableModel, ProviderInfo } from "@bb/domain"; import type { SystemExecutionOptionsResponse } from "@bb/server-contract"; import { describe, expect, it } from "vitest"; import { buildPermissionModeOptions, + buildProviderOptions, buildReasoningOptions, formatModelLabel, resolveModelSelection, @@ -204,6 +205,42 @@ describe("permission modes", () => { }); }); +describe("buildProviderOptions", () => { + it("carries the declared glyph beside the logo so the picker can draw either", () => { + const base: Omit = { + available: true, + experimental_providerHealth: false, + experimental_providerUsage: false, + experimental_providerInstallation: false, + capabilities: { + supportsThreadArchive: false, + supportsThreadRename: false, + supportsServiceTier: false, + supportsNativeUserQuestion: false, + supportsFork: false, + supportsSessionRewind: false, + permissionModes: ["full"], + }, + composerActions: [], + }; + expect( + buildProviderOptions([ + { ...base, id: "codex", displayName: "Codex", logoUrl: "/logo" }, + { + ...base, + id: "echo-agent", + displayName: "Echo", + logoUrl: null, + icon: { glyph: "Zap" }, + }, + ]).map(({ value, logoUrl, glyph }) => ({ value, logoUrl, glyph })), + ).toEqual([ + { value: "codex", logoUrl: "/logo", glyph: null }, + { value: "echo-agent", logoUrl: null, glyph: "Zap" }, + ]); + }); +}); + describe("formatModelLabel", () => { it("title-cases hyphenated ids and keeps version numbers", () => { expect(formatModelLabel("gpt-5.4-mini")).toBe("GPT-5.4-Mini"); diff --git a/apps/mobile/src/data/compose/execution-options.ts b/apps/mobile/src/data/compose/execution-options.ts index af2dac0a2d..57b42986a6 100644 --- a/apps/mobile/src/data/compose/execution-options.ts +++ b/apps/mobile/src/data/compose/execution-options.ts @@ -27,6 +27,8 @@ export interface ProviderPickerOption { value: string; label: string; logoUrl: string | null; + /** The host glyph the provider declared, when it shipped no logo file. */ + glyph: string | null; available: boolean; } @@ -87,6 +89,7 @@ export function buildProviderOptions( value: provider.id, label: provider.displayName, logoUrl: provider.logoUrl, + glyph: provider.icon?.glyph ?? null, available: provider.available, })); } diff --git a/apps/mobile/src/screens/pickers/ProviderPicker.tsx b/apps/mobile/src/screens/pickers/ProviderPicker.tsx index 427d05ebc2..7e2a2d38b8 100644 --- a/apps/mobile/src/screens/pickers/ProviderPicker.tsx +++ b/apps/mobile/src/screens/pickers/ProviderPicker.tsx @@ -1,7 +1,7 @@ import { useMemo } from "react"; import type { ProviderPickerOption } from "@/data/compose"; import { useTheme } from "@/theme"; -import { useSheet } from "@/ui"; +import { isIconName, useSheet, type IconName } from "@/ui"; import { ServerSvgIcon } from "../plugins/ServerSvgIcon"; import { OptionSheet, type PickerOption } from "./OptionSheet"; import { PickerTrigger } from "./PickerTrigger"; @@ -17,8 +17,16 @@ interface ProviderPickerProps { /** * Agent provider (Codex, Claude, …) picker. Provider logos come from the * server (`GET /system/providers/:id/logo`, `currentColor` SVGs) painted in - * the theme foreground; a provider without a logo gets the Zap glyph. + * the theme foreground; a provider that declared a named glyph instead of a + * logo file (`icon: "Zap"` on its declaration) gets that glyph when this app + * knows it, and any other provider gets the Zap glyph. */ +function providerGlyph(option: ProviderPickerOption): IconName { + return option.glyph !== null && isIconName(option.glyph) + ? option.glyph + : "Zap"; +} + export function ProviderPicker({ options, value, @@ -33,7 +41,7 @@ export function ProviderPicker({ options.map((option) => ({ value: option.value, label: option.label, - icon: "Zap", + icon: providerGlyph(option), leading: option.logoUrl === null ? undefined : ( = {}, @@ -292,12 +293,72 @@ describe("buildPluginProviderRegistration", () => { }); expect(registration.info.logoUrl).toBeNull(); + expect(registration.info.icon).toBeUndefined(); expect(registration.info.composerActions).toStrictEqual([ { kind: "skills", trigger: "/" }, ]); // No service tier → no tier options at all, not an empty list. expect(registration.info.serviceTiers).toBeUndefined(); }); + + it("projects a named glyph icon by name and a path icon as a logo URL, never both", () => { + // `icon: "Zap"` has no bytes for the logo route to serve; before this the + // glyph was dropped and the picker showed the display name's initial. + const glyph = buildPluginProviderRegistration({ + available: true, + pluginId: "echo-provider", + declaration: declaration({ id: "echo-agent", icon: "Zap" }), + readSettings: NO_SETTINGS, + }); + expect(glyph.info.icon).toStrictEqual({ glyph: "Zap" }); + expect(glyph.info.logoUrl).toBeNull(); + + const path = buildPluginProviderRegistration({ + available: true, + pluginId: "acme-agent", + declaration: declaration({ icon: "./icons/agent.svg" }), + readSettings: NO_SETTINGS, + }); + expect(path.info.icon).toBeUndefined(); + expect(path.info.logoUrl).toBe("/api/v1/system/providers/my-remote-agent/logo"); + }); + + it("leaves the first-party providers on their SVG assets (no glyph)", async () => { + // The four first-party plugins ship icon files; the glyph projection must + // not touch how they arrive. Pinned against the declarations themselves. + const declarations = await loadFirstPartyProviderDeclarations(); + const projected = [...declarations.entries()].flatMap(([pluginId, list]) => + list.map((declared) => { + const { info } = buildPluginProviderRegistration({ + available: true, + pluginId, + declaration: declared, + readSettings: NO_SETTINGS, + }); + return { id: info.id, logoUrl: info.logoUrl, icon: info.icon }; + }), + ); + // The well-known ACP agents beyond Cursor declare no icon at all: the + // app vendors their marks by id and they must stay that way too. + expect(projected).toStrictEqual([ + { id: "codex", logoUrl: "/api/v1/system/providers/codex/logo", icon: undefined }, + { + id: "claude-code", + logoUrl: "/api/v1/system/providers/claude-code/logo", + icon: undefined, + }, + { id: "pi", logoUrl: "/api/v1/system/providers/pi/logo", icon: undefined }, + { + id: "acp-cursor", + logoUrl: "/api/v1/system/providers/acp-cursor/logo", + icon: undefined, + }, + { id: "acp-opencode", logoUrl: null, icon: undefined }, + { id: "acp-omp", logoUrl: null, icon: undefined }, + { id: "acp-grok", logoUrl: null, icon: undefined }, + { id: "acp-hermes-agent", logoUrl: null, icon: undefined }, + ]); + }); }); function standardSchema() { diff --git a/docs/api_to_audit.md b/docs/api_to_audit.md index 57d22ead47..08afe2ca1c 100644 --- a/docs/api_to_audit.md +++ b/docs/api_to_audit.md @@ -709,9 +709,16 @@ provider-retry) stops vendoring provider names, icons, and copy. signature. 2. **Icons.** `logoUrl` is null for the bundled first-party providers (their marks are vendored in the app), so a plugin still cannot draw every - provider's icon from this hook alone. Decide whether the host serves its - vendored marks through the logo route, or exposes an icon component, before - telling plugins to delete their copies. + provider's icon from this hook alone. A provider that declared a named + glyph (`icon: "Zap"`) now arrives as `icon: { glyph }` beside `logoUrl` + (at most one of the two is set; the host draws vendored mark → `logoUrl` + → `icon.glyph` → the initial), so a plugin can draw glyph-declared + third-party providers through the shared icon set. Decide whether the host + serves its vendored marks through the logo route, or exposes an icon + component, before telling plugins to delete their copies — and whether + `icon` and `logoUrl` fold into one `{ glyph } | { url }` field, the + declaration's own `{ glyph } | { asset }` shape, when `ProviderInfo` + stabilizes. ## `app.slots.experimental_providerIcon` (`@get-bb/plugin-sdk/app`) diff --git a/packages/domain/src/provider-types.ts b/packages/domain/src/provider-types.ts index 0306d2d71d..274f670bca 100644 --- a/packages/domain/src/provider-types.ts +++ b/packages/domain/src/provider-types.ts @@ -142,6 +142,17 @@ export const providerInfoSchema = z.object({ * Absent when the provider declared none. Grouping only. */ family: z.string().min(1).optional(), + /** + * The declared icon, projected by form. A plugin-relative asset path + * (`icon: "./icons/agent.svg"`) is served by the provider-logo route and + * arrives here as `logoUrl`; a named host glyph (`icon: "Zap"`) has no + * bytes to serve and arrives as `icon.glyph`, the same vocabulary an + * item presentation's `icon` uses. A declaration names at most one form, + * so at most one of the two is set; `icon` is absent when the declaration + * named a path or nothing. Clients draw a vendored brand mark first, then + * `logoUrl`, then `icon.glyph`, then the display name's initial. + */ + icon: z.object({ glyph: z.string().min(1) }).optional(), logoUrl: z.string().min(1).nullable(), /** Sessionless maintenance methods declared by the provider plugin. */ experimental_providerHealth: z.boolean(),