feat(agent-adapter,ui): surface codex skill brand icons and display names in the command catalog - #431
Open
Zerlight wants to merge 11 commits into
Open
feat(agent-adapter,ui): surface codex skill brand icons and display names in the command catalog#431Zerlight wants to merge 11 commits into
Zerlight wants to merge 11 commits into
Conversation
…s with integration icons
…ated activity groups
Contributor
There was a problem hiding this comment.
Pull request overview
This PR carries provider-supplied command branding (display names, icons, brand colors) end-to-end—starting from codex skills/list through the schema and wire protocol—so the UI can render branded command rows/chips. It also normalizes opencode MCP tool titles to the shared mcp__<server>__<tool> slug and adds integration brand glyphs + per-brand activity grouping in the chat timeline.
Changes:
- Extend
AgentCommand(schema + wire v74) withdisplayName,iconDataUri, andbrandColor, and propagate these through codex command ingest (with per-icon + total payload caps). - Add UI command catalog context + branded glyph/tint rendering across the composer, transcript command echoes, tool rows, and activity-run headers/groups.
- Normalize opencode MCP tool names to the shared MCP slug for both live streaming and cold history reads, including an engine-supplied injected-server-name hint.
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Add @iconify-json/simple-icons to the shared catalog for brand glyph imports. |
| pnpm-lock.yaml | Lockfile updates for @iconify-json/simple-icons. |
| package.json | Add @iconify-json/simple-icons workspace dependency. |
| eslint.config.cjs | Allow unplugin-icons virtual module usage in the new integration brand component. |
| apps/webview/package.json | Add @iconify-json/simple-icons dependency for webview build. |
| apps/desktop/package.json | Add @iconify-json/simple-icons dependency for desktop build. |
| packages/presentation/ui/src/tool-utils.ts | Update MCP slug parsing doc comment to reflect shared cross-adapter usage. |
| packages/presentation/ui/src/shell/conversation-surface.tsx | Provide command catalog context to the conversation surface for transcript echo chips. |
| packages/presentation/ui/src/shell/composer-editor/directive-state.ts | Add commandFor lookup for branded composer directive chips. |
| packages/presentation/ui/src/shell/composer-editor/chips.tsx | Render branded command chip glyphs/tints in the composer. |
| packages/presentation/ui/src/shell/composer-command.tsx | Show command display names in the command catalog and render branded glyphs for command entries. |
| packages/presentation/ui/src/chat/user-message.tsx | Detect /command args echoes and render a branded chip when the command is in the catalog. |
| packages/presentation/ui/src/chat/tool.tsx | Prefer caller/plugin-provided glyphs even when a tool call fails. |
| packages/presentation/ui/src/chat/tool-call-item.tsx | Add integration brand glyph fallback for MCP tool headers. |
| packages/presentation/ui/src/chat/integration-brand.tsx | New: map MCP server-name tokens to known integration glyphs + labels. |
| packages/presentation/ui/src/chat/command-catalog.ts | New: build/consume an O(1) command lookup for transcript echo branding. |
| packages/presentation/ui/src/chat/command-brand.tsx | New: branded command glyph rendering + runtime tint style + catalog provider. |
| packages/presentation/ui/src/chat/activity-summary.ts | Add brand resolution for activity items/runs based on MCP server name. |
| packages/presentation/ui/src/chat/activity-run.tsx | Render branded activity run headers/icons and add i18n keys for branded wording. |
| packages/presentation/ui/src/chat/activity-groups.ts | Split activity runs by integration brand; glue thinking into surrounding runs. |
| packages/presentation/ui/src/chat/tests/user-message.test.tsx | Test transcript echo branding behavior for known vs unknown leading slashes. |
| packages/presentation/ui/src/chat/tests/tool-call-metadata.test.tsx | Test that known integration MCP calls wear a brand glyph in the header icon slot. |
| packages/presentation/ui/src/chat/tests/integration-brand.test.tsx | New: unit tests for server-name token matching + glyph rendering. |
| packages/presentation/ui/src/chat/tests/activity-run.test.tsx | Test branded run naming/icon behavior, including failure-state tint rules. |
| packages/presentation/ui/src/tests/composer-command.test.ts | Test displayName query matching and hint formatting. |
| packages/presentation/ui/src/tests/activity-summary.test.ts | Add tests for activityRunBrand resolution rules. |
| packages/presentation/ui/src/tests/activity-groups.test.ts | Add tests for per-brand grouping and glue behavior. |
| packages/presentation/i18n/src/locales/zh-cn.ts | Add branded integration activity strings. |
| packages/presentation/i18n/src/locales/en.ts | Add branded integration activity strings. |
| packages/host/engine/src/session/start-options-resolver.ts | Provide engine-side injected MCP server-name hint for cold history reads. |
| packages/host/engine/src/session/history-service.ts | Thread injected server-name hint into adapter history reads (adapter-local context). |
| packages/host/engine/src/engine.ts | Wire the injected-server-name hint provider into HistoryService. |
| packages/host/engine/src/tests/start-options-mcp.test.ts | Test injected MCP server-name enumeration. |
| packages/host/engine/src/tests/history-service.test.ts | Test that injected MCP server names are passed through on cold reads. |
| packages/host/engine/src/tests/fixtures/history-adapter.ts | Update test adapter to accept AgentHistoryReadContext. |
| packages/host/agent-adapter/src/native/opencode/history.ts | Normalize opencode MCP tool names to mcp__<server>__<tool> when resolvable. |
| packages/host/agent-adapter/src/native/opencode/adapter.ts | Use injected/configured server-name sets to retitle opencode MCP tools live + in history. |
| packages/host/agent-adapter/src/native/codex/adapter.ts | Ingest codex skill brand identity and embed/cap icon payloads as data URIs. |
| packages/host/agent-adapter/src/base.ts | Update base adapter history read signature to AgentHistoryReadContext. |
| packages/host/agent-adapter/src/adapter.ts | Introduce AgentHistoryReadContext (wire options + engine-local hints). |
| packages/host/agent-adapter/src/tests/opencode.test.ts | Test live normalization using injected + config-declared MCP servers. |
| packages/host/agent-adapter/src/tests/opencode-history.test.ts | Test opencode MCP prefix splitting + retitling behavior. |
| packages/host/agent-adapter/src/tests/opencode-history-adapter.test.ts | Test cold-read retitling using engine hint + config servers. |
| packages/host/agent-adapter/src/tests/codex-commands.test.ts | Test codex brand identity projection + icon embedding + payload budget cap behavior. |
| packages/foundation/schema/src/wire/message.ts | Bump WIRE_PROTOCOL_VERSION 73 → 74 for additive wire schema changes. |
| packages/foundation/schema/src/model/agent/input.ts | Extend AgentCommand schema with displayName, iconDataUri, brandColor. |
| packages/client/workbench/tests/integration/dev-mock-transport.test.ts | Update mock transport expectations to include branded commands. |
| packages/client/workbench/src/mock/data/showcase.ts | Add branded MCP call examples for the UI showcase. |
| packages/client/workbench/src/mock/data/commands.ts | Add branded mock command fixtures with data-URI icon + brandColor. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/host/agent-adapter/src/native/opencode/adapter.ts:731
readHistoryalso treats MCP server-name resolution as best-effort, butclient.config.get(...)is awaited without error handling. A rejected config read would fail the entire history read (and thus transcript loading) even though only MCP retitling depends on it. Swallow config-get failures and proceed with just the injected/engine-providedmcpServerNames.
// Best-effort MCP server names so replayed MCP tool titles converge with live ones — a
// pure config read, never `mcp.status` (its lazy init would spawn/dial every configured
// server from the shared history instance). Config-declared servers resolve, including
// disabled ones; engine-injected servers exist only on a session's own live instance, so
// the caller's `mcpServerNames` hint is the only way their calls resolve here.
const names = new Set(opts.mcpServerNames);
const config = await client.config.get({ directory: got.data.directory });
for (const name of Object.keys(config.data?.mcp ?? {})) names.add(name);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+699
to
+707
| private async collectMcpServerNames(injected: StartOptions['mcpServers']): Promise<void> { | ||
| const names = new Set((injected ?? []).map((server) => server.name)); | ||
| this.mcpServerNames = [...names]; | ||
| if (!this.client) return; | ||
| const config = await this.client.config.get({ directory: this.directory }); | ||
| if (!config.data?.mcp) return; | ||
| for (const name of Object.keys(config.data.mcp)) names.add(name); | ||
| this.mcpServerNames = [...names]; | ||
| } |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
- Schema is correctly additive.
displayName/iconDataUri/brandColorare all optional, andWIRE_PROTOCOL_VERSION73→74 withMIN_COMPATIBLE_WIRE_VERSIONuntouched is the right call — nothing is removed, renamed, or re-meant, so no lockstep upgrade is forced. - Codex icon embedding looks sound. Extension→MIME allowlist, 32 KiB per-icon cap,
statbeforereadFile, andcapSkillIconPayloaddropping (not truncating) over-budget icons. The refresh-generation staleness re-check is correctly placed after the awaited icon reads, so a concurrent refresh can't emit a stale catalog. - opencode retitling traced end-to-end. Longest-sanitized-prefix matching for
sanitize(server)_sanitize(tool)is the right disambiguator given opencode's flat naming, and routing both live parts and permission asks throughopencodeMcpTitlekeeps the header consistent.AgentHistoryReadContextstaying adapter-local rather than on the wire is the right boundary. - Engine reorder is safe.
SessionStartOptionsResolvermoves aboveHistoryService, and bothcustomMcpandpluginsare already initialized at that point — no TDZ. - Checked and cleared: the desktop CSP already allows
img-src … data:so embedded icons render;--foregroundresolves to a real color so thecolor-mixchip style is valid; the un-guardedclient.config.get()calls are fine because the generated opencode client resolves{ error }rather than throwing (as the existing comment atopencode/adapter.ts:124documents) and both sites null-check.data?.mcp. ACTIVITY_RUN_GLUE_KEYmechanics verified against the new tests — glue items attach to an open run without opening one themselves, andflushRunresetsrunKeyso a trailing glue item can't leak into the next group.
ℹ️ .claude/rules/frontend.md now contradicts the dependency set
The renderer rule still reads:
Brand / agent icons:
@proj-airi/lobe-iconsviaunplugin-icons— chosen oversimple-icons, which at adoption time (2026-07) lacked usableclaudecode/opencodeglyphs after trademark removals.
This PR adds @iconify-json/simple-icons as a second brand-icon collection (catalog, root/desktop/webview manifests, eslint override, /// <reference> — all wired correctly). The wiring is fine; the guidance isn't. As written, the next contributor reaching for an integration glyph is told not to use the collection this PR just standardized on. Worth a line in that rule saying which collection owns which case — lobe-icons for agent/model brands, simple-icons for third-party MCP integrations, or whatever the intended split is.
Nitpicks (not blocking)
toolCallFromPartnow deriveskindfrom the retitled slug rather than the raw provider name.tool-utils.ts's updated JSDoc acknowledges that a server key containing__mis-splits — previously that was cosmetic (a wrong header label), but now the same mis-split also feedstoolKindFromName, so it can flip a call's kind and kind icon. Still low-impact, just a slightly wider blast radius than the JSDoc implies.HistoryService.injectedMcpServerNamesresolves today's injected server set and applies it to historical sessions. A server that was enabled when the session ran but has since been disabled won't retitle on a cold read. Probably acceptable — just noting the reads aren't point-in-time.- Skill icon reads fan out through an unbounded
Promise.all. Fine for realistic catalog sizes; only a concern if a workspace ever carries hundreds of skills.
Claude Opus ｜ 𝕏
| return <CircleXIcon className="size-3.5 shrink-0 text-destructive-foreground" />; | ||
| // A brand/plugin glyph keeps the call recognizable through failure; the red status label | ||
| // already carries the state. | ||
| return icon ?? <CircleXIcon className="size-3.5 shrink-0 text-destructive-foreground" />; |
There was a problem hiding this comment.
icon isn't only a brand glyph here, so this fallback is broader than the comment above it describes.
Two callers always supply an icon independent of branding:
question-call-item.tsx:40unconditionally passes<MessageCircleQuestionMarkIcon … />tool-call-item.tsx:111-117passes theToolCaseIcontoolbox glyph for every ToolSearch call
So a failed question call and a failed ToolSearch call now silently lose the red CircleXIcon. The statusLabel still renders "failed" in red, so the state isn't invisible — but it's a behavior change beyond the brand/plugin case the comment justifies.
If the intent is specifically "keep brand icons on failed calls" (per the commit title), gating on the brand path rather than on any icon would be tighter. Relatedly, the comment at tool-call-item.tsx:107-108 — "state glyphs still override inside ToolIcon" — is now stale for the failed state.
| <Chip style={commandBrandChipStyle(echoedCommand)} variant="info"> | ||
| <CommandBrandGlyph className="size-3.5" command={echoedCommand} />/{echo.name} | ||
| </Chip> | ||
| {echo.args ? <span className="ms-1.5">{echo.args}</span> : null} |
There was a problem hiding this comment.
The echo branch replaces all block rendering, not just the leading /name token, so multi-line or markdown arguments lose their formatting.
commandEcho accepts any single-block message starting with /, and echo.args is everything after the first whitespace — including newlines. Rendering that as a bare <span> means HTML collapses the whitespace, so /review followed by a pasted diff or a fenced code block comes out as one run-on line. And because it's now a single line, the collapsible clamp at line 175 (which keys off text.split('\n').length > 20) is computed on the original text but applied to content that no longer wraps the same way.
Two ways out, either is fine:
- Keep the chip for the
/nametoken but letContentBlockViewrender the remainder, so markdown/newlines survive. - Or restrict the echo path to single-line messages, falling back to normal block rendering otherwise.
The composer draft this mirrors is single-line by construction; the transcript echo isn't.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on the CODE-412 PR (base
ruocheng/code-412); only the 11 commits above it are new here.Codex plugin skills ship brand identity on
skills/list(interface.displayName/iconSmall/brandColor— verified live on codex 0.144.6); carry it through instead of dropping it (Closes CODE-525):AgentCommandfields (displayName,iconDataUri,brandColor); icons travel as data URIs, size-capped per icon and in aggregate per catalog.WIRE_PROTOCOL_VERSION73 → 74, floor untouched — old clients unaffected.mcp__<server>__<tool>slug, live and on cold history reads. Cold reads resolve server names from the session directory's config plus a new engine-suppliedmcpServerNameshint, so calls to engine-injected servers (managed connectors, the daemon's simulator endpoint) retitle too. The hint is adapter-local context (AgentHistoryReadContext), deliberately off the wire.Verification
pnpm check:ci(0 errors) and fullpnpm exec vitest run(2764 passed) at this tip.skills/listinterface fields probed against a livecodex app-server(0.144.6); findings recorded in CODE-525.sanitize(server)_sanitize(tool)join verified against anomalyco/opencode 1.18.15 source; cold reads use a pureconfig.get— nevermcp.status, whose lazy init spawns/dials every configured server (noted inreadHistory).history-service,start-options-mcp).Checklist
pnpm check:ciandpnpm testboth pass (pluscargo fmt/clippy/testfor Rust changes)WIRE_PROTOCOL_VERSIONis bumped (73 → 74)