Skip to content

feat(ext-apps-support): Update ext-apps library to version 0.4.0 - #96

Merged
gabrypavanello merged 10 commits into
mainfrom
claude/update-ext-apps-0.4.0-0g15K
Jan 14, 2026
Merged

feat(ext-apps-support): Update ext-apps library to version 0.4.0#96
gabrypavanello merged 10 commits into
mainfrom
claude/update-ext-apps-0.4.0-0g15K

Conversation

@gabrypavanello

Copy link
Copy Markdown
Contributor

Add support for ext-apps v0.4.0 features:

  • Add updateModelContext() method to inform AI model about app state without triggering follow-up actions
    • MCP Apps: Uses native protocol feature
    • ChatGPT: Uses setState/setWidgetState (which exposes to AI context)
  • Add containerDimensions type for new viewport semantics (fixed vs flexible)
  • Add new HostCapabilities: updateModelContext, message, sandbox
  • Add useUpdateModelContext() React hook for model context updates
  • Add tests for all new functionality

Breaking changes in ext-apps handled:

  • containerDimensions replaces viewport (we derive viewport for backward compat)
  • New capability types for content block modalities

Note: On ChatGPT, both setState and updateModelContext expose state to the AI model. Use setState for persistence-focused use cases, updateModelContext for context-focused use cases.

@coderabbitai

coderabbitai Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Push model-context updates to the host via a new client API and UI hook
    • Flexible container sizing (containerDimensions) with derived viewport handling
    • New content block types and expanded host capabilities for messaging and sandboxing
    • New error code for invalid parameters
  • Chores

    • Dependency version bumps and build-tool dependency adjustments
  • Tests

    • Unit tests covering model-context updates and container-dimensions behavior

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds a Model Context API (AppsClient.updateModelContext + useUpdateModelContext), new types (ContainerDimensions, ContentBlock, UpdateModelContextParams), containerDimensions-driven viewport derivation in the MCP adapter, implementations of updateModelContext in MCP/OpenAI/Mock adapters, dependency/version updates, tests, build config adjustments, and a new UI error code INVALID_PARAMS.

Changes

Cohort / File(s) Summary
Dependency updates
examples/minimal/package.json, examples/restaurant-finder/package.json, examples/weather-app/package.json, packages/core/package.json, packages/testing/package.json, packages/ui/package.json, packages/ui-react-builder/package.json
Bumped zod to ^4.3.5 across examples/packages; bumped @modelcontextprotocol/sdk to ^1.25.2; updated @modelcontextprotocol/ext-apps to ^0.4.0; reorganized/added @typescript-eslint/* and esbuild deps in ui-react-builder.
Public types & exports
packages/ui/src/types.ts, packages/ui/src/index.ts, packages/ui-react/src/index.ts
Added ContainerDimensions, ContentBlock, UpdateModelContextParams; extended HostCapabilities and HostContext (containerDimensions); exported new types and useUpdateModelContext hook.
Adapter surface & behavior
packages/ui/src/adapters/mcp.ts, packages/ui/src/adapters/openai.ts, packages/ui/src/adapters/mock.ts, packages/ui/src/adapters/types.ts
Added updateModelContext(params) to ProtocolAdapter and implemented in MCP/OpenAI/Mock; MCP: containerDimensions handling and deriveViewportFromContainerDimensions; MCP/OpenAI: build/forward modelContext and text summary; Mock: store/get lastModelContext.
Client & React API
packages/ui/src/client.ts, packages/ui-react/src/hooks.ts
Added AppsClient.updateModelContext() delegating to adapter and useUpdateModelContext() hook that calls it and no-ops if client missing.
Tests
packages/ui/tests/unit/mcp-adapter.test.ts, packages/ui/tests/unit/mock-adapter.test.ts, packages/ui/tests/unit/openai-adapter.test.ts
Added tests for updateModelContext behavior, containerDimensions handling, OpenAI setWidgetState interactions, and Mock adapter storage/logging.
Build config
packages/ui-react-builder/tsup.config.ts
Marked @typescript-eslint/typescript-estree and @typescript-eslint/types as externals to avoid bundling.
Misc & examples
packages/ui/src/errors.ts, examples/weather-app/server/index.ts
Added INVALID_PARAMS UI error code; changed weather example protocol from "openai" to "mcp".

Sequence Diagram(s)

sequenceDiagram
  participant App as App (consumer)
  participant Hook as useUpdateModelContext
  participant Client as AppsClient
  participant Adapter as ProtocolAdapter (MCP / OpenAI / Mock)
  participant Host as Host/SDK (ext-apps / window.openai)
  App->>Hook: call updateModelContext(params)
  Hook->>Client: client.updateModelContext(params)
  Client->>Adapter: adapter.updateModelContext(params)
  Adapter->>Host: forward structured modelContext / setWidgetState / SDK call
  Host-->>Adapter: ack / no-op
  Adapter-->>Client: resolve
  Client-->>Hook: resolve
  Hook-->>App: Promise resolved
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective of the PR: updating the ext-apps library to version 0.4.0, which encompasses all the significant changes (updateModelContext, containerDimensions, new HostCapabilities, and React hook).
Description check ✅ Passed The description comprehensively relates to the changeset, documenting key features added (updateModelContext method, containerDimensions type, new HostCapabilities, React hook), implementation details across MCP Apps and ChatGPT adapters, test additions, and breaking changes handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@gabrypavanello
gabrypavanello changed the base branch from dependabot/npm_and_yarn/production-dependencies-2bc629d457 to main January 13, 2026 15:51
dependabot Bot and others added 2 commits January 13, 2026 16:51
…y with 5 updates

Bumps the production-dependencies group with 5 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [@modelcontextprotocol/sdk](https://github.com/modelcontextprotocol/typescript-sdk) | `1.25.1` | `1.25.2` |
| [zod](https://github.com/colinhacks/zod) | `4.2.1` | `4.3.5` |
| [@modelcontextprotocol/ext-apps](https://github.com/modelcontextprotocol/ext-apps) | `0.2.2` | `0.4.0` |
| [@typescript-eslint/typescript-estree](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-estree) | `8.50.1` | `8.53.0` |
| [esbuild](https://github.com/evanw/esbuild) | `0.25.12` | `0.27.2` |



Updates `@modelcontextprotocol/sdk` from 1.25.1 to 1.25.2
- [Release notes](https://github.com/modelcontextprotocol/typescript-sdk/releases)
- [Commits](modelcontextprotocol/typescript-sdk@1.25.1...v1.25.2)

Updates `zod` from 4.2.1 to 4.3.5
- [Release notes](https://github.com/colinhacks/zod/releases)
- [Commits](colinhacks/zod@v4.2.1...v4.3.5)

Updates `@modelcontextprotocol/ext-apps` from 0.2.2 to 0.4.0
- [Release notes](https://github.com/modelcontextprotocol/ext-apps/releases)
- [Changelog](https://github.com/modelcontextprotocol/ext-apps/blob/main/RELEASES.md)
- [Commits](modelcontextprotocol/ext-apps@v0.2.2...v0.4.0)

Updates `@typescript-eslint/typescript-estree` from 8.50.1 to 8.53.0
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-estree/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.53.0/packages/typescript-estree)

Updates `esbuild` from 0.25.12 to 0.27.2
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md)
- [Commits](evanw/esbuild@v0.25.12...v0.27.2)

---
updated-dependencies:
- dependency-name: "@modelcontextprotocol/sdk"
  dependency-version: 1.25.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: production-dependencies
- dependency-name: zod
  dependency-version: 4.3.5
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: production-dependencies
- dependency-name: "@modelcontextprotocol/ext-apps"
  dependency-version: 0.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: production-dependencies
- dependency-name: "@typescript-eslint/typescript-estree"
  dependency-version: 8.53.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: production-dependencies
- dependency-name: esbuild
  dependency-version: 0.27.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: production-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
Add support for ext-apps v0.4.0 features:

- Add updateModelContext() method to inform AI model about app state
  without triggering follow-up actions
  - MCP Apps: Uses native protocol feature
  - ChatGPT: Uses setState/setWidgetState (which exposes to AI context)
- Add containerDimensions type for new viewport semantics (fixed vs flexible)
- Add new HostCapabilities: updateModelContext, message, sandbox
- Add useUpdateModelContext() React hook for model context updates
- Add tests for all new functionality

Breaking changes in ext-apps handled:
- containerDimensions replaces viewport (we derive viewport for backward compat)
- New capability types for content block modalities

Note: On ChatGPT, both setState and updateModelContext expose state to the
AI model. Use setState for persistence-focused use cases, updateModelContext
for context-focused use cases.
@gabrypavanello
gabrypavanello force-pushed the claude/update-ext-apps-0.4.0-0g15K branch from f0f2aa5 to b47ee88 Compare January 13, 2026 15:51
@gabrypavanello

Copy link
Copy Markdown
Contributor Author

@claude Document the relationship - Make it clear that on ChatGPT, updateModelContext uses setState internally

@claude

claude Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @gabrypavanello's task in 2m 27s —— View job


Documentation task completed ✓

  • Read current implementation in openai.ts adapter
  • Read type definitions in types.ts
  • Read React hook implementation
  • Add documentation clarifying setState relationship on ChatGPT
  • Commit and push changes

Changes Made

Updated documentation in three files to clarify the relationship between updateModelContext() and setState() on ChatGPT:

  1. packages/ui/src/adapters/openai.ts (line 529-543) - Updated implementation comment to explain that updateModelContext() uses setState() internally, which calls OpenAI SDK's setWidgetState().

  2. packages/ui/src/types.ts (line 539-572) - Updated JSDoc to explain platform differences and clarify that on ChatGPT, both methods expose state to the AI model.

  3. packages/ui-react/src/hooks.ts (line 258-293) - Updated useUpdateModelContext() hook documentation with same clarifications.

All documentation now clearly states:

  • MCP Apps: Uses native protocol feature for pure context updates
  • ChatGPT: Uses setState/setWidgetState internally (exposes to AI + persists)
  • Important: On ChatGPT, both setState() and updateModelContext() expose state to the AI model

The key message is now clear: developers should use setState() for persistence-focused use cases and updateModelContext() for context-focused use cases, with the understanding that on ChatGPT, both methods expose state to the AI model.

Commit

Pushed commit 1ff5756 to the PR branch with the documentation updates.

--- | Branch: claude/update-ext-apps-0.4.0-0g15K

Document the relationship between updateModelContext() and setState()
on ChatGPT platform:

- On MCP Apps: Uses native protocol feature for pure context updates
- On ChatGPT: Uses setState/setWidgetState internally (exposes to AI + persists)
- Both methods expose state to AI model on ChatGPT

Updated documentation in:
- packages/ui/src/adapters/openai.ts - Implementation comment
- packages/ui/src/types.ts - AppsClient interface JSDoc
- packages/ui-react/src/hooks.ts - useUpdateModelContext hook JSDoc

Co-authored-by: Gabe <gabrypavanello@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @packages/ui-react-builder/package.json:
- Around line 58-59: The package.json currently lists
"@typescript-eslint/typescript-estree" and "esbuild" under "dependencies" which
are build-time tools; move both entries into "devDependencies" (remove them from
"dependencies" and add them under "devDependencies") so the published package
only contains runtime deps referenced by the "files" exports; after updating
package.json run your package manager install to update lockfiles and verify
build/lint configs are compatible with esbuild 0.27 and typescript-eslint v8
(adjust parserOptions or esbuild usage if needed).

In @packages/ui/src/adapters/openai.ts:
- Around line 539-564: The updateModelContext function currently merges
params.structuredContent into modelContext which can overwrite reserved
properties like _type and _textContent; before calling Object.assign in
updateModelContext, check params.structuredContent for reserved
underscore-prefixed keys (e.g., "_type", "_textContent"), and if any are present
either remove those keys from the object or log/throw a clear warning via
clientDebugLogger (or throw an Error) so they aren’t merged; then merge the
cleaned structuredContent into modelContext and proceed to setState with the
safe object so reserved fields remain intact.

In @packages/ui/src/types.ts:
- Around line 537-566: The docstring for updateModelContext incorrectly claims
"On ChatGPT: Silent no-op"; update it to reflect that on ChatGPT the OpenAI
adapter uses setState (which maps to setWidgetState) to expose context to the
model. Edit the comment for updateModelContext to state that on ChatGPT the
adapter calls setState / setWidgetState to surface context to the model (and
that it still behaves as a non-triggering context update), referencing
updateModelContext, setState, and setWidgetState so readers can locate the
implementation in the OpenAI adapter.
🧹 Nitpick comments (2)
packages/ui/tests/unit/mcp-adapter.test.ts (1)

272-281: Consider adding a positive path test for updateModelContext.

The current tests verify error handling and method existence, but there's no test that calls updateModelContext successfully after connection. While the adapter may not have a real parent window in the test environment, you could verify it doesn't throw when connected (similar to how setState is tested on line 35).

💡 Optional: Add positive path test
   describe("updateModelContext (ext-apps v0.4.0+)", () => {
     it("should throw when not connected", async () => {
       await expect(
         adapter.updateModelContext({ structuredContent: { test: true } })
       ).rejects.toThrow("MCP Apps adapter not connected");
     });

     it("should have updateModelContext method", async () => {
       await adapter.connect();
       expect(typeof adapter.updateModelContext).toBe("function");
     });
+
+    it("should accept valid params when connected", async () => {
+      await adapter.connect();
+      // Without a real parent window, this may still fail, but validates the call path
+      await expect(
+        adapter.updateModelContext({ structuredContent: { key: "value" } })
+      ).rejects.toThrow(); // Expected without real host
+    });
   });
packages/ui/src/types.ts (1)

443-463: Consider using a stricter discriminated union for ContentBlock.

The current interface allows any combination of optional fields regardless of type. A stricter discriminated union would provide better type safety:

type ContentBlock =
  | { type: "text"; text: string }
  | { type: "image"; data: string; mimeType: string }
  | { type: "audio"; data: string; mimeType: string }
  | { type: "resource"; uri: string; text?: string }
  | { type: "resource_link"; uri: string; name?: string; description?: string };

However, the current approach is simpler and acceptable for an @internal type. This is a minor refinement that could be deferred.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f185d6 and b47ee88.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • examples/minimal/package.json
  • examples/restaurant-finder/package.json
  • examples/weather-app/package.json
  • packages/core/package.json
  • packages/testing/package.json
  • packages/ui-react-builder/package.json
  • packages/ui-react/src/hooks.ts
  • packages/ui-react/src/index.ts
  • packages/ui/package.json
  • packages/ui/src/adapters/mcp.ts
  • packages/ui/src/adapters/mock.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/src/adapters/types.ts
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui/src/types.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • packages/ui/tests/unit/mock-adapter.test.ts
  • packages/ui/tests/unit/openai-adapter.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Express 5 (not 4) - async error handling works differently

Files:

  • packages/ui/src/adapters/types.ts
  • packages/ui-react/src/index.ts
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui/src/types.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/tests/unit/mock-adapter.test.ts
  • packages/ui/tests/unit/openai-adapter.test.ts
  • packages/ui-react/src/hooks.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • packages/ui/src/adapters/mock.ts
  • packages/ui/src/adapters/mcp.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use strict TypeScript - no any types, use unknown with type narrowing instead
Use defineTool and defineUI (or defineReactUI for React components) for type inference
Use export type for type-only exports to prevent runtime imports of types

Files:

  • packages/ui/src/adapters/types.ts
  • packages/ui-react/src/index.ts
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui/src/types.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/tests/unit/mock-adapter.test.ts
  • packages/ui/tests/unit/openai-adapter.test.ts
  • packages/ui-react/src/hooks.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • packages/ui/src/adapters/mock.ts
  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

packages/ui/**/*.{ts,tsx,js,jsx}: Use createClient() with proper await - it is an async function, not synchronous
Handle both MCP and OpenAI response formats when calling tools
Ensure adapter.connect() is awaited before subscribing to events or rendering
Set up onHostContextChange subscription before first render to ensure theme applies correctly

