Skip to content

docs: add developer guide for MCP Apps UI#23

Open
jstjoe wants to merge 3 commits into
mainfrom
claude/laughing-brahmagupta-HTfSF
Open

docs: add developer guide for MCP Apps UI#23
jstjoe wants to merge 3 commits into
mainfrom
claude/laughing-brahmagupta-HTfSF

Conversation

@jstjoe

@jstjoe jstjoe commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • New internal doc at docs/mcp-apps-ui.md describing the apps UI layer
  • Lists the tools that ship an inline app UI (de-identify, re-identify; notes the disabled de-identify_file)
  • Documents the SDK building blocks (App, PostMessageTransport, host-theming helpers, lifecycle hooks) and the shared theme/types/styles in ui/shared/
  • Includes a short "Adding a new app" checklist that points back to the modify-tool list in CLAUDE.md

Test plan

  • Skim docs/mcp-apps-ui.md for accuracy against current ui/ and src/server.ts
  • Confirm the component table matches classes actually defined in ui/shared/styles.css

Generated by Claude Code

Documents which tools ship an inline app UI, the SDK building blocks
used by each app, and the shared theme/types/styles in ui/shared.
@vercel

vercel Bot commented May 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sky-mcp-streamable Ready Ready Preview, Comment Jun 3, 2026 4:31pm

Request Review

@github-actions

Copy link
Copy Markdown

Gitleaks Findings: No secrets detected. Safe to proceed!

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown

Code Review

Docs-only change (+133/-0). I verified the new docs/mcp-apps-ui.md against ui/, src/server.ts, scripts/generate-ui-imports.ts, and ui/shared/styles.css. Overall the doc is well-organized and accurate to the current code; the issues below are mostly small inaccuracies a future contributor following the guide would trip on.

Bugs / inaccuracies in the doc

  1. "Adding a new app", step 5 points contributors at the wrong file.

    Register the new HTML import in src/generated/ui-html.js (generated from the Vite build) …

    src/generated/ui-html.ts is auto-generated and shouldn't be edited by hand (scripts/generate-ui-imports.ts:27 even emits an AUTO-GENERATED … do not edit banner). A contributor adding a new app actually needs to:

    • add an entry to the tools array in scripts/generate-ui-imports.ts:15-19
    • add the matching export declare const …Html: string; to src/generated/ui-html.d.ts
    • add the new INPUT=<dir>/mcp-app.html vite build step to the build:ui script in package.json

    That's the part most likely to bite someone — worth being explicit.

  2. Wrong file extension. The generated module is ui-html.ts (and ui-html.d.ts), not ui-html.js. The doc references .js in two places (the "Per-app anatomy" section and the "Adding a new app" checklist). The runtime import in src/server.ts:14 does use a .js specifier — that's the TS-with-NodeNext convention — but the source file on disk is .ts. Worth clarifying so readers don't go looking for a .js file.

  3. "imported as strings from src/generated/ui-html.js" misdescribes the pipeline. They're not imported — scripts/generate-ui-imports.ts:32-33 reads each dist/ui/<tool>/mcp-app.html file and inlines its contents into a TS template literal. The build flow is: build:ui (vite, per-entry) → build:ui-imports (the generator script) → build:server (tsc). A one-line callout to that order would make the section more useful.

Smaller things

  1. Component table omits three classes defined in ui/shared/styles.css: .card-grid (line 266), .file-viewer (line 307), and .entity-gallery (line 324). These are all currently file-tool-only, so it's defensible to leave them out — but since de-identify_file is listed as "UI exists, tool disabled" in the very first table, a future contributor re-enabling that tool would benefit from knowing these exist. Either list them with a "(file tool)" annotation or add a sentence noting they're reserved for the disabled file UI.

  2. DeIdentifyResult type description is slightly out of date relative to the server schema. The doc accurately describes ui/shared/types.ts:12-20, but the server outputSchema in src/server.ts:99-124 also returns error, code, message, details on the de-identify path (and helpUrl for re-identify). The "When you change the tool output schema in src/server.ts, update the matching type here" callout is correct, but the shared type itself is already missing those fields — so a strict reader following the doc could conclude the UI types are in sync when they actually aren't. Worth either updating ui/shared/types.ts or noting the drift in the doc.

  3. Minor: the re-identify UI uses the side-by-side .panels grid rather than tabs (see ui/re-identify/main.ts:214-223), while de-identify uses tabs. The doc mentions the tab pattern only in the context of de-identify, which is fine — just calling it out in case you want to make the "two layout patterns" choice explicit for new-app authors.

