feat: Add MCP Apps (ext-apps) support #224
Conversation
📝 WalkthroughWalkthroughAdds MCP Apps (ext-apps, SEP-1865): new protocol types, JSON‑RPC handler, adapter and bridge methods (model-context, logging, tool register/unregister), MIME/display-mode utilities, UI config/schema fields, transport flow integration, extensive tests, and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant W as Widget (ext-app)
participant I as IIFE / ExtAppsAdapter
participant H as ExtAppsMessageHandler
participant B as Bridge / HostContext
participant S as Server / Tool runtime
W->>I: postMessage(JSON-RPC request e.g. ui/callServerTool)
I->>I: validate origin / handshake
I->>H: handleRequest(request)
H->>H: route method, validate hostCapabilities
H->>B: delegate operation (callTool / updateModelContext / registerTool / ...)
B->>S: execute operation (tool run, update state, open link)
S-->>B: result or error
B-->>H: return result
H->>I: JSON-RPC response or error
I->>W: postMessage(response)
S->>I: notification (ui/notifications/tool-input-partial or cancelled)
I->>W: emit notification event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/tool/flows/tools-list.flow.ts (1)
478-516: ScoperesourceUrioverride to non-OpenAI platforms only.
resourceUriis designed for ext-apps/MCP Apps per SEP-1865 and should not override OpenAI'sopenai/outputTemplate. ThebuildToolDiscoveryMetafunction (the reference implementation) does not applyresourceUrito OpenAI, and E2E tests validate thatopenai/outputTemplateuses only the auto-generatedui://widget/...format. Apply the suggested guard to ensure OpenAI always receives the auto-generated URI:Guard to fix resourceUri scope
- const widgetUri = uiConfig.resourceUri || `ui://widget/${encodeURIComponent(finalName)}.html`; + const defaultWidgetUri = `ui://widget/${encodeURIComponent(finalName)}.html`; + const widgetUri = isOpenAIPlatform ? defaultWidgetUri : (uiConfig.resourceUri ?? defaultWidgetUri);
🤖 Fix all issues with AI agents
In `@docs/draft/docs/ui/advanced/platforms.mdx`:
- Around line 149-158: The example discovery response uses a flattened key
"toolListChanged" but the SDK emits a nested shape under
"ui/capabilities.tools.listChanged"; update the JSON example so
"ui/capabilities" contains a "tools" object with a "listChanged" boolean (keep
"supportsPartialInput" as-is under "ui/capabilities") to match the actual
emitted shape (i.e., replace "toolListChanged" with "tools": { "listChanged":
... } under "ui/capabilities").
In `@docs/draft/docs/ui/integration/tools.mdx`:
- Around line 219-225: The example has a mismatch between
widgetCapabilities.toolListChanged and its comment: currently toolListChanged is
false but the comment says the widget can emit tool list changes; update the
example by either setting toolListChanged: true if the intent is to indicate the
widget can emit tool list changes, or change the comment to "Widget cannot emit
tool list changes" to match toolListChanged: false; locate this in the ui: {
template: MyWidget, widgetCapabilities: { ... } } block and make the value and
comment consistent.
In `@libs/sdk/src/ext-apps/README.md`:
- Around line 148-172: The README example imports ExtAppsMessageHandler and
createExtAppsMessageHandler from a non-exported subpath; update the import to
pull these symbols from the package main entry instead (import
ExtAppsMessageHandler and createExtAppsMessageHandler from '@frontmcp/sdk'),
since ext-apps is re-exported via the main index and the subpath
'@frontmcp/sdk/ext-apps' is not configured in package.json.
In `@libs/ui/src/bridge/adapters/__tests__/ext-apps.adapter.test.ts`:
- Around line 1-185: Add unit tests in this file for the four public
ExtAppsAdapter methods updateModelContext, log, registerTool, and
unregisterTool: for each method include success-path tests that simulate the
host exposing the corresponding capability on the window (e.g., mock the host
API the adapter calls so updateModelContext resolves,
registerTool/unregisterTool call succeed, and log calls the host logger),
error-path tests where the host capability is absent or throws (assert the
adapter throws or returns the expected error), and for log include a test where
host logging is unavailable so the method falls back to console logging; locate
the adapter via ExtAppsAdapter and mock window properties/host functions used
inside updateModelContext, log, registerTool, and unregisterTool to trigger each
branch.
In `@libs/uipack/src/adapters/platform-meta.constants.ts`:
- Around line 30-37: Add unit tests for the exported function mapDisplayMode to
cover standard modes, aliases and unknown inputs: assert that "inline",
"fullscreen", and "pip" map to the expected ExtAppsDisplayMode values (use the
same expected enum/constant values used by DISPLAY_MODE_MAP), assert that
aliases "widget" maps to the same value as "inline" and "panel" maps to the same
value as "fullscreen", and assert that unknown/invalid inputs (e.g.,
"unknownMode", empty string, and a non-matching value) return undefined; locate
mapDisplayMode and DISPLAY_MODE_MAP in platform-meta.constants.ts to reference
the exact expected outputs in your assertions.
In `@libs/uipack/src/bridge-runtime/iife-generator.ts`:
- Around line 367-376: The originTrustPending flag is set to true when
attempting to establish trustedOrigin but is never reset, which can block future
valid messages; update the logic around trustedOrigin and originTrustPending so
that originTrustPending is reset to false after the trust attempt completes —
set originTrustPending = false when trust is successfully established (after
assigning trustedOrigin) and also reset it on failure paths (e.g., when origin
is falsy or window.parent === window) or implement a short timeout fallback to
clear originTrustPending; modify the block handling
trustedOrigin/originTrustPending (the variables trustedOrigin and
originTrustPending in this module) accordingly.
🧹 Nitpick comments (8)
libs/sdk/src/ext-apps/README.md (1)
165-168: Consider adding type annotations to the handler context example.The example shows
callToolreturningPromise<unknown>via the flows call, but the actual type information is lost. Consider showing the typed return for clarity.libs/ui/src/bridge/adapters/__tests__/ext-apps.adapter.test.ts (1)
177-184: Incomplete SSR test - consider removing or implementing properly.This test doesn't actually verify SSR behavior since jsdom always provides a
windowobject. The assertionexpect(typeof window).toBe('object')just confirms jsdom is working, not that the adapter handles SSR correctly.Consider either:
- Removing this test (the behavior is implicitly tested by the code structure)
- Using a separate test environment without jsdom for true SSR testing
♻️ Option 1: Remove the incomplete test
- describe('canHandle in SSR context', () => { - it('should return false when window is undefined', () => { - // We can't easily mock window being undefined in jsdom, - // but we can verify the first check in the method - expect(typeof window).toBe('object'); - // The method should handle window being undefined gracefully - }); - });libs/ui/src/bridge/types.ts (1)
364-365: Import ExtAppsDisplayMode from SDK for consistency with ext-apps type definitions.The
displayModesfield inExtAppsInitializeResultuses the inline union('inline' | 'fullscreen' | 'pip')[]. SinceExtAppsDisplayModeis exported fromlibs/sdk/src/ext-apps/ext-apps.types.tswith the identical definition, import and reuse it to maintain consistency across the codebase and with the SEP-1865 ext-apps protocol specification.libs/sdk/src/ext-apps/__tests__/ext-apps.handler.test.ts (1)
310-345: Consider testing all log levels individually with assertions.The loop at lines 320-330 iterates through log levels but doesn't verify that the correct logger method was called for each level. Consider adding explicit assertions.
♻️ Optional: Add explicit logger call verification
for (const level of levels) { const response = await handler.handleRequest( createRequest('ui/log', { level, message: `Test ${level} message`, data: { extra: 'data' }, }), ); expect(response.error).toBeUndefined(); + // Verify logger was called with correct level + expect(mockLogger[level]).toHaveBeenCalledWith( + expect.stringContaining(`Test ${level} message`), + expect.objectContaining({ extra: 'data' }) + ); }libs/ui/src/bridge/adapters/ext-apps.adapter.ts (1)
240-246: Use ExtAppsNotSupportedError instead of generic Error for capability checks.Per coding guidelines, adapters should use specific error classes with MCP error codes. Replace generic
ErrorwithExtAppsNotSupportedErrorfor better error categorization and consistency with the SDK pattern.The error class is exported from
@frontmcp/sdkand already used in the same pattern throughoutlibs/sdk/src/ext-apps/ext-apps.handler.ts. Apply this to all four capability check failures:
- Line 196 in
callTool(): Server tool proxy check- Line 242 in
updateModelContext(): Model context update check- Line 281 in
registerTool(): Widget tool registration check- Line 296 in
unregisterTool(): Widget tool unregistration checkAdd the import:
import { ExtAppsNotSupportedError } from '@frontmcp/sdk'libs/sdk/src/ext-apps/ext-apps.handler.ts (3)
136-151: Consider omitting stack traces in production error responses.Line 148 exposes
error.stackin JSON-RPC error responses. While useful for debugging, stack traces can leak internal implementation details to untrusted widgets.♻️ Suggested approach
return { jsonrpc: '2.0', id, error: { code: errorCode, message: errorMessage, - data: error instanceof Error ? { stack: error.stack } : undefined, + // Omit stack in production to avoid leaking internal details + data: process.env.NODE_ENV === 'development' && error instanceof Error + ? { stack: error.stack } + : undefined, }, };
331-360: Consider validating log level against known values.The
handleLogmethod accepts anylevelvalue and falls through toinfofor unknown levels. While this is resilient, explicitly validating againstExtAppsLogLevelvalues would catch misuse early.♻️ Optional: Add level validation
private async handleLog(params: ExtAppsLogParams): Promise<void> { if (!this.hostCapabilities.logging) { throw new ExtAppsNotSupportedError('Logging not supported by host'); } const { level, message, data } = params; if (!message || typeof message !== 'string') { throw new ExtAppsInvalidParamsError('Log message is required'); } + const validLevels = ['debug', 'info', 'warn', 'error']; + if (level && !validLevels.includes(level)) { + throw new ExtAppsInvalidParamsError(`Invalid log level '${level}'. Must be one of: ${validLevels.join(', ')}`); + } const widgetLogger = this.logger.child(`Widget:${this.context.sessionId}`);
383-385: Consider stricter validation forinputSchema.The check
typeof inputSchema !== 'object'would pass for arrays andnull. For a JSON Schema, you likely want a plain object.♻️ Stricter schema validation
- if (!inputSchema || typeof inputSchema !== 'object') { + if (!inputSchema || typeof inputSchema !== 'object' || Array.isArray(inputSchema)) { throw new ExtAppsInvalidParamsError('Tool input schema is required'); }
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@libs/sdk/src/ext-apps/__tests__/ext-apps.handler.test.ts`:
- Around line 19-33: The mockLogger in this test uses an unsafe `as any` cast
and is missing the `debug` method and the correct shape expected by
FrontMcpLogger; update the mock to include debug (jest.fn()) and make its shape
conform to FrontMcpLogger (or create a MockFrontMcpLogger type) and remove the
`as any` cast in createMockContext; specifically modify the mockLogger
declaration and then return it from createMockContext typed as FrontMcpLogger
(or the dedicated mock type) so ExtAppsHandlerContext receives a properly typed
logger without using `as any`, keeping identifiers mockLogger,
createMockContext, ExtAppsHandlerContext and FrontMcpLogger in mind.
In `@libs/sdk/src/ext-apps/ext-apps.handler.ts`:
- Around line 33-52: Make ExtAppsHandlerContext generic over a tool result type
(e.g. <TToolResult = unknown>) and change callTool from returning
Promise<unknown> to Promise<TToolResult>; then propagate that generic parameter
through ExtAppsMessageHandlerOptions, ExtAppsMessageHandler, and the
createExtAppsMessageHandler factory so handler implementations and callers can
type tool responses; update all referenced type signatures
(ExtAppsHandlerContext, ExtAppsMessageHandlerOptions, ExtAppsMessageHandler,
createExtAppsMessageHandler) to accept the TToolResult generic and use it for
callTool return types and any handler return types.
- Around line 159-195: routeMethod currently forwards possibly null/undefined
params to handlers which destructure them and cause TypeError that gets mapped
to INTERNAL_ERROR; add a small guard (e.g., isParamsObject(params) or
validateParamsObject) at the top of routeMethod to check that params is a
non-null object (and if needed an array check depending on handler expectations)
and if the check fails throw or return an MCP-compliant InvalidParams error
(INVALID_PARAMS) before switching to handlers; update routeMethod to call this
guard prior to the switch and mention the affected handlers (handleInitialize,
handleUpdateModelContext, handleOpenLink, handleSetDisplayMode, handleClose,
handleLog, handleRegisterTool, handleUnregisterTool) so they never receive
null/undefined params.
🧹 Nitpick comments (1)
libs/ui/src/bridge/adapters/ext-apps.adapter.ts (1)
530-552: Tighten initialize response validation to reject arrays.
hostCapabilitiesandhostContextshould be objects, not arrays; addingArray.isArraychecks prevents malformed responses from passing validation.♻️ Suggested tweak
- if ( - r['hostCapabilities'] !== undefined && - (typeof r['hostCapabilities'] !== 'object' || r['hostCapabilities'] === null) - ) { + if ( + r['hostCapabilities'] !== undefined && + (typeof r['hostCapabilities'] !== 'object' || r['hostCapabilities'] === null || Array.isArray(r['hostCapabilities'])) + ) { return false; } - if (r['hostContext'] !== undefined && (typeof r['hostContext'] !== 'object' || r['hostContext'] === null)) { + if ( + r['hostContext'] !== undefined && + (typeof r['hostContext'] !== 'object' || r['hostContext'] === null || Array.isArray(r['hostContext'])) + ) { return false; }
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (1)
147-154: Add runtime type guard before callingstartsWithonmethod.Optional chaining (
?.) only protects againstnullandundefined, not non-string values. A malformed request withmethod: 123ormethod: []will crash withmethod.startsWith is not a function, causing a 500 error instead of a proper RPC error. SincehttpInputSchemadoesn't validate request body structure, runtime type safety is necessary.Suggested fix
- } else if (method?.startsWith('ui/')) { + } else if (typeof method === 'string' && method.startsWith('ui/')) { // Ext-apps methods: ui/initialize, ui/callServerTool, etc. this.state.set('requestType', 'extApps');
🤖 Fix all issues with AI agents
In `@libs/sdk/src/transport/flows/handle.streamable-http.flow.ts`:
- Around line 491-509: The callTool implementation currently returns the raw
result when CallToolResultSchema.safeParse fails; instead, follow the pattern in
call-tool-request.handler.ts and throw an InternalMcpError so ExtApps will
convert it to a JSON-RPC error. Update the callTool async function to run
CallToolResultSchema.safeParse(result) and if parsed.success is false, throw a
new InternalMcpError with a short message and include parse error details (and
optionally the original result) in the error metadata; keep the existing return
of parsed.data when successful.
🧹 Nitpick comments (2)
libs/sdk/src/common/types/options/ext-apps/schema.ts (1)
60-60: Consider extracting the displayModes enum values to a shared constant.The literal values
'inline','fullscreen','pip'are duplicated across the codebase. While anExtAppsDisplayModetype already exists inlibs/sdk/src/ext-apps/ext-apps.types.ts, consider extracting the literal values themselves as a constant (e.g.,const DISPLAY_MODES = ['inline', 'fullscreen', 'pip'] as const) that can be reused in this schema and other locations likeext-apps.handler.ts.libs/sdk/src/transport/flows/__tests__/handle.streamable-http.ext-apps.test.ts (1)
68-96: Consider exercising router behavior instead of string primitives.These checks only validate
startsWith('ui/')rather than the actual router stage; a small helper or light router invocation would make this test more meaningful and resilient to routing logic changes.
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.