Files:

  • packages/ui/src/adapters/types.ts
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui/src/types.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/tests/unit/mock-adapter.test.ts
  • packages/ui/tests/unit/openai-adapter.test.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • packages/ui/src/adapters/mock.ts
  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

Provide type parameter to createClient<typeof app.tools()>() for proper typed tool calls

Files:

  • packages/ui/src/adapters/types.ts
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui/src/types.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/tests/unit/mock-adapter.test.ts
  • packages/ui/tests/unit/openai-adapter.test.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • packages/ui/src/adapters/mock.ts
  • packages/ui/src/adapters/mcp.ts
**/index.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Export only through index.ts files to keep public API clean and enable safe refactoring

Files:

  • packages/ui-react/src/index.ts
  • packages/ui/src/index.ts
packages/ui-react/**/*.{tsx,ts}

📄 CodeRabbit inference engine (packages/ui-react/AGENTS.md)

Always type the useToolResult<T>() hook with a generic parameter specifying the tool output type

Files:

  • packages/ui-react/src/index.ts
  • packages/ui-react/src/hooks.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper
📚 Learning: 2026-01-11T17:08:13.651Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:13.651Z
Learning: Applies to packages/core/src/**/*.{ts,tsx} : Ensure Zod 4 APIs are used, not Zod 3 APIs which have breaking changes

Applied to files:

  • examples/minimal/package.json
  • packages/testing/package.json
  • examples/restaurant-finder/package.json
  • examples/weather-app/package.json
  • packages/core/package.json
📚 Learning: 2026-01-11T17:08:03.538Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T17:08:03.538Z
Learning: Use Zod 4 (not 3) - be aware of breaking changes from v3 migration

Applied to files:

  • examples/minimal/package.json
  • examples/restaurant-finder/package.json
  • examples/weather-app/package.json
  • packages/core/package.json
📚 Learning: 2026-01-11T17:08:30.346Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react-builder/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:30.346Z
Learning: Ensure the vite peer dependency is installed when using the mcp-apps-kit/ui-react-builder/vite plugin

Applied to files:

  • examples/minimal/package.json
  • examples/restaurant-finder/package.json
  • packages/ui-react-builder/package.json
  • examples/weather-app/package.json
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper

Applied to files:

  • examples/minimal/package.json
  • packages/ui-react/src/index.ts
  • packages/ui/package.json
  • examples/restaurant-finder/package.json
  • packages/ui/src/index.ts
  • packages/ui-react-builder/package.json
  • packages/ui-react/src/hooks.ts
  • packages/ui/tests/unit/mcp-adapter.test.ts
  • examples/weather-app/package.json
  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:19.638Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/create-app/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:19.638Z
Learning: Applies to packages/create-app/**/*.{ts,tsx,js} : Generate projects using npm (not pnpm) to avoid workspace conflicts in mcp-apps-kit/create-app scaffolded projects

Applied to files:

  • examples/minimal/package.json
  • packages/ui/package.json
  • examples/restaurant-finder/package.json
  • packages/ui-react-builder/package.json
  • examples/weather-app/package.json
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Wrap your React application with `AppsProvider` context provider to enable access to MCP client and host context

Applied to files:

  • examples/minimal/package.json
  • packages/ui-react/src/index.ts
  • packages/ui/package.json
  • examples/restaurant-finder/package.json
  • packages/ui-react/src/hooks.ts
  • examples/weather-app/package.json
  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:19.638Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/create-app/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:19.638Z
Learning: Applies to packages/create-app/**/*.{ts,tsx,js} : Verify that package versions fetched from npm registry at runtime are valid before generating projects in mcp-apps-kit/create-app

Applied to files:

  • examples/minimal/package.json
  • packages/ui/package.json
  • examples/restaurant-finder/package.json
  • packages/ui-react-builder/package.json
  • examples/weather-app/package.json
📚 Learning: 2026-01-11T17:08:30.346Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react-builder/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:30.346Z
Learning: Applies to packages/ui-react-builder/**/*.tsx : Use .tsx extension for files containing JSX and ensure jsx: 'automatic' is configured in esbuild settings

Applied to files:

  • examples/minimal/package.json
  • packages/ui-react/src/index.ts
  • packages/ui/package.json
  • packages/ui-react-builder/package.json
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Use `useDocumentTheme()` and `useHostStyleVariables()` hooks to automatically apply theming based on host context (theme and locale)

Applied to files:

  • packages/ui-react/src/index.ts
  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,ts} : Always type the `useToolResult<T>()` hook with a generic parameter specifying the tool output type

Applied to files:

  • packages/ui-react/src/index.ts
  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Provide type parameter to `createClient<typeof app.tools()>()` for proper typed tool calls

Applied to files:

  • packages/ui-react/src/index.ts
  • packages/ui/package.json
  • packages/ui/src/client.ts
  • packages/ui/src/index.ts
  • packages/ui-react-builder/package.json
  • packages/ui/src/adapters/openai.ts
📚 Learning: 2026-01-11T17:08:13.651Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:13.651Z
Learning: Applies to packages/core/src/**/*.{ts,tsx} : Use `defineTool` and `defineUI` for type inference instead of inline objects

Applied to files:

  • packages/ui-react/src/index.ts
  • packages/ui/src/index.ts
📚 Learning: 2026-01-11T17:08:03.538Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T17:08:03.538Z
Learning: Applies to **/*.{ts,tsx} : Use `defineTool` and `defineUI` (or `defineReactUI` for React components) for type inference

Applied to files:

  • packages/ui-react/src/index.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Handle both MCP and OpenAI response formats when calling tools

Applied to files:

  • packages/ui-react/src/index.ts
  • examples/restaurant-finder/package.json
  • packages/ui/src/index.ts
  • packages/ui/src/adapters/openai.ts
  • examples/weather-app/package.json
  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:24.463Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/testing/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:24.463Z
Learning: Ensure optional peer dependencies (vitest, jest, fast-check, openai, anthropic-ai/sdk) are installed when using LLM evaluation features

Applied to files:

  • packages/testing/package.json
  • packages/ui/tests/unit/openai-adapter.test.ts
📚 Learning: 2026-01-11T17:08:03.538Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T17:08:03.538Z
Learning: Applies to tests/**/*.{ts,tsx,js,jsx} : Tests should mirror source structure in `tests/` directory

Applied to files:

  • packages/ui/tests/unit/mock-adapter.test.ts
📚 Learning: 2026-01-11T17:08:24.463Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/testing/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:24.463Z
Learning: Applies to packages/testing/**/vitest.config.ts : Import and set up vitest or jest matchers in the test setup file before tests run (e.g., setupVitestMatchers for vitest)

Applied to files:

  • packages/ui/tests/unit/openai-adapter.test.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Set up `onHostContextChange` subscription before first render to ensure theme applies correctly

Applied to files:

  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Ensure adapter.connect() is awaited before subscribing to events or rendering

Applied to files:

  • packages/ui/tests/unit/mcp-adapter.test.ts
🧬 Code graph analysis (5)
packages/ui/src/adapters/types.ts (3)
packages/ui-react/src/index.ts (1)
  • UpdateModelContextParams (23-23)
packages/ui/src/index.ts (1)
  • UpdateModelContextParams (58-58)
packages/ui/src/types.ts (1)
  • UpdateModelContextParams (468-480)
packages/ui/src/client.ts (1)
packages/ui/src/types.ts (1)
  • UpdateModelContextParams (468-480)
packages/ui/src/types.ts (2)
packages/ui-react/src/index.ts (3)
  • ContainerDimensions (21-21)
  • ContentBlock (22-22)
  • UpdateModelContextParams (23-23)
packages/ui/src/index.ts (3)
  • ContainerDimensions (29-29)
  • ContentBlock (57-57)
  • UpdateModelContextParams (58-58)
packages/ui/src/adapters/mock.ts (3)
packages/ui-react/src/index.ts (1)
  • UpdateModelContextParams (23-23)
packages/ui/src/index.ts (1)
  • UpdateModelContextParams (58-58)
packages/ui/src/types.ts (1)
  • UpdateModelContextParams (468-480)
packages/ui/src/adapters/mcp.ts (1)
packages/ui/src/types.ts (4)
  • ContainerDimensions (274-278)
  • HostContext (317-364)
  • UpdateModelContextParams (468-480)
  • HostCapabilities (15-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-and-lint
  • GitHub Check: claude-review
🔇 Additional comments (30)
examples/restaurant-finder/package.json (1)

22-22: LGTM!

The zod version bump to ^4.3.5 aligns with the coordinated dependency updates across the monorepo.

examples/weather-app/package.json (1)

24-24: LGTM!

Consistent zod version bump matching other examples in this PR.

examples/minimal/package.json (1)

25-25: LGTM!

Consistent zod version bump across all example packages.

packages/testing/package.json (1)

64-66: LGTM!

Both dependency bumps are appropriate:

  • @modelcontextprotocol/sdk patch bump (1.25.1 → 1.25.2)
  • zod version aligned with other packages in the monorepo
packages/ui/package.json (1)

47-47: Core dependency update driving this PR.

The bump to @modelcontextprotocol/ext-apps ^0.4.0 enables the new features (updateModelContext, containerDimensions, HostCapabilities). The package version (0.4.0) appropriately aligns with the ext-apps dependency version, and all three features are extensively utilized throughout the codebase with comprehensive test coverage.

packages/ui/src/adapters/types.ts (2)

18-18: LGTM!

The import is correctly added alongside other type imports from ../types.


72-82: LGTM!

The updateModelContext method is well-documented with clear platform-specific behavior (MCP Apps vs ChatGPT graceful degradation). The method signature correctly uses the new UpdateModelContextParams type and follows the established patterns in this interface.

packages/core/package.json (2)

45-49: LGTM!

The dependency updates are appropriate:

  • @modelcontextprotocol/sdk patch bump (1.25.1 → 1.25.2)
  • zod stays within v4 (4.0.0 → 4.3.5), consistent with the project's Zod 4 requirement per learnings

56-56: LGTM!

The zod peer dependency version correctly matches the regular dependency version (^4.3.5), ensuring consistency for consumers.

packages/ui/src/index.ts (2)

29-29: LGTM!

The ContainerDimensions type is correctly added to the public type exports, supporting the new viewport semantics introduced in ext-apps v0.4.0.


56-58: LGTM!

The model context types (ContentBlock, UpdateModelContextParams) are properly exported with a clear descriptive comment indicating they are for ext-apps v0.4.0+. This follows the established export organization pattern in this file.

packages/ui/tests/unit/mcp-adapter.test.ts (1)

254-270: LGTM!

Good test coverage for the updateModelContext method:

  • Validates error handling when adapter is not connected
  • Confirms method availability after connection

The test pattern is consistent with other method tests in this file.

packages/ui-react/src/index.ts (2)

20-24: LGTM!

The new model context types (ContainerDimensions, ContentBlock, UpdateModelContextParams) are correctly exported using export type syntax as required by coding guidelines, with clear version annotations.


58-60: LGTM!

The useUpdateModelContext hook is properly exported with a clear comment indicating the ext-apps v0.4.0+ requirement, maintaining consistency with the existing hook export pattern.

packages/ui/src/client.ts (2)

23-24: LGTM!

The UpdateModelContextParams type import is correctly added alongside other type imports.


121-126: LGTM!

The updateModelContext method correctly delegates to the adapter, following the same pattern as other methods like sendMessage and openLink. The async/await handling is consistent with the existing codebase patterns.

packages/ui-react/src/hooks.ts (2)

17-18: LGTM!

The UpdateModelContextParams type import is correctly added with the other type imports from @mcp-apps-kit/ui.


254-303: LGTM!

The useUpdateModelContext hook is well-implemented:

  • Follows the established pattern of using useAppsContext() and useCallback with proper dependencies
  • Provides graceful degradation with a console warning when client is unavailable
  • The JSDoc example correctly demonstrates usage with useEffect and proper dependency management
  • The memoized callback ensures stable function identity when client doesn't change

Based on learnings, components using this hook should be rendered within an AppsProvider context wrapper.

packages/ui/tests/unit/mock-adapter.test.ts (1)

407-461: LGTM!

Comprehensive test coverage for the new updateModelContext API:

  • Tests console logging behavior for debugging
  • Tests state storage via getLastModelContext() helper
  • Tests both text content blocks and structured content input types
  • Properly follows existing test patterns with beforeEach setup and console spy cleanup
packages/ui/tests/unit/openai-adapter.test.ts (2)

9-9: LGTM!

The afterEach import is correctly added to support test cleanup.


404-485: LGTM!

Excellent test coverage for the OpenAI adapter's updateModelContext implementation:

  • Tests verify the transformation to setWidgetState format with _type: "modelContext" marker
  • Tests text content concatenation logic (joining with newlines)
  • Tests graceful degradation when SDK is unavailable, confirming local state storage still works
  • Mock setup and cleanup is handled correctly in each test
packages/ui/src/adapters/mock.ts (1)

139-155: LGTM! Clean mock implementation for updateModelContext.

The mock adapter correctly stores the last model context for test verification and follows the established logging pattern. The getLastModelContext() helper enables straightforward unit test assertions.

packages/ui/src/adapters/openai.ts (1)

550-559: LGTM! Text content extraction is correctly implemented.

The filter ensures only text blocks with actual content are processed, and joining with newlines provides a readable format for the model context.

packages/ui/src/adapters/mcp.ts (4)

280-296: LGTM! Clean containerDimensions parsing with backward-compatible viewport derivation.

The implementation correctly:

  1. Extracts containerDimensions when provided by the host
  2. Derives viewport from container dimensions for backward compatibility
  3. Falls back to explicit viewport or defaults when containerDimensions is absent

This ensures existing code using viewport continues to work while supporting the new ext-apps v0.4.0+ semantics.


321-336: LGTM! Viewport derivation correctly handles both fixed and flexible dimension semantics.

The helper properly maps:

  • Fixed dimensions (height/width) directly to viewport
  • Flexible dimensions (maxHeight/maxWidth) preserved for apps that need them
  • Falls back to defaults when specific dimensions aren't provided

425-457: LGTM! Model context update implementation with appropriate type mappings.

The method correctly:

  • Validates connection state before proceeding
  • Maps text, image, and audio content blocks to ext-apps format
  • Provides sensible fallbacks for resource types (converting to text representation)
  • Passes through structuredContent directly

The default mimeType values (image/png, audio/wav) are reasonable fallbacks when the caller doesn't specify.


649-654: LGTM! New ext-apps v0.4.0+ capabilities correctly exposed.

The capabilities mapping properly exposes updateModelContext, message, and sandbox from the SDK, allowing apps to feature-detect these new capabilities.

packages/ui/src/types.ts (3)

74-136: LGTM! Well-structured capability definitions for ext-apps v0.4.0+.

The new capabilities follow the established pattern:

  • updateModelContext and message enumerate supported content block types
  • sandbox properly models permissions and CSP configuration domains
  • All use Record<string, never> for boolean-like capability flags, consistent with existing definitions

265-278: LGTM! ContainerDimensions type correctly models the ext-apps dimension semantics.

The union type captures the four valid combinations of fixed vs. flexible dimensions, with clear documentation explaining when each applies.


465-480: LGTM! Clean public interface for model context updates.

The interface provides flexibility by making both content and structuredContent optional, allowing callers to use either or both as needed.

Comment thread packages/ui-react-builder/package.json Outdated
Comment thread packages/ui/src/adapters/openai.ts
Comment thread packages/ui/src/types.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b47ee8808a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +561 to +562
// Use existing setState which calls setWidgetState
this.setState(modelContext);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve widget state when updating model context

On ChatGPT, updateModelContext() builds a fresh modelContext object and passes it directly to setState, which replaces the entire widget state. Any app that uses useWidgetState/setState for persistence will lose its stored state as soon as updateModelContext runs (and getState() will return the model context on reload). This makes the new API incompatible with existing persisted state unless the update merges with prior state or stores model context under a namespaced key.

Useful? React with 👍 / 👎.

Comment thread packages/ui/src/adapters/mcp.ts Outdated
Comment on lines +447 to +449
default:
// For resource types, fall back to text representation
return { type: "text" as const, text: block.text ?? block.uri ?? "" };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve resource content blocks in MCP updateModelContext

UpdateModelContextParams allows resource/resource_link content blocks, but the MCP adapter maps every non-text/image/audio block to a text fallback. As a result, resource URIs/names/descriptions are dropped and the host never receives real resource blocks, even when HostCapabilities.updateModelContext advertises them. In MCP Apps contexts that send resource blocks, this silently downgrades content and breaks host-side handling.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/ui/src/adapters/openai.ts (1)

527-569: LGTM! Good documentation of platform-specific behavior.

The JSDoc clearly documents that updateModelContext uses setState/setWidgetState internally on ChatGPT, which addresses the PR objective about clarifying this relationship. The _type: "modelContext" marker is a nice touch for distinguishing context updates from regular state.

Consider using a type guard or type assertion for slightly improved type safety in the content block filtering:

♻️ Optional type safety improvement
     // Convert content blocks to text representation for the model
     if (params.content && params.content.length > 0) {
       const textContent = params.content
-        .filter((block) => block.type === "text" && block.text)
-        .map((block) => block.text)
+        .filter((block): block is { type: "text"; text: string } => 
+          block.type === "text" && typeof block.text === "string" && block.text.length > 0)
+        .map((block) => block.text)
         .join("\n");
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b47ee88 and 1ff5756.

📒 Files selected for processing (3)
  • packages/ui-react/src/hooks.ts
  • packages/ui/src/adapters/openai.ts
  • packages/ui/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ui/src/types.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Express 5 (not 4) - async error handling works differently

Files:

  • packages/ui/src/adapters/openai.ts
  • packages/ui-react/src/hooks.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use strict TypeScript - no any types, use unknown with type narrowing instead
Use defineTool and defineUI (or defineReactUI for React components) for type inference
Use export type for type-only exports to prevent runtime imports of types

Files:

  • packages/ui/src/adapters/openai.ts
  • packages/ui-react/src/hooks.ts
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

packages/ui/**/*.{ts,tsx,js,jsx}: Use createClient() with proper await - it is an async function, not synchronous
Handle both MCP and OpenAI response formats when calling tools
Ensure adapter.connect() is awaited before subscribing to events or rendering
Set up onHostContextChange subscription before first render to ensure theme applies correctly

Files:

  • packages/ui/src/adapters/openai.ts
packages/ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

Provide type parameter to createClient<typeof app.tools()>() for proper typed tool calls

Files:

  • packages/ui/src/adapters/openai.ts
packages/ui-react/**/*.{tsx,ts}

📄 CodeRabbit inference engine (packages/ui-react/AGENTS.md)

Always type the useToolResult<T>() hook with a generic parameter specifying the tool output type

Files:

  • packages/ui-react/src/hooks.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Handle both MCP and OpenAI response formats when calling tools
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Provide type parameter to `createClient<typeof app.tools()>()` for proper typed tool calls

Applied to files:

  • packages/ui/src/adapters/openai.ts
  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper

Applied to files:

  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Use `useDocumentTheme()` and `useHostStyleVariables()` hooks to automatically apply theming based on host context (theme and locale)

Applied to files:

  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Wrap your React application with `AppsProvider` context provider to enable access to MCP client and host context

Applied to files:

  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,ts} : Always type the `useToolResult<T>()` hook with a generic parameter specifying the tool output type

Applied to files:

  • packages/ui-react/src/hooks.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Set up `onHostContextChange` subscription before first render to ensure theme applies correctly

Applied to files:

  • packages/ui-react/src/hooks.ts
🔇 Additional comments (3)
packages/ui-react/src/hooks.ts (2)

17-17: LGTM!

Import addition for UpdateModelContextParams is correctly placed with other type imports from @mcp-apps-kit/ui.


254-308: LGTM! Well-documented hook with proper React patterns.

The implementation correctly:

  • Uses useCallback with [client] dependency for a stable function reference
  • Handles the missing client case gracefully with a warning
  • Documents the platform-specific behavior differences (MCP Apps vs ChatGPT)
  • Is properly exported from the package's public API
packages/ui/src/adapters/openai.ts (1)

19-19: LGTM!

Import addition for UpdateModelContextParams is correctly placed with other type imports.

@AndurilCode AndurilCode deleted a comment from claude Bot Jan 13, 2026
@AndurilCode AndurilCode deleted a comment from claude Bot Jan 13, 2026
- Move build-time deps (@typescript-eslint/typescript-estree, esbuild) to
  devDependencies in ui-react-builder package.json
- Add filtering for reserved underscore-prefixed keys in
  updateModelContext to prevent overwriting internal fields like _type
- Fix linting error by avoiding non-null assertion

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Code Review: ext-apps v0.4.0 Support

Summary

This PR adds support for ext-apps v0.4.0 features including updateModelContext(), containerDimensions, and new host capabilities. The implementation is generally well-structured with good documentation and test coverage.

✅ Strengths

1. Excellent Documentation

  • Clear JSDoc comments explaining platform differences (MCP vs ChatGPT)
  • Helpful usage examples in React hooks
  • Implementation notes about setState vs updateModelContext behavior

2. Good Test Coverage

  • New tests for updateModelContext() in all three adapters (MCP, OpenAI, Mock)
  • Tests cover structured content, text content blocks, and error cases
  • Mock adapter includes test utilities (getLastModelContext())

3. Backward Compatibility

  • containerDimensions properly derives viewport for backward compatibility (mcp.ts:288-295)
  • Graceful handling when features aren't available

4. Consistent API Design

  • Unified interface across both protocols (MCP Apps and ChatGPT)
  • Clear separation of concerns in adapter implementations

⚠️ Issues & Recommendations

1. Type Safety Issue in ContentBlock (Medium Priority)

Location: packages/ui/src/types.ts:448-463

The ContentBlock interface has all fields optional, which could lead to invalid states:

export interface ContentBlock {
  type: "text" | "image" | "audio" | "resource" | "resource_link";
  text?: string;      // Required for text, optional for others
  data?: string;      // Required for image/audio
  mimeType?: string;  // Required for image/audio
  uri?: string;       // Required for resource types
  // ...
}

Problem: You can create { type: "text" } without text, or { type: "image" } without data/mimeType.

Recommendation: Use discriminated unions for type safety:

export type ContentBlock =
  | { type: "text"; text: string }
  | { type: "image"; data: string; mimeType: string }
  | { type: "audio"; data: string; mimeType: string }
  | { type: "resource"; uri: string; text?: string; description?: string }
  | { type: "resource_link"; uri: string; name?: string; description?: string };

This prevents invalid states at compile time and makes the API more intuitive.

2. Default Fallback Values May Hide Bugs (Low Priority)

Location: packages/ui/src/adapters/mcp.ts:434-450

The code uses default fallbacks (?? "") which could mask missing required data:

case "text":
  return { type: "text" as const, text: block.text ?? "" };  // Empty string if text missing
case "image":
  return {
    type: "image" as const,
    data: block.data ?? "",           // Empty base64?
    mimeType: block.mimeType ?? "image/png",  // Default MIME type
  };

Concern: If a developer forgets to provide text or data, this silently sends empty content instead of failing fast.

Recommendation: Consider validation or at minimum add debug logging:

case "text":
  if (!block.text) {
    clientDebugLogger.warn("[MCP Adapter] Text block missing text content");
  }
  return { type: "text" as const, text: block.text ?? "" };

3. Underscore Key Filtering Could Be More Robust (Low Priority)

Location: packages/ui/src/adapters/openai.ts:551-566

The code filters underscore-prefixed keys from structuredContent but then adds _textContent:

// Filter out underscore-prefixed keys
Object.keys(structuredContent).forEach((key) => {
  if (!key.startsWith("_")) {
    modelContext[key] = structuredContent[key];
  }
});
// ...
if (textContent) {
  modelContext._textContent = textContent;  // But we add our own underscore key
}

Recommendation: Document why _textContent and _type are safe internal keys, or consider a whitelist approach for reserved keys.

4. Inconsistent Error Types (Low Priority)

Location: packages/ui/src/adapters/mcp.ts:427 vs mcp.ts:463

// Line 427 - Uses UIError
throw new UIError(UIErrorCode.NOT_CONNECTED, "MCP Apps adapter not connected");

// Line 463 - Uses generic Error
throw new Error("MCP Apps adapter not connected");

Recommendation: Use consistent error types throughout the adapter for better error handling.

5. Missing Validation for containerDimensions (Low Priority)

Location: packages/ui/src/adapters/mcp.ts:321-335

The deriveViewportFromContainerDimensions method assumes numeric types but doesn't validate:

return {
  width: typeof d.width === "number" ? d.width : defaults.width,
  height: typeof d.height === "number" ? d.height : defaults.height,
  maxWidth: typeof d.maxWidth === "number" ? d.maxWidth : undefined,
  maxHeight: typeof d.maxHeight === "number" ? d.maxHeight : undefined,
};

Concern: Negative or zero dimensions could cause rendering issues.

Recommendation: Add validation:

const validateDimension = (val: unknown, fallback: number): number => {
  return typeof val === "number" && val > 0 ? val : fallback;
};

🔒 Security Review

✅ No security issues identified

  • No user input is directly executed or interpolated into code
  • Base64 data handling is safe (passed through to SDK)
  • No SQL injection, XSS, or command injection vectors
  • Proper type checking before accessing properties

🎯 Performance Considerations

  • Object spread usage is appropriate - Small objects, not in hot paths
  • Array mapping in updateModelContext is efficient for expected small arrays
  • No unnecessary re-renders - React hook uses useCallback correctly

📋 Required Actions

Before Merge:

  1. Run the full test suite as per CLAUDE.md:

    pnpm build && pnpm test && pnpm lint && pnpm typecheck

    All four must pass.

  2. Consider addressing the type safety issue (Add Claude Code GitHub Workflow #1 above) - This would prevent runtime errors and improve DX.

Nice to Have:

📝 Package Version Updates

The dependency updates look correct:

  • ✅ Zod 4.0.0 → 4.3.5 (patch updates)
  • ✅ @modelcontextprotocol/sdk 1.25.1 → 1.25.2
  • ✅ @modelcontextprotocol/ext-apps 0.2.2 → 0.4.0 (target version)
  • ✅ ui-react-builder: Moved dev dependencies correctly

Overall Assessment

Rating: 8.5/10

This is a solid implementation that follows the codebase conventions (strict TypeScript, good documentation, test coverage). The main improvement would be stronger type safety for ContentBlock to prevent invalid states at compile time rather than runtime.

The dual-protocol abstraction is well-maintained, with clear documentation of platform differences. The code is ready to merge after running the required test suite, though I'd strongly recommend addressing the type safety issue in a follow-up if not in this PR.

Great work on maintaining comprehensive documentation and test coverage! 🎉

- Moved @typescript-eslint/types and @typescript-eslint/typescript-estree from devDependencies to dependencies in package.json for ui-react-builder.
- Updated tsup.config.ts to include these packages in the build configuration.

This change ensures that the necessary TypeScript ESLint packages are available during runtime.
@claude

claude Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Code Review - PR #96: ext-apps v0.4.0 Support

Overview

This PR adds comprehensive support for ext-apps v0.4.0 features, including the new updateModelContext() method, containerDimensions type, and new host capabilities. The implementation is well-structured with good test coverage.


✅ Strengths

1. Excellent Documentation

  • Clear JSDoc comments explaining platform differences (MCP Apps vs ChatGPT)
  • Helpful inline comments about backward compatibility (e.g., deriveViewportFromContainerDimensions)
  • Good usage examples in the React hook documentation

2. Strong Test Coverage

  • Comprehensive tests for updateModelContext across all adapters (MCP, OpenAI, Mock)
  • Tests cover edge cases (missing SDK, graceful degradation)
  • Test organization mirrors the new feature structure

3. Type Safety

  • Proper TypeScript types with good use of discriminated unions
  • Correct use of Record<string, never> for empty capability objects
  • Type exports properly handled with export type

4. Backward Compatibility

  • containerDimensionsviewport derivation preserves existing API
  • Graceful fallbacks when new features aren't supported
  • Optional fields don't break existing code

5. Consistent Architecture

  • Protocol adapter pattern consistently applied
  • Proper separation of concerns between adapters
  • Clean abstraction over platform differences

⚠️ Issues & Recommendations

1. Security Concern: Reserved Key Filtering Logic (Medium Priority)

Location: packages/ui/src/adapters/openai.ts:550-566

Issue: The reserved key filtering only warns but still includes the _type key in the model context:

const modelContext: Record<string, unknown> = {
  _type: "modelContext",  // This is a reserved key\!
};

// Later, filters user's underscore keys but not the one we added
if (reservedKeys.length > 0) {
  clientDebugLogger.debug(/*...*/);
}

Problem: The code filters reserved keys from structuredContent but the adapter itself uses _type and _textContent. If a user passes { _type: "something" }, it will be filtered out, but this creates an inconsistency.

Recommendation:

// Option 1: Use a namespace to avoid collisions
const modelContext: Record<string, unknown> = {
  __mcp_type: "modelContext",  // Double underscore = internal
};

// Option 2: Document that single-underscore keys are reserved
// and validate at runtime with a clear error message

2. Potential Bug: Missing Validation in Content Block Mapping (Low Priority)

Location: packages/ui/src/adapters/mcp.ts:431-451

Issue: The updateModelContext method uses ?? operator with empty strings as fallbacks:

case "text":
  return { type: "text" as const, text: block.text ?? "" };
case "image":
  return {
    type: "image" as const,
    data: block.data ?? "",  // Empty base64?
    mimeType: block.mimeType ?? "image/png",
  };

Problem: An empty string is not a valid base64 image. If block.data is missing, silently passing an empty string could cause downstream errors.

Recommendation:

case "image":
  if (\!block.data) {
    throw new UIError(
      UIErrorCode.INVALID_PARAMS,
      "Image content block requires data field"
    );
  }
  return {
    type: "image" as const,
    data: block.data,
    mimeType: block.mimeType ?? "image/png",
  };

3. Type Safety: ContentBlock Type Could Be More Precise (Low Priority)

Location: packages/ui/src/types.ts:448-463

Issue: All fields are optional, making it easy to create invalid combinations:

const invalid: ContentBlock = {
  type: "image",
  // Missing required 'data' field - TypeScript doesn't catch this
};

Recommendation: Use discriminated unions for better type safety:

export type ContentBlock =
  | { type: "text"; text: string }
  | { type: "image"; data: string; mimeType?: string }
  | { type: "audio"; data: string; mimeType?: string }
  | { type: "resource"; uri: string; description?: string }
  | { type: "resource_link"; uri: string; name?: string; description?: string };

This would make the mapping code in mcp.ts type-safe and eliminate runtime validation needs.

4. Package.json: Dependency Organization (Nitpick)

Location: packages/ui-react-builder/package.json:69-73

Issue: @typescript-eslint/types was moved from devDependencies to dependencies, but it's only used for build-time type parsing, not runtime.

Recommendation: Keep it in dependencies as is (since the builder needs it at runtime when processing TypeScript code), but add a comment explaining why:

"dependencies": {
  // Runtime dependencies for parsing user TypeScript code
  "@typescript-eslint/types": "^8.53.0",
  "@typescript-eslint/typescript-estree": "^8.53.0",
  "esbuild": "^0.27.2"
}

5. Documentation: Platform Behavior Clarification

Location: Multiple files (hooks.ts:264-270, client.ts, adapters/openai.ts:529-543)

Issue: The documentation states that ChatGPT uses setState/setWidgetState for updateModelContext, which exposes data to the AI. However, it's not clear:

  1. How long this context persists
  2. Whether it accumulates or replaces previous context
  3. Memory/size limits

Recommendation: Add a note about the limitations:

/**
 * On ChatGPT, both setState() and updateModelContext() expose state to the
 * AI model. Use setState() for persistence-focused use cases, and
 * updateModelContext() for context-focused use cases.
 * 
 * Note: On ChatGPT, context persists for the widget session and is
 * replaced on each call (not accumulated). Consider data size when
 * sending large structured content.
 */

📊 Additional Observations

Positive Patterns

  1. Error handling: Proper error throwing with UIError and error codes
  2. Logging: Good use of debug logger for troubleshooting
  3. Testing: Mock adapter includes getLastModelContext() helper for testing
  4. Hook design: useUpdateModelContext properly uses useCallback with correct dependencies

Performance Considerations

  • updateModelContext in OpenAI adapter calls setState, which may trigger re-renders. This is expected behavior but worth noting for apps that update context frequently.
  • Content block mapping creates new arrays/objects - acceptable for typical use cases but consider memoization if called in tight loops.

🎯 Summary

Overall Assessment:Approve with minor suggestions

This is a solid implementation of the ext-apps v0.4.0 features. The code follows the project's conventions (per CLAUDE.md), has good test coverage, and handles cross-platform differences elegantly.

Priority Actions:

  1. Medium: Address the reserved key filtering inconsistency in OpenAI adapter
  2. Low: Consider stricter validation for content blocks (or use discriminated unions)
  3. Nice-to-have: Improve documentation about platform-specific behavior limits

The PR can be merged after addressing #1, with #2-3 potentially handled in a follow-up if desired.

Before merging: Ensure pnpm build && pnpm test && pnpm lint && pnpm typecheck all pass (per CLAUDE.md requirements).


Great work overall! The dual-protocol abstraction continues to work well. 🚀

…pnpm-lock.yaml

- Restored @typescript-eslint/types, @typescript-eslint/typescript-estree, and esbuild to dependencies for ui-react-builder.
- This change ensures these packages are available during runtime, aligning with the recent updates in package management.
@claude

claude Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Code Review - PR #96: Update ext-apps to v0.4.0

Summary

This PR adds support for ext-apps v0.4.0 features, introducing updateModelContext() and containerDimensions with appropriate backward compatibility handling. Overall, this is a well-structured implementation with good test coverage and documentation.


✅ Strengths

1. Excellent Documentation

  • Clear JSDoc comments explaining platform differences between MCP Apps and ChatGPT
  • Good inline comments explaining backward compatibility decisions
  • Comprehensive examples in hook documentation (e.g., useUpdateModelContext)

2. Strong Test Coverage

  • Tests added for all three adapters (MCP, OpenAI, Mock)
  • Edge cases covered (missing client, graceful degradation, reserved keys filtering)
  • Mock adapter provides getLastModelContext() for testing - good pattern

3. Backward Compatibility

  • containerDimensions properly derives viewport for backward compat (packages/ui/src/adapters/mcp.ts:288-318)
  • Graceful handling when features aren't available
  • Type-safe approach with proper type guards

4. Dependency Management

  • Zod updated from ^4.0.0 to ^4.3.5 across all packages (consistent)
  • MCP SDK updated to ^1.25.2
  • ext-apps updated to ^0.4.0
  • Good move to add @typescript-eslint/types to dependencies in ui-react-builder (prevents peer dependency issues)

🔍 Issues & Recommendations

High Priority

1. Type Safety Issue in ContainerDimensions ⚠️

Location: packages/ui/src/types.ts:274-278

The current union type allows ambiguous cases:

export type ContainerDimensions =
  | { height: number; width: number }
  | { height: number; maxWidth?: number }  // ❌ Can overlap with first type
  | { maxHeight?: number; width: number }   // ❌ Can overlap with first type
  | { maxHeight?: number; maxWidth?: number }

Problem: { height: 100, width: 200, maxWidth: 300 } matches both the first and second types, creating ambiguity.

Recommendation: Use discriminated unions or XOR types to ensure mutual exclusivity:

export type ContainerDimensions =
  | { type: 'fixed'; height: number; width: number }
  | { type: 'flexible-width'; height: number; maxWidth?: number }
  | { type: 'flexible-height'; width: number; maxHeight?: number }
  | { type: 'flexible-both'; maxHeight?: number; maxWidth?: number }

Or ensure the implementation validates that fixed and flexible dimensions are mutually exclusive.

2. Reserved Key Filtering Could Be More Robust 🔧

Location: packages/ui/src/adapters/openai.ts:550-566

const reservedKeys = Object.keys(structuredContent).filter((key) => key.startsWith("_"));

Issues:

  • Filters but only logs, doesn't prevent runtime errors if reserved keys cause issues
  • The filtering happens but then immediately adds _type: "modelContext" and potentially _textContent

Recommendation:

  • Document why underscore-prefixed keys are reserved
  • Consider using a more explicit reserved keys list if there are specific keys to avoid
  • Ensure consistency: if _type and _textContent are OK, document the pattern

Medium Priority

3. Missing Validation in updateModelContext 📝

Location: packages/ui/src/adapters/mcp.ts:420-456

The content block mapping doesn't validate required fields:

case "text":
  return { type: "text" as const, text: block.text ?? "" };  // ❌ Empty string if missing
case "image":
  return {
    type: "image" as const,
    data: block.data ?? "",  // ❌ Empty string for missing required field
    mimeType: block.mimeType ?? "image/png",
  };

Recommendation: Add validation or at least warn when required fields are missing:

case "text":
  if (\!block.text) {
    throw new UIError(UIErrorCode.INVALID_PARAMS, "Text content block requires 'text' field");
  }
  return { type: "text" as const, text: block.text };

4. Potential Runtime Type Issue 🔧

Location: packages/ui/src/adapters/mcp.ts:619-630

const sdkCaps = mcpCaps as Record<string, unknown>;
// ...
logging: sdkCaps.logging as HostCapabilities["logging"],

This double-casting pattern (as Record<string, unknown> then as HostCapabilities["logging"]) bypasses type checking. If the MCP SDK changes its capability structure, this could silently fail.

Recommendation: Use runtime validation or at least type guards:

const isLoggingCapability = (cap: unknown): cap is HostCapabilities["logging"] => {
  // Basic validation
  return cap === undefined || (typeof cap === "object" && cap \!== null);
};

5. Hook Dependency Array Issue ⚠️

Location: packages/ui-react/src/hooks.ts:297-307

export function useUpdateModelContext(): (params: UpdateModelContextParams) => Promise<void> {
  const { client } = useAppsContext();

  return useCallback(
    async (params: UpdateModelContextParams) => {
      if (\!client) {
        console.warn("[useUpdateModelContext] Client not available");
        return;
      }
      await client.updateModelContext(params);
    },
    [client]  // ✅ Correct
  );
}

This is actually correct! The dependency array includes client which is appropriate. However, users might be tempted to include updateContext in their own dependency arrays:

useEffect(() => {
  updateContext({ ... });
}, [items, updateContext]);  // updateContext will change when client changes

Recommendation: Add a note in the JSDoc about this pattern being safe since the function is memoized by client.


Low Priority / Nice to Have

6. Export Type Only Where Needed 📦

Location: packages/ui/src/index.ts:26-29

export type {
  Viewport,
  ContainerDimensions,  // ⚠️ Marked as @internal in types.ts but exported publicly
  // ...
}

Issue: ContainerDimensions is marked @internal in types.ts:272 but exported in the public API.

Recommendation: Either:

  • Remove @internal tag if it's meant to be public
  • Don't export it if it's truly internal
  • Use a separate internal types file

7. Consider Adding Integration Test

The PR has good unit tests but lacks an integration test that verifies:

  • MCP adapter properly calls ext-apps SDK's updateModelContext
  • OpenAI adapter properly calls setState under the hood
  • End-to-end flow through the client

Recommendation: Add an integration test in a future PR to verify the full flow.


🔒 Security Review

✅ No Major Security Concerns

  1. Input Sanitization: Reserved key filtering is good practice
  2. No Code Injection: Content blocks are properly typed and validated
  3. No Exposed Secrets: No credentials or sensitive data handling
  4. Safe Type Assertions: While there are some as casts, they're on trusted SDK data

Minor Note:

  • The structuredContent allows arbitrary Record<string, unknown> which could potentially be used to pass large amounts of data. Consider adding size limits in a future enhancement.

📊 Test Coverage Assessment

Coverage appears strong:

  • ✅ MCP adapter: updateModelContext + containerDimensions tests
  • ✅ OpenAI adapter: multiple test cases including text content, graceful degradation
  • ✅ Mock adapter: test utilities + getLastModelContext
  • ✅ React hook: proper memoization with client dependency

Missing:

  • Integration tests (mentioned above)
  • Error boundary tests for malformed content blocks
  • Tests for very large structuredContent payloads

🎯 Recommended Actions Before Merge

Must Fix:

  1. Resolve ContainerDimensions type ambiguity (High Priority Add Claude Code GitHub Workflow #1)

Should Fix:

  1. Add validation for required fields in content blocks (Medium Priority Fix documentation misalignment in codebase #3)
  2. Clarify ContainerDimensions export vs @internal tag (Low Priority Delete Unified MCP Apps Builder SDK specifications and tasks #6)

Nice to Have:

  1. Improve type safety in capability mapping (Medium Priority Fix linting errors and warnings #4)
  2. Document reserved key filtering rationale (Medium Priority Create FUNDING.yml #2)

📝 Additional Notes

Following CLAUDE.md Guidelines:

  • ✅ Uses Zod 4 (updated to 4.3.5)
  • ✅ Exports through index.ts
  • ✅ Tests mirror source structure
  • ✅ TypeScript strict mode compliance
  • ⚠️ Should add note to CLAUDE.md Learnings section about the @typescript-eslint/types dependency move

Suggested CLAUDE.md Learning Entry:

- Moved `@typescript-eslint/types` from devDependencies to dependencies in ui-react-builder → AST parser needs it at runtime, not just for type checking during dev

🎉 Conclusion

This is a solid PR that properly implements ext-apps v0.4.0 support. The main concern is the ContainerDimensions type ambiguity which should be addressed before merge. The rest of the recommendations are enhancements that could be addressed in follow-up PRs.

Recommendation: Request changes to fix the type ambiguity, then approve after fix.

Great work on the comprehensive documentation and test coverage! 👏

- Changed protocol from "openai" to "mcp" in the weather app server configuration.
- Added new error code `INVALID_PARAMS` to handle missing fields in content blocks.
- Enhanced content block validation in MCP adapter to ensure required fields are present for each type.
- Updated `ContentBlock` type definition for improved type safety using discriminated unions.
- Clarified documentation in `updateModelContext` regarding context persistence and reserved keys.

These changes improve the robustness of the protocol handling and ensure that content blocks are validated correctly before processing.
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Code Review: ext-apps v0.4.0 Support

Summary

This PR successfully adds support for ext-apps v0.4.0 features, introducing updateModelContext() functionality and containerDimensions semantics. The implementation follows the codebase patterns and includes tests. However, there are several issues that need attention.


🔴 Critical Issues

1. Non-exhaustive switch statement in MCP adapter

Location: packages/ui/src/adapters/mcp.ts:431-486

The switch statement in updateModelContext() is missing a default case, which means TypeScript won't catch if new content block types are added to the ContentBlock union type.

Issue:

const content = params.content?.map((block) => {
  switch (block.type) {
    case "text": ...
    case "image": ...
    case "audio": ...
    case "resource": ...
    case "resource_link": ...
    // Missing default case - no exhaustiveness checking!
  }
});

Fix: Add a default case that performs exhaustiveness checking:

switch (block.type) {
  case "text": ...
  case "image": ...
  case "audio": ...
  case "resource": ...
  case "resource_link": ...
  default: {
    const _exhaustive: never = block;
    throw new UIError(
      UIErrorCode.INVALID_PARAMS,
      \`Unsupported content block type: \${(_exhaustive as ContentBlock).type}\`
    );
  }
}

This ensures TypeScript will error at compile time if new types are added to ContentBlock but not handled in the switch.

2. Incomplete content handling in OpenAI adapter

Location: packages/ui/src/adapters/openai.ts:581-589

The OpenAI adapter only handles text content blocks when converting to model context, silently ignoring image, audio, and resource blocks.

Issue:

const textContent = params.content
  .filter((block) => block.type === "text")
  .map((block) => block.text)
  .join("\n");

This means image, audio, resource, and resource_link blocks are silently dropped on ChatGPT. Either:

  • Document this limitation in the JSDoc with a clear warning
  • Add a debug log when non-text blocks are filtered
  • Convert other block types to text representations (like MCP adapter does for resources)

Recommendation: Add logging and documentation:

if (params.content && params.content.length > 0) {
  const nonTextBlocks = params.content.filter((block) => block.type !== "text");
  if (nonTextBlocks.length > 0) {
    clientDebugLogger.debug(
      \`[OpenAI Adapter] Non-text content blocks not supported on ChatGPT, ignoring:\`,
      nonTextBlocks.map(b => b.type)
    );
  }
  
  const textContent = params.content
    .filter((block) => block.type === "text")
    .map((block) => block.text)
    .join("\n");
  // ...
}

⚠️ Important Issues

3. Reserved keys filtering is inconsistent

Location: packages/ui/src/adapters/openai.ts:564-577

The code filters keys starting with _, but then uses __mcp_* for internal keys. The documentation states single-underscore is reserved, but the implementation uses double-underscore.

Clarify: Either:

  • Reserve only double-underscore (__) and allow single underscore for users
  • Or document that single-underscore is truly reserved and no double-underscore keys will leak into user space

Current code filters single-underscore but that means __mcp_type and __mcp_textContent could potentially collide with user data if they manage to bypass validation.

4. Package.json dependency structure changed

Location: packages/ui-react-builder/package.json

Moving @typescript-eslint/types from devDependencies to dependencies is correct if it's a runtime dependency. However, the tsup.config comments say these are needed "at build time", which suggests they should be peerDependencies or devDependencies.

Question: Are these truly runtime dependencies, or are they only needed during the build process? If only build-time:

  • Keep them in devDependencies
  • Users of the package don't need to install them
  • The built artifacts don't reference them

If truly runtime (the built code imports them), then dependencies is correct.

5. Zod version bump from 4.0.0 → 4.3.5

This is a minor version bump and should be safe, but Zod 4 is still relatively new and the CLAUDE.md notes it has breaking changes from v3. Ensure this doesn't introduce any compatibility issues with existing validation schemas.


💡 Code Quality Suggestions

6. Type safety in capability extraction

Location: packages/ui/src/adapters/mcp.ts:651-689

The capability extraction uses type assertions that bypass type checking:

const sdkCaps = mcpCaps as Record<string, unknown>;
// ...
logging: sdkCaps.logging as HostCapabilities["logging"],

Suggestion: Create a helper function with proper validation:

function extractCapability<K extends keyof HostCapabilities>(
  sdkCaps: Record<string, unknown>,
  key: K
): HostCapabilities[K] | undefined {
  const value = sdkCaps[key];
  // Add runtime validation if needed
  return value as HostCapabilities[K];
}

7. Inconsistent null handling in useUpdateModelContext

Location: packages/ui-react/src/hooks.ts:298-304

The hook silently returns early with a console.warn if client is not available. Consider:

  • Throwing an error (matches other hook behavior)
  • Or documenting this graceful degradation in JSDoc
  • Or returning a boolean/status to indicate success

Current behavior might hide bugs where components try to use the hook before connection.

8. Test coverage for containerDimensions

The tests check that updateModelContext exists and can be called, but I don't see specific tests for the new containerDimensions parsing logic in the MCP adapter. Consider adding tests for:

  • Fixed dimensions (height + width)
  • Flexible height (height + maxWidth)
  • Flexible width (width + maxHeight)
  • Fully flexible (maxHeight + maxWidth)
  • Backward compatibility fallback to viewport

✅ Good Practices Observed

  1. Discriminated unions for ContentBlock types provide excellent type safety
  2. JSDoc documentation is thorough and explains platform differences
  3. Tests added for new functionality across all three adapters
  4. Backward compatibility maintained with viewport derivation from containerDimensions
  5. Error codes properly extended with INVALID_PARAMS
  6. Hook example in useUpdateModelContext documentation is clear and helpful
  7. Type exports properly included in index files

🏗️ Architecture Notes

The dual-protocol abstraction (MCP vs ChatGPT) is well-handled. The platform differences are clearly documented, particularly around how updateModelContext() works differently:

  • MCP: Pure context update
  • ChatGPT: Uses setState which persists + exposes to AI

This is a critical distinction and the documentation does a good job explaining when to use which method.


📋 Recommendations

Before merging:

  1. ✅ Fix the non-exhaustive switch statement in MCP adapter (critical)
  2. ✅ Add logging or documentation for dropped content blocks in OpenAI adapter
  3. ✅ Clarify reserved key prefix policy (single vs double underscore)
  4. ✅ Add tests for containerDimensions edge cases
  5. ✅ Run full test suite: `pnpm build && pnpm test && pnpm lint && pnpm typecheck`

Post-merge considerations:

  • Monitor for any issues with Zod 4.3.5 compatibility
  • Consider adding integration tests that verify updateModelContext behavior end-to-end
  • Document the package.json dependency structure decision in the learnings section

🎯 Overall Assessment

Quality: Good - The implementation is solid with proper types, tests, and documentation

Readiness: Needs fixes - Address the critical switch exhaustiveness issue and content handling inconsistency before merging

Complexity: Appropriate - The dual-protocol abstraction adds necessary complexity that's well-managed

This is a well-structured PR that adds significant functionality. The main issues are fixable patterns that will prevent future bugs. Great job on the documentation and type safety overall! 🚀

- Added exhaustiveness check for content block types in MCP adapter to throw an error for unsupported types.
- Updated OpenAI adapter to log dropped non-text content blocks, ensuring only text blocks are processed.

These changes improve error handling and logging for content block processing across adapters.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5d21a3 and 7ccdcca.

📒 Files selected for processing (2)
  • packages/ui/src/adapters/mcp.ts
  • packages/ui/src/adapters/openai.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ui/src/adapters/openai.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Express 5 (not 4) - async error handling works differently

Files:

  • packages/ui/src/adapters/mcp.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use strict TypeScript - no any types, use unknown with type narrowing instead
Use defineTool and defineUI (or defineReactUI for React components) for type inference
Use export type for type-only exports to prevent runtime imports of types

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

packages/ui/**/*.{ts,tsx,js,jsx}: Use createClient() with proper await - it is an async function, not synchronous
Handle both MCP and OpenAI response formats when calling tools
Ensure adapter.connect() is awaited before subscribing to events or rendering
Set up onHostContextChange subscription before first render to ensure theme applies correctly

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

Provide type parameter to createClient<typeof app.tools()>() for proper typed tool calls

Files:

  • packages/ui/src/adapters/mcp.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper

Applied to files:

  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Handle both MCP and OpenAI response formats when calling tools

Applied to files:

  • packages/ui/src/adapters/mcp.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-and-lint
  • GitHub Check: claude-review
🔇 Additional comments (5)
packages/ui/src/adapters/mcp.ts (5)

12-12: LGTM!

Type imports are correctly added for the new updateModelContext and containerDimensions functionality.

Also applies to: 48-50


281-296: LGTM - tolerant parsing aligns with existing patterns.

The containerDimensions parsing follows the file's established pattern of graceful/tolerant parsing. The isObjectLike check combined with defensive handling in deriveViewportFromContainerDimensions (which validates each numeric field individually) ensures robustness without strict upfront validation.


322-337: LGTM!

The method correctly derives viewport dimensions from containerDimensions with proper type guards and fallback to defaults. The defensive casting to Record<string, unknown> allows flexible property checking.


486-493: Good exhaustiveness check.

The never type pattern ensures TypeScript will flag any unhandled content block types at compile time, while the runtime throw provides a safety net.


663-697: LGTM!

The new ext-apps v0.4.0 capabilities (updateModelContext, message, sandbox) are correctly mapped from the SDK capabilities object, following the established pattern for capability extraction.

@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Code Review: ext-apps v0.4.0 Support

Critical Issue Found: Test/Implementation Mismatch

The implementation uses __mcp_type and __mcp_textContent (double underscore) in packages/ui/src/adapters/openai.ts:558, but tests expect _type and _textContent (single underscore) at packages/ui/tests/unit/openai-adapter.test.ts:429.

This will cause test failures. Please align the implementation with test expectations.


Other Issues:

  1. Protocol changed from openai to mcp in examples/weather-app/server/index.ts:118 - is this intentional?
  2. Unsafe type assertions for containerDimensions without runtime validation (packages/ui/src/adapters/mcp.ts:286)
  3. Missing type narrowing when accessing block.text after filtering (packages/ui/src/adapters/openai.ts:593)
  4. Incomplete test coverage for edge cases

Strengths:

  • Excellent documentation with clear JSDoc comments
  • Good backward compatibility via viewport derivation
  • Proper error handling with UIError codes
  • Type safety using discriminated unions
  • Clear PR description

Action Items:

Must fix:

  1. Fix _type vs __mcp_type mismatch
  2. Clarify protocol change in weather-app
  3. Run pnpm build && pnpm test && pnpm lint && pnpm typecheck

Recommended:
4. Add runtime validation for containerDimensions
5. Add edge case tests
6. Verify dependency placement in ui-react-builder

Overall: Good work! Main blocker is the test mismatch. Once fixed and build passes, ready to merge.

…rmat

- Modified the MCP adapter to return a nested resource object for content blocks, including both `uri` and `text/blob` fields.
- Enhanced handling for `resource_link` blocks to match the new structure, ensuring compatibility with ext-apps v0.4.0.

These changes improve the integration with external applications by standardizing the resource format.
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Pull Request Review

This PR adds support for ext-apps v0.4.0 features, including the new updateModelContext() method, containerDimensions, and new host capabilities. The implementation is well-structured with comprehensive documentation and test coverage.

✅ Strengths

  1. Excellent Type Safety

    • Discriminated unions for ContentBlock type (packages/ui/src/types.ts:451-456) ensure compile-time correctness
    • Proper TypeScript exhaustiveness checks in switch statements (packages/ui/src/adapters/mcp.ts:499-506)
    • Clean separation of concerns with protocol-specific adapters
  2. Comprehensive Documentation

    • Clear JSDoc comments explaining platform differences between MCP Apps and ChatGPT
    • Well-documented implementation notes about setState behavior on ChatGPT (packages/ui/src/adapters/openai.ts:529-553)
    • Helpful examples in React hook documentation (packages/ui-react/src/hooks.ts:274-292)
  3. Good Test Coverage

    • Tests added for all three adapters (MCP, OpenAI, Mock)
    • Tests cover both success and error cases
    • Proper mocking of external dependencies
  4. Backward Compatibility

    • Graceful handling of containerDimensions with fallback to viewport (packages/ui/src/adapters/mcp.ts:322-336)
    • Optional new fields don't break existing code

🔍 Code Quality Observations

  1. ContentBlock Type Design (packages/ui/src/types.ts:451-456)

    • The discriminated union pattern is excellent for type safety
    • Each content block type only exposes valid fields for that type
    • ✅ Good: Prevents invalid field combinations at compile time
  2. Reserved Key Filtering (packages/ui/src/adapters/openai.ts:561-577)

    • Filters underscore-prefixed keys from structuredContent
    • Uses double-underscore for internal keys (__mcp_*)
    • ✅ Good: Prevents collisions with user data
    • Note: The filtering only checks single underscore prefix, but internal keys use double underscore - this is intentional and correct
  3. Error Handling (packages/ui/src/adapters/mcp.ts:432-508)

    • Validates required fields for each content block type
    • Throws descriptive errors with proper error codes
    • ✅ Good: Clear error messages help debugging
  4. OpenAI Adapter Implementation (packages/ui/src/adapters/openai.ts:554-602)

    • Uses setState internally, which calls setWidgetState
    • Only supports text content blocks (others are dropped with logging)
    • ✅ Good: Graceful degradation with debug logging for dropped content

⚠️ Potential Issues

  1. Test Assertion Mismatch (packages/ui/tests/unit/openai-adapter.test.ts:428-432)

    expect(mockSetWidgetState).toHaveBeenCalledWith({
      _type: "modelContext",  // ❌ Expected
      itemCount: 3,
      total: 150,
    });

    However, the implementation uses __mcp_type (double underscore):

    const modelContext: Record<string, unknown> = {
      __mcp_type: "modelContext",  // ✅ Actual
    };

    Impact: Test expects _type but code produces __mcp_type. This test may be passing incorrectly or needs update.

  2. Missing Validation in MCP Adapter

    • The deriveViewportFromContainerDimensions method (packages/ui/src/adapters/mcp.ts:326-336) doesn't validate that at least one dimension is provided
    • If all fields are undefined, it falls back to defaults (which is fine), but no explicit validation
  3. Type Casting Without Validation (packages/ui/src/adapters/mcp.ts:315-317)

    deviceCapabilities: ctx.deviceCapabilities as HostContext["deviceCapabilities"],
    safeAreaInsets: ctx.safeAreaInsets as HostContext["safeAreaInsets"],

    Type assertions without runtime validation could mask incorrect data shapes

🔒 Security Considerations

  1. Reserved Key Filtering - ✅ Good security practice to prevent key collisions
  2. URI Handling - Resource URIs are not validated or sanitized; ensure upstream validation exists
  3. Data Size - No size limits on structuredContent or data fields; consider documenting limits

📊 Test Coverage

Good test coverage for the new functionality:

  • ✅ MCP adapter: Basic functionality tested (packages/ui/tests/unit/mcp-adapter.test.ts:259-282)
  • ✅ OpenAI adapter: Comprehensive tests including edge cases (packages/ui/tests/unit/openai-adapter.test.ts:409-486)
  • ✅ Mock adapter: Tests for test utilities (packages/ui/tests/unit/mock-adapter.test.ts:412)
  • ⚠️ Missing: Tests for validation errors (missing required fields)
  • ⚠️ Missing: Tests for containerDimensions derivation logic

🎯 Recommendations

  1. Fix test assertion in packages/ui/tests/unit/openai-adapter.test.ts:429 to match __mcp_type
  2. Add validation tests for content block required fields
  3. Add tests for deriveViewportFromContainerDimensions with various input combinations
  4. Consider adding runtime validation for type-casted objects (deviceCapabilities, safeAreaInsets)
  5. Document size limits for data and structuredContent fields if any exist

📝 Minor Suggestions

  1. The protocol change in weather-app example (packages/examples/weather-app/server/index.ts:118) from "openai" to "mcp" should be mentioned in PR description or reverted if unintentional
  2. Consider exporting ContainerDimensions type from ui-react package for advanced users

✅ Overall Assessment

This is a high-quality PR with:

  • ✅ Clean architecture and separation of concerns
  • ✅ Strong type safety with discriminated unions
  • ✅ Comprehensive documentation
  • ✅ Good test coverage
  • ⚠️ One test assertion issue to fix
  • 💡 Some validation tests to add

Recommendation: Approve with minor fixes suggested above.


Review performed following CLAUDE.md conventions for this codebase.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @packages/ui/src/adapters/mcp.ts:
- Around line 466-498: The resource_link branch currently returns a nested
ext-apps "resource" object; change the case "resource_link" handling in the
switch to return a flat ext-apps v0.4.0 shape with type: "resource_link" and
top-level fields uri: block.uri, name: block.name, description:
block.description (and keep the UIError check for missing uri using
UIError/UIErrorCode), and update or remove the misleading comment about ext-apps
expecting a nested resource object; also review the case "resource" mapping to
ensure it matches the spec and adjust its returned structure or comment
accordingly (symbols to edit: the switch cases for "resource" and
"resource_link" in packages/ui/src/adapters/mcp.ts).
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ccdcca and 4dad2ab.

📒 Files selected for processing (1)
  • packages/ui/src/adapters/mcp.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Express 5 (not 4) - async error handling works differently

Files:

  • packages/ui/src/adapters/mcp.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use strict TypeScript - no any types, use unknown with type narrowing instead
Use defineTool and defineUI (or defineReactUI for React components) for type inference
Use export type for type-only exports to prevent runtime imports of types

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

packages/ui/**/*.{ts,tsx,js,jsx}: Use createClient() with proper await - it is an async function, not synchronous
Handle both MCP and OpenAI response formats when calling tools
Ensure adapter.connect() is awaited before subscribing to events or rendering
Set up onHostContextChange subscription before first render to ensure theme applies correctly

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

Provide type parameter to createClient<typeof app.tools()>() for proper typed tool calls

Files:

  • packages/ui/src/adapters/mcp.ts
🧠 Learnings (2)
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper

Applied to files:

  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Handle both MCP and OpenAI response formats when calling tools

Applied to files:

  • packages/ui/src/adapters/mcp.ts
🧬 Code graph analysis (1)
packages/ui/src/adapters/mcp.ts (1)
packages/ui/src/types.ts (5)
  • ContainerDimensions (274-278)
  • HostContext (317-364)
  • UpdateModelContextParams (461-473)
  • ContentBlock (451-456)
  • HostCapabilities (15-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-and-lint
  • GitHub Check: claude-review
🔇 Additional comments (5)
packages/ui/src/adapters/mcp.ts (5)

12-12: LGTM!

Imports are properly organized and align with the new types for ext-apps v0.4.0 support.

Also applies to: 48-50


281-296: LGTM!

The containerDimensions parsing and viewport derivation logic correctly maintains backward compatibility. The fallback chain (containerDimensions → explicit viewport → defaults) ensures existing code relying on viewport continues to work.


322-337: LGTM!

The derivation logic correctly distinguishes between fixed dimensions (width/height → fall back to defaults) and flexible dimensions (maxWidth/maxHeight → fall back to undefined). This properly reflects the ContainerDimensions semantics where absence of max dimensions means no constraint.


499-506: Exhaustiveness check is well-implemented.

Using const exhaustiveCheck: never = block ensures TypeScript will produce a compile-time error if a new ContentBlock type is added but not handled in the switch statement. This is a good defensive pattern for union type handling.


706-710: LGTM!

The new v0.4.0 capability fields (updateModelContext, message, sandbox) are properly exposed through the unified HostCapabilities interface. The type assertion pattern is consistent with the existing SDK capability mapping above.

Comment thread packages/ui/src/adapters/mcp.ts
- Updated the MCP adapter to clarify the mapping of resource and resource_link formats to align with ext-apps v0.4.0 specifications.
- Adjusted comments for better understanding of the expected structure, ensuring that the 'name' field is now required for resource_link.

These changes enhance the clarity and correctness of the resource handling in the MCP adapter.
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Pull Request Review: ext-apps v0.4.0 Support

Summary

This PR successfully updates the framework to support ext-apps v0.4.0 features, including model context updates, container dimensions, and new host capabilities. The implementation is comprehensive and well-tested.

✅ Strengths

1. Excellent Type Safety

  • Discriminated unions for ContentBlock types provide compile-time validation
  • Exhaustiveness checking in the switch statement (lines 497-504 in mcp.ts) ensures all content types are handled
  • Proper use of type narrowing throughout

2. Strong Documentation

  • Clear JSDoc comments explaining platform differences (MCP vs ChatGPT)
  • Helpful examples in hook documentation
  • Well-documented breaking changes and migration paths

3. Backward Compatibility

  • deriveViewportFromContainerDimensions() maintains compatibility with old viewport API
  • Graceful degradation where features are not supported (e.g., OpenAI adapter drops non-text content blocks)

4. Good Test Coverage

  • Comprehensive tests for new updateModelContext functionality
  • Tests cover both adapters (MCP and OpenAI)
  • Edge cases are tested (missing SDK, error handling)

5. Clean Architecture

  • Protocol adapter abstraction keeps platform differences isolated
  • New functionality follows existing patterns consistently

🔍 Issues Found

Critical

None found - the core implementation is solid.

Major

1. OpenAI Adapter: State Key Naming Inconsistency

Location: packages/ui/src/adapters/openai.ts:558-559

const modelContext: Record<string, unknown> = {
  __mcp_type: "modelContext",  // Double underscore
};

But later:

modelContext.__mcp_textContent = textContent;  // Also double underscore

Issue: Documentation says "keys starting with underscore (_) are filtered" but uses double underscore (_mcp*) for internal keys. The distinction between single and double underscore is mentioned in code but not clearly documented in the JSDoc.

Recommendation: Update the JSDoc to clarify:

  • Single underscore (_foo): Reserved, filtered from user structuredContent
  • Double underscore (_mcp*): Internal framework use only

2. Test Assertion Mismatch

Location: packages/ui/tests/unit/openai-adapter.test.ts:428-432

The test expects _type but the implementation uses __mcp_type:

expect(mockSetWidgetState).toHaveBeenCalledWith({
  _type: "modelContext",  // Should be __mcp_type
  itemCount: 3,
  total: 150,
});

This test should fail but may be passing due to partial matching. Please update to match implementation.

Minor

3. Error Handling: Missing Validation Context

Location: packages/ui/src/adapters/mcp.ts:432-506

The content block validation throws errors for missing required fields, but there is no validation that data for image/audio blocks is actually base64-encoded. While this may be intentional (trusting caller), a malformed data field could cause runtime errors in the host.

Recommendation: Add a comment explaining that validation of data format is the caller responsibility, or add optional base64 validation.

4. Inconsistent Documentation

Location: packages/ui/src/adapters/openai.ts:527-552

The extensive JSDoc comment is excellent but slightly redundant with the AppsClient interface documentation. Consider extracting common documentation to reduce duplication.

5. Package Version Changes

The PR updates Zod from 4.0.0 to 4.3.5 across multiple packages. While this is good, the PR description does not mention why this specific version was chosen. Consider documenting any breaking changes from Zod if relevant.

6. ui-react-builder Dependency Reorganization

Moving @typescript-eslint/* from devDependencies to dependencies is correct (they are needed at runtime), but the comment in tsup.config.ts could be more specific about when they are used (build time vs runtime).

🎯 Suggestions

Performance

1. Reserved Key Filtering

Current code:

const reservedKeys = Object.keys(structuredContent).filter((key) => key.startsWith("_"));

This filters keys twice (once to log, once to skip). Consider:

Object.entries(structuredContent).forEach(([key, value]) => {
  if (key.startsWith("_")) {
    // log if needed
    return;
  }
  modelContext[key] = value;
});

Code Quality

1. Magic Strings

Consider extracting magic strings to constants:

const MODEL_CONTEXT_TYPE = "modelContext" as const;
const INTERNAL_KEY_PREFIX = "__mcp_" as const;

2. Error Messages Enhancement

The error messages are good, but could include more context:

throw new UIError(
  UIErrorCode.INVALID_PARAMS,
  `Image content block requires data field (type: ${block.type})`
);

Testing

1. Missing Edge Cases

Consider adding tests for:

  • Very large structuredContent (size limits)
  • Content blocks with extra unknown fields
  • Concurrent calls to updateModelContext
  • Empty content arrays and structuredContent objects

2. Integration Tests

The unit tests are solid, but consider adding integration tests that verify the full flow from React hook → client → adapter → protocol.

🔒 Security

No critical security issues found.

Minor note: The base64 decoding in readResource (mcp.ts:578) could potentially throw on malformed input. Wrap in try-catch if untrusted input is possible.

📊 Performance

No significant performance concerns. The implementation is efficient for typical use cases. For apps sending frequent context updates with large payloads, document best practices in the user guide.

✅ Checklist Compliance

Per CLAUDE.md requirements:

  • ✅ Follows Zod 4 patterns
  • ✅ Uses proper TypeScript strictness (no any found)
  • ✅ Tests mirror source structure
  • ✅ Exports through index.ts
  • ✅ No circular dependencies detected
  • ⚠️ Build/test verification: Unable to run full suite in review environment, but CI should validate this

🎉 Conclusion

Recommendation: APPROVE with minor fixes

This is a high-quality PR that successfully implements ext-apps v0.4.0 support. The architecture is sound, the code is well-tested, and the documentation is thorough. The issues identified are minor and mostly cosmetic.

Required Actions Before Merge:

  1. Fix the test assertion on line 428-432 of openai-adapter.test.ts (__mcp_type consistency)
  2. Clarify underscore prefix documentation in types.ts

Optional Improvements:

  1. Add constants for magic strings
  2. Consider the performance optimizations for reserved key filtering
  3. Add integration tests if time permits

Great work on maintaining backward compatibility while adding substantial new functionality! 🚀

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dad2ab and 17ee330.

📒 Files selected for processing (1)
  • packages/ui/src/adapters/mcp.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Express 5 (not 4) - async error handling works differently

Files:

  • packages/ui/src/adapters/mcp.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use strict TypeScript - no any types, use unknown with type narrowing instead
Use defineTool and defineUI (or defineReactUI for React components) for type inference
Use export type for type-only exports to prevent runtime imports of types

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

packages/ui/**/*.{ts,tsx,js,jsx}: Use createClient() with proper await - it is an async function, not synchronous
Handle both MCP and OpenAI response formats when calling tools
Ensure adapter.connect() is awaited before subscribing to events or rendering
Set up onHostContextChange subscription before first render to ensure theme applies correctly

Files:

  • packages/ui/src/adapters/mcp.ts
packages/ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/ui/AGENTS.md)

Provide type parameter to createClient<typeof app.tools()>() for proper typed tool calls

Files:

  • packages/ui/src/adapters/mcp.ts
🧠 Learnings (2)
📚 Learning: 2026-01-11T17:08:36.676Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui-react/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:36.676Z
Learning: Applies to packages/ui-react/**/*.{tsx,jsx} : Ensure components using MCP hooks (`useAppsClient`, `useToolResult`, `useHostContext`, `useDocumentTheme`, `useHostStyleVariables`) are rendered within an `AppsProvider` context wrapper

Applied to files:

  • packages/ui/src/adapters/mcp.ts
📚 Learning: 2026-01-11T17:08:42.176Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: packages/ui/AGENTS.md:0-0
Timestamp: 2026-01-11T17:08:42.176Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Handle both MCP and OpenAI response formats when calling tools

Applied to files:

  • packages/ui/src/adapters/mcp.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-and-lint
  • GitHub Check: claude-review
🔇 Additional comments (6)
packages/ui/src/adapters/mcp.ts (6)

12-12: LGTM!

Type imports are correctly structured using import type pattern to prevent runtime imports.

Also applies to: 48-50


281-310: LGTM!

The containerDimensions parsing and backward-compatible viewport derivation are well-implemented. The fallback chain (containerDimensions → explicit viewport → defaults) ensures existing code continues to work while supporting the new ext-apps v0.4.0 semantics.


322-337: LGTM!

The viewport derivation correctly handles both fixed (width/height) and flexible (maxWidth/maxHeight) semantics from containerDimensions while falling back to defaults for missing values.


424-465: LGTM!

The updateModelContext method properly validates connection state and content block requirements. Error handling uses appropriate UIError codes, and the text/image/audio block mappings with sensible mimeType defaults are well-structured.


481-506: LGTM - resource_link mapping now uses correct flat structure.

The resource_link case now correctly uses the flat structure with type: "resource_link", uri, name, and optional description as required by ext-apps v0.4.0. The fallback of name to uri ensures the required field is always present.

The exhaustiveness check with never type provides compile-time safety if new content block types are added.


674-708: LGTM!

The capability mapping correctly exposes the new ext-apps v0.4.0 capabilities (updateModelContext, message, sandbox) while maintaining the existing MCP SDK capability mappings. The casting to Record<string, unknown> is appropriate for handling the external library's dynamic capability structure.

Comment on lines +466 to +480
case "resource":
if (!block.uri) {
throw new UIError(
UIErrorCode.INVALID_PARAMS,
"Resource content block requires 'uri' field"
);
}
// Map to ext-apps v0.4.0 resource format (nested structure)
return {
type: "resource" as const,
resource: {
uri: block.uri,
text: block.description ?? block.uri,
},
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the file structure and context around the resource mapping
cd packages/ui && find . -name "*.ts" -o -name "*.tsx" | head -20

Repository: AndurilCode/mcp-apps-kit

Length of output: 523


🏁 Script executed:

# Search for ext-apps references and resource-related code
rg "ext-apps" --type ts --type tsx -A 3 -B 3

Repository: AndurilCode/mcp-apps-kit

Length of output: 95


🏁 Script executed:

# Look for tests or documentation about resource content blocks
rg "resource.*content.*block|content.*block.*resource" --type ts --type tsx -i

Repository: AndurilCode/mcp-apps-kit

Length of output: 95


🌐 Web query:

ext-apps v0.4.0 MCP resource content block format specification

💡 Result:

Short summary of the ext-apps v0.4.0 MCP "resource content block" format

  • Content blocks are polymorphic objects with a type discriminator (e.g., "text", "image", "audio", "resource" (embedded), "resourceLink"). [1][2]
  • Common/metadata fields:
    • name (display filename), title (optional), uri (resource URI), mimeType / mime_type (optional), annotations such as audience, lastModified (ISO‑8601), priority (0.0–1.0). [2][3]
  • Text block ("text"):
    • type: "text"
    • text: UTF‑8 string
    • optional mimeType/mime_type for richer typing. [2][3]
  • Binary blocks ("image", "audio", etc.):
    • type: e.g., "image" or "audio"
    • blob (base64-encoded data) or data field, plus mimeType/mime_type. [2][3]
  • Embedded resource block ("resource"):
    • type: "resource"
    • resource: object containing resource metadata and either text or blob content (uri, name, mimeType, text or blob). [1][2][3]
  • Resource link ("resourceLink"):
    • type: "resourceLink"
    • name, uri (reference only — no inline content). [2]
  • Implementation notes:
    • text and blob are mutually exclusive for a single content item; URIs are canonical identifiers. [3]

Sources:
[1] modelcontextprotocol/ext-apps (repo & SDK).
[2] Model Context Protocol — Resources / schema (MCP specification).
[3] McpServer.Resource.Content (HTTP MCP Server v0.4.0) documentation.


🏁 Script executed:

# Find the ContentBlock or resource block type definition
rg "ContentBlock|resource.*block" --type ts -B 3 -A 5 -i

Repository: AndurilCode/mcp-apps-kit

Length of output: 11896


🏁 Script executed:

# Look at the imports and surrounding context in the mcp.ts file
head -100 packages/ui/src/adapters/mcp.ts | tail -50

Repository: AndurilCode/mcp-apps-kit

Length of output: 1814


🏁 Script executed:

# Search for where this function is defined and what input types it expects
rg "case.*resource" packages/ui/src/adapters/mcp.ts -B 10 -A 15

Repository: AndurilCode/mcp-apps-kit

Length of output: 1449


🏁 Script executed:

# Look for any test files that show resource content block handling
rg "resource" packages/ui/tests -l

Repository: AndurilCode/mcp-apps-kit

Length of output: 152


The resource block mapping has a semantic issue: description should not be mapped to text field, and defaulting to uri is incorrect.

The code maps the input description field to the output text field in the resource object. However, description is metadata about the resource, not its actual content. Per the ext-apps v0.4.0 specification, the text field should contain the actual resource content. Additionally, falling back to uri when description is absent (line 473) is semantically wrong—a URI is not resource content.

The input ContentBlock type for resource only provides uri and optional description fields, with no actual text or blob content. The current mapping conflates these concepts. Consider whether:

  1. description should be used as-is (preserving it as separate metadata, not as content), or
  2. The input type needs to be extended to include the actual resource content

@gabrypavanello
gabrypavanello merged commit 9c2a45b into main Jan 14, 2026
6 checks passed
@gabrypavanello
gabrypavanello deleted the claude/update-ext-apps-0.4.0-0g15K branch January 14, 2026 01:08
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.

3 participants