Things that check out

  • Tool table (registered vs. disabled), resource URIs, and _meta.ui.resourceUri wiring all match src/server.ts:69-79, 125, 159.
  • The per-app anatomy (mcp-app.html shell + main.ts) and the lifecycle hook list (ontoolinput, ontoolresult, onhostcontextchanged, onteardown, connect) match both ui/de-identify/main.ts and ui/re-identify/main.ts exactly.
  • setupHostTheming / applyInitialContext descriptions match ui/shared/theme.ts:9-28.
  • All eleven shared CSS classes listed in the component table exist in ui/shared/styles.css.
  • getEntityClass() convention (entity-<lowercased_snake_case>) matches both ui/de-identify/main.ts:26-29 and ui/re-identify/main.ts:35-38.
  • The pointer back to the "Modifying Tools" checklist in CLAUDE.md is correct and useful.

Other considerations

  • Security: N/A — docs only.
  • Performance: N/A — docs only.
  • Tests: No test changes; none needed for a doc addition, though it might be worth a tiny check in CI that the tool table in this doc and the registrations in src/server.ts don't drift (out of scope for this PR).

Nice addition — once the ui-html.ts generation note is fixed, this becomes a solid landing page for the apps layer.

Address review feedback on PR #23:
- Describe the three-step build pipeline (build:ui → build:ui-imports
  → build:server) and clarify that src/generated/ui-html.ts is
  auto-generated, with the .ts/.js specifier distinction explained.
- Rewrite the "Adding a new app" step that pointed at the generated
  file: contributors now edit package.json's build:ui, the tools
  array in scripts/generate-ui-imports.ts, and ui-html.d.ts.
- List the file-tool-only CSS classes (.card-grid, .file-viewer,
  .entity-gallery) so a future de-identify_file revival has the
  reference.
- Note the known drift between server outputSchema error fields and
  ui/shared/types.ts.
- Document the two available layout patterns (tabs vs. side-by-side
  panels) using the existing apps as examples.
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Gitleaks Findings: No secrets detected. Safe to proceed!

@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review: docs: add developer guide for MCP Apps UI

Thanks for this — it's a genuinely useful internal guide, and I verified the claims against the current ui/, src/server.ts, scripts/generate-ui-imports.ts, and package.json. The vast majority is accurate, including some easy-to-get-wrong details. A few notes below, with one real correction.

✅ Verified accurate

  • Tools table — resource URIs (ui://de-identify/mcp-app.html, ui://re-identify/mcp-app.html), registration via registerAppResource/registerAppTool, and de-identify_file being UI-present-but-unregistered all match src/server.ts (imports only deIdentifyHtml, reIdentifyHtml).
  • Three-step build pipelinebuild:ui (one Vite run per entry) → build:ui-imports (generate-ui-imports.ts inlining into src/generated/ui-html.ts) → build:server. The .ts generated / .d.ts ambient declaration / .js import-specifier distinction is all correct, and ui-html.ts is indeed auto-generated.
  • ui/shared/types.ts field lists for all four result interfaces match exactly.
  • CSS classes — every class in both tables exists in ui/shared/styles.css, including the three file-tool-only ones (.card-grid, .file-viewer, .entity-gallery), which are referenced only by the disabled ui/de-identify-file/main.ts. Nice catch listing those.
  • Known-drift note — confirmed: server outputSchema declares error/code/message/details (+helpUrl on re-identify) while ui/shared/types.ts only mirrors error/message/anonymousModeRestricted. Accurate and a helpful flag.
  • Layout patternsde-identify uses .tab-bar/.tab-content; re-identify uses a .panels grid of two .panels. Correct.
  • Lifecycle hooks (ontoolinput, ontoolresult, onteardown returning {}), new App({ name, version }), and app.connect(new PostMessageTransport()) then applyInitialContext — all match both main.ts files. setupHostTheming/applyInitialContext in theme.ts are described correctly.

🔧 One correction

"SDK building blocks" table is mis-attributed. It says "Each main.ts follows the same shape, pulling from @modelcontextprotocol/ext-apps" and then lists applyDocumentTheme, applyHostStyleVariables, applyHostFonts, and McpUiHostContext. In reality the two main.ts files import only App and PostMessageTransport from the SDK — the three applyHost*/applyDocumentTheme helpers and the McpUiHostContext type are imported and used exclusively in ui/shared/theme.ts. Suggest splitting the table (or adding a "defined/used in" column) so readers don't go looking for these imports in main.ts.

💬 Minor nits (non-blocking)

  • The "Lifecycle hooks each app sets" list includes app.onhostcontextchanged(ctx), but the apps don't set that directly — setupHostTheming(app) registers it inside theme.ts. You do explain this later under the theme.ts section, so it's just a small internal tension; consider noting "(registered via setupHostTheming)" inline.
  • The guide isn't linked from README.md or CLAUDE.md. A one-line pointer from CLAUDE.md's "MCP Apps UI Layer" section would make it discoverable (and keep the two from silently diverging).
  • Since this doc duplicates details that live in code (build script steps, class names, type fields), it's the kind of thing that drifts. Not actionable here, but worth keeping the "Modifying Tools" checklist in CLAUDE.md as the source of truth and treating this as the narrative companion.

No bugs, security, or performance concerns — it's docs-only and doesn't touch the build. Test plan items (skim for accuracy, verify class table) are essentially satisfied by the verification above. Once the SDK-building-blocks attribution is fixed, this is good to merge. 👍

Address second-round review feedback on PR #23:
- Split the "SDK building blocks" table into per-tool main.ts imports
  (App, PostMessageTransport) and the helpers used only in
  ui/shared/theme.ts (applyDocumentTheme, applyHostStyleVariables,
  applyHostFonts, McpUiHostContext). The previous single table
  implied every symbol was imported in main.ts.
- Note that onhostcontextchanged is registered indirectly by
  setupHostTheming, not by the per-tool main.ts directly.
- Add a pointer to docs/mcp-apps-ui.md from CLAUDE.md's
  "MCP Apps UI Layer" section so the guide is discoverable.
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Gitleaks Findings: No secrets detected. Safe to proceed!

@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review: docs: add developer guide for MCP Apps UI

Reviewed the new docs/mcp-apps-ui.md and the one-line CLAUDE.md pointer against the actual ui/, scripts/, src/server.ts, and package.json. This is a docs-only PR (+190/−0), so I focused on factual accuracy — the main risk for a guide like this is silent drift from the code it describes.

Accuracy — verified ✅

I cross-checked nearly every concrete claim, and they hold:

  • Tool table — resource URIs (ui://de-identify/mcp-app.html, ui://re-identify/mcp-app.html) match src/server.ts:69-70; de-identify_file UI exists but is unregistered. ✅
  • Build pipelinebuild:ui → build:ui-imports → build:server matches package.json:8-13; src/generated/ contains only the hand-written ui-html.d.ts, with ui-html.ts emitted by scripts/generate-ui-imports.ts. The .js-specifier-resolves-to-generated-.ts note is correct. ✅
  • SDK import attributionmain.ts imports exactly App + PostMessageTransport (ui/*/main.ts:1); the theming symbols (applyDocumentTheme, applyHostStyleVariables, applyHostFonts, McpUiHostContext) are imported only in ui/shared/theme.ts:1-7. The split-table fix from round 2 is accurate. ✅
  • Lifecycle hooksontoolinput/ontoolresult/onteardown set directly in each main.ts; onhostcontextchanged registered indirectly via setupHostTheming (theme.ts:10). ✅
  • Shared typesEntityInfo/DeIdentifyResult/ReIdentifyResult/DeIdentifyFileResult shapes match ui/shared/types.ts. ✅
  • CSS class table — every listed class (incl. compound .tab.active, .tab-content.active, .loading .spinner, and the file-tool-only .card-grid/.file-viewer/.entity-gallery) is defined in ui/shared/styles.css. ✅
  • "Known drift" note — confirmed: de-identify outputSchema declares error/code/message/details and re-identify adds helpUrl (server.ts:120-157), none of code/details/helpUrl mirrored in types.ts. Nice that this surfaced a real inconsistency.
  • Layout patternsde-identify uses .tab-bar, re-identify uses .panels and no tabs. ✅

This is unusually well-verified documentation — the iterative review rounds clearly tightened it.

Minor nits (non-blocking)

  1. Slightly imprecise sentence in the "Known drift" callout. It says the UIs "read these via the optional `error` / `message` fields they do declare." That's true for `ReIdentifyResult`, but `DeIdentifyResult` declares neither `error` nor `message` — so the clause reads as if it applies to both result types. Consider scoping it to re-identify.

  2. Maintenance cost of the exhaustive tables. The full CSS-class list and per-step build commands duplicate content that lives in `styles.css` and `package.json` — exactly what's most likely to go stale. Two light options: (a) frame the class table as illustrative ("common components") rather than exhaustive, or (b) since the doc already documents the type drift, a follow-up that fixes it (aligning `types.ts` with the schemas) may be more durable than documenting it. Not for this PR.

  3. Discoverability is good — the `CLAUDE.md` pointer is the right call so the guide isn't orphaned.

Verdict

No bugs, no security/performance concerns (docs only), and no test impact. Accurate, well-structured, and discoverable. The nits above are optional polish. LGTM. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants