Repository navigation
Refactor MCP generated UI shell rendering - #146
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements a DOM-based generated-UI shell render pipeline (preserving head/body slots, classifying/executing scripts), introduces Changes
Sequence DiagramsequenceDiagram
participant Host as Host (page)
participant Runtime as KodyWidgetRuntime
participant Iframe as Generated UI iframe
Host->>Runtime: updateGeneratedUiRuntimeBootstrap(bootstrapA)
Runtime->>Runtime: normalizeBootstrap() / applyBootstrapToRuntimeState()
Host->>Iframe: postMessage(renderInline, paramsA)
Iframe->>Iframe: parse HTML, ensure head/body mount slots, classify scripts
Iframe->>Iframe: remount managed head/body, execute classified scripts
Iframe->>Host: postMessage(ui/initialize)
Iframe->>Host: postMessage(ui/notifications/size-changed)
Iframe->>Host: postMessage(tools/call execute)
Host->>Runtime: updateGeneratedUiRuntimeBootstrap(bootstrapB)
Runtime->>Runtime: normalizeBootstrap() / applyBootstrapToRuntimeState()
Host->>Iframe: postMessage(renderSaved, paramsB)
Iframe->>Iframe: remount managed nodes for saved render, execute scripts
Iframe->>Host: postMessage(tools/call ui_load_app_source)
Iframe->>Host: postMessage(tools/call execute)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kentcdodds
marked this pull request as ready for review
April 10, 2026 02:12
Contributor
|
🔎 Preview deployed: https://kody-pr-146.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts (1)
225-257: Consider extending test coverage to verify appSession update.The test validates that
paramsare updated correctly, but doesn't verify that theappSession(token and endpoints) is also applied. Since the function updates both, it would strengthen the test to assert both are propagated.🧪 Proposed extension to verify appSession
expect(kodyWidget.params).toEqual({ owner: 'updated', limit: 3 }) + + // Access internal state to verify appSession was updated + const state = (globalThis as any).__kodyWidgetRuntimeState + expect(state?.appSession?.token).toBe('token-2') + expect(state?.appSession?.endpoints?.execute).toBe( + 'https://kody.example/ui-api/next/execute', + ) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts` around lines 225 - 257, The test currently checks that params are updated after calling updateGeneratedUiRuntimeBootstrap but doesn't assert that the new appSession was applied; update the test (around getKodyWidgetRuntimeStateForTest(), updateGeneratedUiRuntimeBootstrap(...) and the existing expect(kodyWidget.params) assertion) to also assert that kodyWidget.appSession.token equals 'token-2' and that kodyWidget.appSession.endpoints matches the updated endpoints object (source, execute, secrets, deleteSecret) to ensure the session token and endpoint URLs were propagated.packages/worker/client/mcp-apps/kody-ui-utils.ts (1)
1219-1221: Add origin validation to the message event listener for defense-in-depth.While
handleBridgeResponseMessageandhandleLifecycleMessagedo validate message structure viaisRecord()checks and defensive early returns, the message listener itself lacks origin validation. Adding a check likeif (event.origin !== expectedParentOrigin) returnwould provide an additional layer of security before the message reaches the handlers, preventing even structurally-valid messages from untrusted origins from being processed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1219 - 1221, Add an origin check before delegating incoming postMessage events so only trusted origins reach hostBridge.handleHostMessage: inside the globalThis.window.addEventListener('message', ...) callback, compare event.origin to a trusted origin (e.g., an expectedParentOrigin constant or a value from hostBridge like hostBridge.expectedParentOrigin) and return early if it does not match, then call hostBridge.handleHostMessage(event.data); this provides defense-in-depth by blocking messages from untrusted origins before they hit handleHostMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts`:
- Around line 225-257: The test currently checks that params are updated after
calling updateGeneratedUiRuntimeBootstrap but doesn't assert that the new
appSession was applied; update the test (around
getKodyWidgetRuntimeStateForTest(), updateGeneratedUiRuntimeBootstrap(...) and
the existing expect(kodyWidget.params) assertion) to also assert that
kodyWidget.appSession.token equals 'token-2' and that
kodyWidget.appSession.endpoints matches the updated endpoints object (source,
execute, secrets, deleteSecret) to ensure the session token and endpoint URLs
were propagated.
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1219-1221: Add an origin check before delegating incoming
postMessage events so only trusted origins reach hostBridge.handleHostMessage:
inside the globalThis.window.addEventListener('message', ...) callback, compare
event.origin to a trusted origin (e.g., an expectedParentOrigin constant or a
value from hostBridge like hostBridge.expectedParentOrigin) and return early if
it does not match, then call hostBridge.handleHostMessage(event.data); this
provides defense-in-depth by blocking messages from untrusted origins before
they hit handleHostMessage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a89a2329-8f9d-4855-8c32-423711561f1f
📒 Files selected for processing (4)
e2e/generated-ui-shell.spec.tspackages/worker/client/mcp-apps/kody-ui-utils.node.test.tspackages/worker/client/mcp-apps/kody-ui-utils.tspackages/worker/client/mcp-apps/kody-widget-runtime.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/client/mcp-apps/kody-ui-utils.ts (1)
1153-1219:⚠️ Potential issue | 🔴 CriticalGuard shell renders with a generation token.
renderGeneratedUiShellDocument()now awaits external/module scripts, but nothing cancels an older render once a newer envelope arrives. That lets stale renders resume later and append old scripts into the current slots, mixing two app versions in one document.🛡️ Suggested direction
+ let renderGeneration = 0 const renderEnvelope = async (envelope: RenderEnvelope | null) => { + const generation = ++renderGeneration latestEnvelope = envelope if (!envelope) { return } const renderCode = async (code: string, runtime: AppRuntime) => { if (!shellRenderState) { return } const renderSource = buildGeneratedUiShellRenderSource({ code, runtime, baseHref, }) await renderGeneratedUiShellDocument({ state: shellRenderState, htmlSource: renderSource.htmlSource, preserveDocumentChrome: renderSource.preserveDocumentChrome, + isCurrent: () => generation === renderGeneration, }) }Then bail out inside
renderGeneratedUiShellDocument()before DOM writes and after each awaited script ifisCurrent()is false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1153 - 1219, The render flow can let older async renders resume and mutate the DOM; to fix, introduce a short-lived generation token (e.g., renderToken) updated at the start of renderEnvelope and capture it in each async path (renderCode, renderError, and before/after calling resolveSavedAppCode), then pass that token into renderGeneratedUiShellDocument (or attach it to shellRenderState) and have renderGeneratedUiShellDocument check the token and bail (return) before any DOM writes and after awaiting external/module scripts if the token is no longer current; ensure every early-return path in renderEnvelope (inline_code, missing appId, catch) also checks the token (compare latestEnvelope or token) so only the most recent envelope's render proceeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1193-1199: In the envelope.mode === 'inline_code' branch the call
to renderCode(envelope.code, envelope.runtime ?? 'html') can throw and is not
caught, so wrap that await call in a try/catch (similar to the saved-apps path)
and on error call await renderError(...) with the error details and return;
ensure you still validate envelope.code first and preserve the runtime fallback
(envelope.runtime ?? 'html') when invoking renderCode.
- Around line 712-724: The function getShellScriptExecutionMode currently
returns 'ignore' for normalizedType === 'importmap', which drops importmap
scripts and breaks module specifier resolution; change the behavior so importmap
scripts are preserved (e.g., return 'data' for normalizedType === 'importmap')
so they are processed before replaying modules; keep 'speculationrules' as
'ignore' if desired, and leave other branches (module -> 'module', non-classic
-> 'data', classic -> 'native-classic'|'isolated-classic') intact; update the
conditional in getShellScriptExecutionMode and ensure callers that treat 'data'
scripts will retain and re-insert import maps.
---
Outside diff comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1153-1219: The render flow can let older async renders resume and
mutate the DOM; to fix, introduce a short-lived generation token (e.g.,
renderToken) updated at the start of renderEnvelope and capture it in each async
path (renderCode, renderError, and before/after calling resolveSavedAppCode),
then pass that token into renderGeneratedUiShellDocument (or attach it to
shellRenderState) and have renderGeneratedUiShellDocument check the token and
bail (return) before any DOM writes and after awaiting external/module scripts
if the token is no longer current; ensure every early-return path in
renderEnvelope (inline_code, missing appId, catch) also checks the token
(compare latestEnvelope or token) so only the most recent envelope's render
proceeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddbe557d-9843-4f9c-931f-9d2d412f20e9
📒 Files selected for processing (1)
packages/worker/client/mcp-apps/kody-ui-utils.ts
Comment on lines
1193
to
1199
| if (envelope.mode === 'inline_code') { | ||
| if (!envelope.code) { | ||
| writeDocument( | ||
| renderGeneratedUiErrorDocument( | ||
| 'The tool result did not include inline code.', | ||
| ), | ||
| ) | ||
| await renderError('The tool result did not include inline code.') | ||
| return | ||
| } | ||
| writeDocument(buildDocument(envelope.code, envelope.runtime ?? 'html')) | ||
| activateMcpGeneratedUiRuntime( | ||
| hostBridge, | ||
| mcpRuntimeBootstrap, | ||
| latestRenderDataRef, | ||
| ) | ||
| await renderCode(envelope.code, envelope.runtime ?? 'html') | ||
| return |
There was a problem hiding this comment.
Inline-code renders need the same error fallback as saved apps.
renderCode() can now throw, but the inline_code branch does not catch it. A bad external/module script here leaves the shell partially rendered with no error document.
💡 Proposed fix
if (envelope.mode === 'inline_code') {
if (!envelope.code) {
await renderError('The tool result did not include inline code.')
return
}
- await renderCode(envelope.code, envelope.runtime ?? 'html')
+ try {
+ await renderCode(envelope.code, envelope.runtime ?? 'html')
+ } catch (error) {
+ if (latestEnvelope !== envelope) return
+ await renderError(
+ error instanceof Error ? error.message : 'Unknown render error.',
+ )
+ }
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1193 - 1199,
In the envelope.mode === 'inline_code' branch the call to
renderCode(envelope.code, envelope.runtime ?? 'html') can throw and is not
caught, so wrap that await call in a try/catch (similar to the saved-apps path)
and on error call await renderError(...) with the error details and return;
ensure you still validate envelope.code first and preserve the runtime fallback
(envelope.runtime ?? 'html') when invoking renderCode.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Test joins script lines without newlines breaking ASI
- Updated the test HTML assembly to join lines with newlines so inline and module scripts retain valid statement separation.
- ✅ Fixed: App session lost during bootstrap normalization round-trip
- Relaxed app session coercion to accept bootstrap sessions without sessionId/expiresAt while preserving token and endpoints for HTTP execution.
Preview (21023b45e6)
diff --git a/e2e/generated-ui-shell.spec.ts b/e2e/generated-ui-shell.spec.ts
new file mode 100644
--- /dev/null
+++ b/e2e/generated-ui-shell.spec.ts
@@ -1,0 +1,246 @@
+import { expect, test } from '@playwright/test'
+
+test('generated UI shell rerenders inline and saved apps without document rewrites', async ({
+ page,
+ baseURL,
+}) => {
+ if (!baseURL) {
+ throw new Error('Playwright baseURL is required for generated UI shell tests.')
+ }
+
+ const runtimeUrl = new URL('/dev/generated-ui', baseURL).toString()
+ const inlineHtml = [
+ '<main id="inline-app"></main>',
+ '<script>',
+ "const btn = document.createElement('button')",
+ "btn.id = 'first-inline-button'",
+ "btn.textContent = 'First inline button'",
+ "document.querySelector('#inline-app')?.append(btn)",
+ '</script>',
+ '<script>',
+ "const btn = document.createElement('button')",
+ "btn.id = 'second-inline-button'",
+ "btn.textContent = 'Second inline button'",
+ "document.querySelector('#inline-app')?.append(btn)",
+ "window.kodyWidget.executeCode('async () => ({ phase: \"inline\", owner: window.kodyWidget.params.owner })').then((result) => {",
+ "\tdocument.body.dataset.inlineExecute = JSON.stringify(result)",
+ '})',
+ '</script>',
+ ].join('\n')
+ const savedHtml = [
+ '<!doctype html>',
+ '<html lang="en" data-shell-phase="saved">',
+ '<head>',
+ '<title>Saved app</title>',
+ '<style>body { font-family: system-ui, sans-serif; }</style>',
+ '</head>',
+ '<body data-shell-body="saved">',
+ '<main id="saved-app"><button id="saved-app-button">Saved app</button></main>',
+ '<script type="module">',
+ "const result = await window.kodyWidget.executeCode('async () => ({ phase: \"saved\", owner: window.kodyWidget.params.owner })')",
+ "window.document.body.dataset.savedExecute = JSON.stringify(result)",
+ "window.document.body.dataset.savedParams = JSON.stringify(window.kodyWidget.params)",
+ "window.document.documentElement.dataset.savedHtmlAttr = window.document.documentElement.getAttribute('data-shell-phase') ?? 'missing'",
+ '</script>',
+ '</body>',
+ '</html>',
+ ].join('\n')
+
+ await page.setContent(`
+ <!doctype html>
+ <html lang="en">
+ <body>
+ <iframe
+ id="generated-ui-frame"
+ src=${JSON.stringify(runtimeUrl)}
+ style="width: 960px; height: 720px; border: 0;"
+ ></iframe>
+ <script>
+ const frame = document.getElementById('generated-ui-frame')
+ const hostState = {
+ renderData: undefined,
+ lastSize: null,
+ toolCalls: [],
+ savedSource: {
+ code: ${JSON.stringify(savedHtml)},
+ runtime: 'html',
+ },
+ }
+ window.__generatedUiHostState = hostState
+ function postToFrame(message) {
+ frame.contentWindow?.postMessage(message, '*')
+ }
+ function postRenderData() {
+ postToFrame({
+ type: 'ui-lifecycle-iframe-render-data',
+ payload: {
+ renderData: hostState.renderData,
+ },
+ })
+ }
+ window.__generatedUiHostActions = {
+ renderInline(params) {
+ hostState.renderData = {
+ toolOutput: {
+ renderSource: 'inline_code',
+ code: ${JSON.stringify(inlineHtml)},
+ runtime: 'html',
+ params,
+ },
+ }
+ postRenderData()
+ },
+ renderSaved(params) {
+ hostState.renderData = {
+ toolOutput: {
+ renderSource: 'saved_app',
+ appId: 'saved-app-123',
+ params,
+ },
+ }
+ postRenderData()
+ },
+ }
+ window.addEventListener('message', (event) => {
+ if (event.source !== frame.contentWindow) {
+ return
+ }
+ const message = event.data
+ if (!message || typeof message !== 'object') {
+ return
+ }
+ if (message.type === 'ui-request-render-data') {
+ postRenderData()
+ return
+ }
+ if (message.jsonrpc === '2.0' && message.method === 'ui/initialize') {
+ postToFrame({
+ jsonrpc: '2.0',
+ id: message.id,
+ result: {
+ protocolVersion: '2026-01-26',
+ hostInfo: {
+ name: 'playwright-shell-host',
+ version: '1.0.0',
+ },
+ hostCapabilities: {
+ message: { text: {} },
+ serverTools: {},
+ },
+ hostContext: hostState.renderData ?? {},
+ },
+ })
+ return
+ }
+ if (
+ message.jsonrpc === '2.0' &&
+ message.method === 'ui/notifications/size-changed'
+ ) {
+ hostState.lastSize = message.params ?? null
+ return
+ }
+ if (message.jsonrpc === '2.0' && message.method === 'tools/call') {
+ hostState.toolCalls.push(message.params ?? {})
+ if (message.params?.name === 'ui_load_app_source') {
+ postToFrame({
+ jsonrpc: '2.0',
+ id: message.id,
+ result: {
+ structuredContent: hostState.savedSource,
+ },
+ })
+ return
+ }
+ if (message.params?.name === 'execute') {
+ postToFrame({
+ jsonrpc: '2.0',
+ id: message.id,
+ result: {
+ structuredContent: {
+ result: {
+ phase:
+ hostState.toolCalls.filter(
+ (call) => call?.name === 'execute',
+ ).length === 1
+ ? 'inline'
+ : 'saved',
+ owner:
+ hostState.renderData?.toolOutput?.params?.owner ??
+ null,
+ },
+ },
+ },
+ })
+ }
+ }
+ })
+ </script>
+ </body>
+ </html>
+ `)
+
+ await page.waitForSelector('#generated-ui-frame')
+ await page.evaluate(() => {
+ ;(window as typeof window & {
+ __generatedUiHostActions: {
+ renderInline: (params: Record<string, unknown>) => void
+ }
+ }).__generatedUiHostActions.renderInline({ owner: 'alpha' })
+ })
+
+ const frame = page.frameLocator('#generated-ui-frame')
+ await expect(
+ frame.getByRole('button', { name: 'First inline button' }),
+ ).toBeVisible()
+ await expect(
+ frame.getByRole('button', { name: 'Second inline button' }),
+ ).toBeVisible()
+ await expect(frame.locator('body')).toHaveAttribute(
+ 'data-inline-execute',
+ /alpha/,
+ )
+ await expect
+ .poll(async () => {
+ return await page.evaluate(() => {
+ return (
+ (window as typeof window & {
+ __generatedUiHostState: { lastSize?: { height?: number } | null }
+ }).__generatedUiHostState.lastSize?.height ?? 0
+ )
+ })
+ })
+ .toBeGreaterThan(0)
+
+ await page.evaluate(() => {
+ ;(window as typeof window & {
+ __generatedUiHostActions: {
+ renderSaved: (params: Record<string, unknown>) => void
+ }
+ }).__generatedUiHostActions.renderSaved({ owner: 'beta', count: 2 })
+ })
+
+ await expect(
+ frame.getByRole('button', { name: 'Saved app' }),
+ ).toBeVisible()
+ await expect(frame.locator('html')).toHaveAttribute('data-shell-phase', 'saved')
+ await expect(frame.locator('body')).toHaveAttribute('data-shell-body', 'saved')
+ await expect(frame.locator('body')).toHaveAttribute('data-saved-execute', /beta/)
+ await expect(frame.locator('body')).toHaveAttribute('data-saved-params', /beta/)
+ await expect(frame.locator('html')).toHaveAttribute(
+ 'data-saved-html-attr',
+ 'saved',
+ )
+ await expect
+ .poll(async () => {
+ return await page.evaluate(() => {
+ return (
+ (window as typeof window & {
+ __generatedUiHostState: {
+ toolCalls: Array<{ name?: string }>
+ }
+ }).__generatedUiHostState.toolCalls.map((call) => call.name)
+ )
+ })
+ })
+ .toEqual(['execute', 'ui_load_app_source', 'execute'])
+})
diff --git a/packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts b/packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts
--- a/packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts
+++ b/packages/worker/client/mcp-apps/kody-ui-utils.node.test.ts
@@ -1,6 +1,9 @@
import { expect, test, vi } from 'vitest'
import * as uiUtils from './kody-ui-utils.ts'
-import { getKodyWidgetRuntimeStateForTest } from './kody-widget-runtime.ts'
+import {
+ getKodyWidgetRuntimeStateForTest,
+ updateGeneratedUiRuntimeBootstrap,
+} from './kody-widget-runtime.ts'
const {
absolutizeHtmlAttributeUrls,
@@ -219,6 +222,40 @@
await expect(kodyWidget.executeCode('return "ok"')).resolves.toBe('ok')
})
+test('runtime bootstrap updates refresh params and app session without reinit', () => {
+ const runtimeState = getKodyWidgetRuntimeStateForTest()
+ runtimeState.reset()
+ runtimeState.install({
+ mode: 'mcp',
+ params: { owner: 'kody' },
+ appSession: {
+ token: 'token-1',
+ endpoints: {
+ source: 'https://kody.example/ui-api/app/source',
+ execute: 'https://kody.example/ui-api/app/execute',
+ secrets: 'https://kody.example/ui-api/app/secrets',
+ deleteSecret: 'https://kody.example/ui-api/app/secrets/delete',
+ },
+ },
+ })
+
+ updateGeneratedUiRuntimeBootstrap({
+ mode: 'mcp',
+ params: { owner: 'updated', limit: 3 },
+ appSession: {
+ token: 'token-2',
+ endpoints: {
+ source: 'https://kody.example/ui-api/next/source',
+ execute: 'https://kody.example/ui-api/next/execute',
+ secrets: 'https://kody.example/ui-api/next/secrets',
+ deleteSecret: 'https://kody.example/ui-api/next/secrets/delete',
+ },
+ },
+ })
+
+ expect(kodyWidget.params).toEqual({ owner: 'updated', limit: 3 })
+})
+
test('runtime-backed helpers time out if the runtime never becomes ready', async () => {
vi.useFakeTimers()
try {
diff --git a/packages/worker/client/mcp-apps/kody-ui-utils.ts b/packages/worker/client/mcp-apps/kody-ui-utils.ts
--- a/packages/worker/client/mcp-apps/kody-ui-utils.ts
+++ b/packages/worker/client/mcp-apps/kody-ui-utils.ts
@@ -4,6 +4,7 @@
import {
initializeGeneratedUiRuntime,
setGeneratedUiRuntimeHooks,
+ updateGeneratedUiRuntimeBootstrap,
} from './kody-widget-runtime.ts'
import {
buildGeneratedUiRuntimeImportMap,
@@ -38,8 +39,8 @@
type DisplayMode = 'inline' | 'fullscreen' | 'pip'
type AppSessionEnvelope = {
- sessionId: string
- expiresAt: string
+ sessionId?: string
+ expiresAt?: string
endpoints: {
source: string
execute: string
@@ -91,6 +92,39 @@
documentElement?: SizeMeasurementElement | null
}
+type ShellScriptExecutionMode =
+ | 'module'
+ | 'native-classic'
+ | 'isolated-classic'
+ | 'ignore'
+ | 'data'
+
+type ShellScriptDescriptor = {
+ target: 'head' | 'body'
+ executionMode: ShellScriptExecutionMode
+ attributes: Array<{ name: string; value: string }>
+ src: string | null
+ textContent: string
+}
+
+type ParsedShellRenderDocument = {
+ title: string | null
+ htmlAttributes: Array<{ name: string; value: string }>
+ bodyAttributes: Array<{ name: string; value: string }>
+ headNodes: Array<Node>
+ bodyNodes: Array<Node>
+ scripts: Array<ShellScriptDescriptor>
+}
+
+type GeneratedUiShellRenderState = {
+ headSlot: HTMLElement
+ bodySlot: HTMLElement
+ mountedHeadNodes: Array<Node>
+ managedHtmlAttributes: Set<string>
+ managedBodyAttributes: Set<string>
+ defaultTitle: string
+}
+
type HostToolResult = {
structuredContent?: unknown
isError?: boolean
@@ -172,19 +206,13 @@
function coerceAppSession(value: unknown): AppSessionEnvelope | null {
if (!isRecord(value)) return null
- if (
- typeof value.sessionId !== 'string' ||
- typeof value.expiresAt !== 'string'
- ) {
- return null
- }
const endpoints = coerceGeneratedUiEndpoints(value.endpoints)
if (!endpoints) {
return null
}
return {
- sessionId: value.sessionId,
- expiresAt: value.expiresAt,
+ sessionId: typeof value.sessionId === 'string' ? value.sessionId : undefined,
+ expiresAt: typeof value.expiresAt === 'string' ? value.expiresAt : undefined,
endpoints,
token: typeof value.token === 'string' ? value.token : undefined,
}
@@ -561,14 +589,334 @@
}
}
-function writeDocument(html: string) {
+const generatedUiUserHeadSlotSelector = '[data-generated-ui-user-head-slot="true"]'
+const generatedUiBodySlotSelector = '[data-generated-ui-body-slot="true"]'
+
+function ensureGeneratedUiShellRenderState():
+ | GeneratedUiShellRenderState
+ | null {
const documentRef = globalThis.document
+ if (!documentRef?.head || !documentRef.body) {
+ return null
+ }
+ const existingHeadSlot = documentRef.head.querySelector(
+ generatedUiUserHeadSlotSelector,
+ )
+ let headSlot: HTMLElement
+ if (existingHeadSlot instanceof HTMLElement) {
+ headSlot = existingHeadSlot
+ } else {
+ headSlot = documentRef.createElement('meta')
+ headSlot.setAttribute('data-generated-ui-user-head-slot', 'true')
+ documentRef.head.appendChild(headSlot)
+ }
+ const existingBodySlot = documentRef.body.querySelector(
+ generatedUiBodySlotSelector,
+ )
+ let bodySlot: HTMLElement
+ if (existingBodySlot instanceof HTMLElement) {
+ bodySlot = existingBodySlot
+ } else {
+ bodySlot = documentRef.createElement('div')
+ bodySlot.setAttribute('id', 'app')
+ bodySlot.setAttribute('data-generated-ui-root', '')
+ bodySlot.setAttribute('data-generated-ui-body-slot', 'true')
+ documentRef.body.insertBefore(bodySlot, documentRef.body.firstChild)
+ }
+ return {
+ headSlot,
+ bodySlot,
+ mountedHeadNodes: [],
+ managedHtmlAttributes: new Set<string>(),
+ managedBodyAttributes: new Set<string>(),
+ defaultTitle: documentRef.title,
+ }
+}
+
+function clearGeneratedUiShellHead(state: GeneratedUiShellRenderState) {
+ for (const node of state.mountedHeadNodes) {
+ node.parentNode?.removeChild(node)
+ }
+ state.mountedHeadNodes = []
+}
+
+function setManagedElementAttributes(
+ element: HTMLElement,
+ nextAttributes: Array<{ name: string; value: string }>,
+ managedAttributes: Set<string>,
+) {
+ for (const name of managedAttributes) {
+ element.removeAttribute(name)
+ }
+ managedAttributes.clear()
+ for (const attribute of nextAttributes) {
+ element.setAttribute(attribute.name, attribute.value)
+ managedAttributes.add(attribute.name)
+ }
+}
+
+function mountGeneratedUiShellHeadNodes(
+ state: GeneratedUiShellRenderState,
+ nodes: Array<Node>,
+) {
+ const documentRef = globalThis.document
if (!documentRef) return
- documentRef.open()
- documentRef.write(html)
- documentRef.close()
+ const parentNode = state.headSlot.parentNode
+ if (!parentNode) return
+ for (const node of nodes) {
+ const clone = documentRef.importNode(node, true)
+ parentNode.insertBefore(clone, state.headSlot)
+ state.mountedHeadNodes.push(clone)
+ }
}
+function mountGeneratedUiShellBodyNodes(
+ state: GeneratedUiShellRenderState,
+ nodes: Array<Node>,
+) {
+ const documentRef = globalThis.document
+ if (!documentRef) return
+ state.bodySlot.replaceChildren(
+ ...nodes.map((node) => documentRef.importNode(node, true)),
+ )
+}
+
+function preserveGeneratedUiHeadNode(node: Node) {
+ if (node instanceof HTMLTitleElement) return false
+ if (node instanceof HTMLBaseElement) return false
+ if (node instanceof HTMLMetaElement) {
+ return !node.hasAttribute('charset') && node.httpEquiv.length === 0
+ }
+ if (node.nodeType === Node.TEXT_NODE) {
+ return (node.textContent ?? '').trim().length > 0
+ }
+ return true
+}
+
+function isClassicJavascriptScriptType(type: string) {
+ return (
+ type === '' ||
+ type === 'application/ecmascript' ||
+ type === 'application/javascript' ||
+ type === 'text/ecmascript' ||
+ type === 'text/javascript'
+ )
+}
+
+function getShellScriptExecutionMode(script: HTMLScriptElement) {
+ const normalizedType = script.type.trim().toLowerCase()
+ if (normalizedType === 'importmap' || normalizedType === 'speculationrules') {
+ return 'ignore' as const
+ }
+ if (normalizedType === 'module') {
+ return 'module' as const
+ }
+ if (!isClassicJavascriptScriptType(normalizedType)) {
+ return 'data' as const
+ }
+ return script.src ? ('native-classic' as const) : ('isolated-classic' as const)
+}
+
+function collectElementAttributes(element: Element) {
+ return Array.from(element.attributes, (attribute) => ({
+ name: attribute.name,
+ value: attribute.value,
+ }))
+}
+
+function classifyGeneratedUiShellRenderDocument(input: {
+ documentRef: Document
+ preserveDocumentChrome: boolean
+}) {
+ const parsed: ParsedShellRenderDocument = {
+ title: input.preserveDocumentChrome ? (input.documentRef.title || null) : null,
+ htmlAttributes: input.preserveDocumentChrome
+ ? collectElementAttributes(input.documentRef.documentElement)
+ : [],
+ bodyAttributes: input.preserveDocumentChrome
+ ? collectElementAttributes(input.documentRef.body)
+ : [],
+ headNodes: [],
+ bodyNodes: [],
+ scripts: [],
+ }
+
+ for (const node of Array.from(input.documentRef.head.childNodes)) {
+ if (node instanceof HTMLScriptElement) {
+ const executionMode = getShellScriptExecutionMode(node)
+ if (executionMode === 'data') {
+ if (preserveGeneratedUiHeadNode(node)) {
+ parsed.headNodes.push(node)
+ }
+ continue
+ }
+ if (executionMode === 'ignore') {
+ continue
+ }
+ parsed.scripts.push({
+ target: 'head',
+ executionMode,
+ attributes: collectElementAttributes(node),
+ src: node.src || null,
+ textContent: node.textContent ?? '',
+ })
+ continue
+ }
+ if (preserveGeneratedUiHeadNode(node)) {
+ parsed.headNodes.push(node)
+ }
+ }
+
+ for (const node of Array.from(input.documentRef.body.childNodes)) {
+ if (node instanceof HTMLScriptElement) {
+ const executionMode = getShellScriptExecutionMode(node)
+ if (executionMode === 'data') {
+ parsed.bodyNodes.push(node)
+ continue
+ }
+ if (executionMode === 'ignore') {
+ continue
+ }
+ parsed.scripts.push({
+ target: 'body',
+ executionMode,
+ attributes: collectElementAttributes(node),
+ src: node.src || null,
+ textContent: node.textContent ?? '',
+ })
+ continue
+ }
+ parsed.bodyNodes.push(node)
+ }
+
+ return parsed
+}
+
+function wrapInlineClassicScriptForIsolation(source: string) {
+ return [';(function () {', source, '}).call(window);'].join('\n')
+}
+
+async function executeGeneratedUiShellScript(
+ state: GeneratedUiShellRenderState,
+ scriptDescriptor: ShellScriptDescriptor,
+) {
+ const documentRef = globalThis.document
+ if (!documentRef) return
+ const script = documentRef.createElement('script')
+ const insertScript = () => {
+ if (scriptDescriptor.target === 'head') {
+ state.headSlot.parentNode?.insertBefore(script, state.headSlot)
+ state.mountedHeadNodes.push(script)
+ return
+ }
+ state.bodySlot.appendChild(script)
+ }
+ for (const attribute of scriptDescriptor.attributes) {
+ if (attribute.name === 'src' || attribute.name === 'type') {
+ continue
+ }
+ script.setAttribute(attribute.name, attribute.value)
+ }
+ if (scriptDescriptor.executionMode === 'module') {
+ script.type = 'module'
+ }
+ if (scriptDescriptor.executionMode === 'isolated-classic') {
+ script.textContent = wrapInlineClassicScriptForIsolation(
+ scriptDescriptor.textContent,
+ )
+ insertScript()
+ return
+ }
+ if (!scriptDescriptor.src) {
+ script.textContent = scriptDescriptor.textContent
+ if (scriptDescriptor.executionMode === 'module') {
+ const loading = new Promise<void>((resolve, reject) => {
+ script.addEventListener('load', () => resolve(), { once: true })
+ script.addEventListener(
+ 'error',
+ () => {
+ reject(new Error('Failed to execute generated UI module script.'))
+ },
+ { once: true },
+ )
+ })
+ insertScript()
+ await loading
+ return
+ }
+ insertScript()
+ return
+ }
+ const loading = new Promise<void>((resolve, reject) => {
+ script.addEventListener('load', () => resolve(), { once: true })
+ script.addEventListener(
+ 'error',
+ () => {
+ reject(new Error(`Failed to load generated UI script: ${script.src}`))
+ },
+ { once: true },
+ )
+ })
+ if (scriptDescriptor.executionMode === 'native-classic') {
+ script.async = false
+ }
+ script.src = scriptDescriptor.src
+ insertScript()
+ await loading
+}
+
+function buildGeneratedUiShellRenderSource(input: {
+ code: string
+ runtime: AppRuntime
+ baseHref: string | null
+}) {
+ return {
+ htmlSource: renderGeneratedUiDocument({
+ code: input.code,
+ runtime: input.runtime,
+ headInjection: '',
+ baseHref: input.baseHref,
+ }),
+ preserveDocumentChrome:
+ input.runtime === 'html' && /<(?:!doctype|html|head|body)\b/i.test(input.code),
+ }
+}
+
+async function renderGeneratedUiShellDocument(input: {
+ state: GeneratedUiShellRenderState
+ htmlSource: string
+ preserveDocumentChrome: boolean
+}) {
+ const documentRef = globalThis.document
+ if (!documentRef) return
+ const parsedDocument = new DOMParser().parseFromString(
+ input.htmlSource,
+ 'text/html',
+ )
+ const classifiedDocument = classifyGeneratedUiShellRenderDocument({
+ documentRef: parsedDocument,
+ preserveDocumentChrome: input.preserveDocumentChrome,
+ })
+ clearGeneratedUiShellHead(input.state)
+ input.state.bodySlot.replaceChildren()
+ setManagedElementAttributes(
+ documentRef.documentElement,
+ classifiedDocument.htmlAttributes,
+ input.state.managedHtmlAttributes,
+ )
+ setManagedElementAttributes(
+ documentRef.body,
+ classifiedDocument.bodyAttributes,
+ input.state.managedBodyAttributes,
+ )
+ documentRef.title = classifiedDocument.title ?? input.state.defaultTitle
+ mountGeneratedUiShellHeadNodes(input.state, classifiedDocument.headNodes)
+ mountGeneratedUiShellBodyNodes(input.state, classifiedDocument.bodyNodes)
+ for (const scriptDescriptor of classifiedDocument.scripts) {
+ await executeGeneratedUiShellScript(input.state, scriptDescriptor)
+ }
+}
+
export type BuildGeneratedUiHeadInjectionInput = {
mode: Extract<GeneratedUiRuntimeMode, 'hosted' | 'mcp'>
params?: Record<string, unknown>
@@ -648,13 +996,15 @@
}
return (await hostBridge.requestDisplayMode(nextMode)) ?? null
}
+ updateGeneratedUiRuntimeBootstrap(bootstrap)
installGeneratedUiRuntimeHooks({
sendMessage: (text) => hostBridge.sendUserMessageWithFallback(text),
openLink: (url) => hostBridge.openLink(url),
requestDisplayMode,
executeCode: async (code, params) => {
+ const { appSession } = readGeneratedUiBootstrap()
const viaHttp = await executeCodeWithHttp(
- bootstrap.appSession,
+ appSession,
code,
params,
)
@@ -705,6 +1055,7 @@
},
}
let latestEnvelope: RenderEnvelope | null = null
+ const shellRenderState = ensureGeneratedUiShellRenderState()
const hostBridge = createWidgetHostBridge({
appInfo: {
@@ -799,50 +1150,51 @@
return
}
- const buildDocument = (code: string, runtime: AppRuntime) =>
- renderGeneratedUiDocument({
+ const mcpRuntimeBootstrap: GeneratedUiRuntimeBootstrap = {
+ mode: 'mcp',
+ params: envelope.params ?? {},
+ ...(envelope.appSession ? { appSession: envelope.appSession } : {}),
+ }
+ updateGeneratedUiRuntimeBootstrap(mcpRuntimeBootstrap)
+
+ const renderCode = async (code: string, runtime: AppRuntime) => {
+ if (!shellRenderState) {
+ return
+ }
+ const renderSource = buildGeneratedUiShellRenderSource({
code,
runtime,
- headInjection: buildGeneratedUiRuntimeHeadInjection({
- mode: 'mcp',
- params: envelope.params,
- appSession: envelope.appSession ?? undefined,
- baseHref,
- includeRuntimeScript: false,
- }),
baseHref,
})
+ await renderGeneratedUiShellDocument({
+ state: shellRenderState,
+ htmlSource: renderSource.htmlSource,
+ preserveDocumentChrome: renderSource.preserveDocumentChrome,
+ })
+ }
- const mcpRuntimeBootstrap: GeneratedUiRuntimeBootstrap = {
- mode: 'mcp',
- params: envelope.params ?? {},
- ...(envelope.appSession ? { appSession: envelope.appSession } : {}),
+ const renderError = async (message: string) => {
+ if (!shellRenderState) {
+ return
+ }
... diff truncated: showing 800 of 971 linesYou can send follow-ups to the cloud agent here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/worker/client/mcp-apps/kody-ui-utils.ts (2)
1187-1192:⚠️ Potential issue | 🟠 MajorHandle
inline_coderender failures the same way as saved apps.
renderCode()can now throw during script replay, but this branch lets the error escape. A broken inline app leaves the shell partially rendered instead of switching to the error document.Suggested fix
if (envelope.mode === 'inline_code') { if (!envelope.code) { await renderError('The tool result did not include inline code.') return } - await renderCode(envelope.code, envelope.runtime ?? 'html') + try { + await renderCode(envelope.code, envelope.runtime ?? 'html') + } catch (error) { + if (latestEnvelope !== envelope) return + await renderError( + error instanceof Error ? error.message : 'Unknown render error.', + ) + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1187 - 1192, In the inline_code branch (when envelope.mode === 'inline_code'), wrap the call to renderCode(envelope.code, ...) in a try/catch and on any thrown error call await renderError(...) (same message used for saved apps or a descriptive one) and return; also handle the missing envelope.code case as before; this ensures renderCode errors are caught and the UI switches to the error document instead of leaving a partially rendered shell.
706-709:⚠️ Potential issue | 🟠 MajorPreserve
importmapscripts instead of dropping them.Returning
'ignore'here removes the import map before the replay loop runs, so later module scripts can lose bare-specifier resolution on rerender.importmapneeds to survive classification and be inserted before dependent modules.Suggested fix
- if (normalizedType === 'importmap' || normalizedType === 'speculationrules') { + if (normalizedType === 'speculationrules') { return 'ignore' as const } + if (normalizedType === 'importmap') { + return 'data' as const + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 706 - 709, The function getShellScriptExecutionMode currently treats script.type === 'importmap' as 'ignore', which drops the importmap before replay and breaks bare-specifier resolution; change the branch in getShellScriptExecutionMode so that when normalizedType === 'importmap' it is preserved (e.g., return a non-ignoring execution mode such as 'preserve' or the existing mode you use for keeping scripts) instead of 'ignore', and ensure preserved importmap nodes are inserted before dependent module scripts during replay so module resolution remains correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 999-1007: The executeCode hook currently calls
readGeneratedUiBootstrap() and thus uses the shell's initial
window.__kodyGeneratedUiBootstrap (stale appSession); change executeCode in
installGeneratedUiRuntimeHooks to obtain the latest bootstrap from the runtime’s
shared ref/state that updateGeneratedUiRuntimeBootstrap updates (or accept the
current appSession via closure) before calling executeCodeWithHttp so
executeCodeWithHttp receives the up-to-date appSession; update references to
readGeneratedUiBootstrap() in the executeCode implementation to use the runtime
getter/shared ref instead.
---
Duplicate comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1187-1192: In the inline_code branch (when envelope.mode ===
'inline_code'), wrap the call to renderCode(envelope.code, ...) in a try/catch
and on any thrown error call await renderError(...) (same message used for saved
apps or a descriptive one) and return; also handle the missing envelope.code
case as before; this ensures renderCode errors are caught and the UI switches to
the error document instead of leaving a partially rendered shell.
- Around line 706-709: The function getShellScriptExecutionMode currently treats
script.type === 'importmap' as 'ignore', which drops the importmap before replay
and breaks bare-specifier resolution; change the branch in
getShellScriptExecutionMode so that when normalizedType === 'importmap' it is
preserved (e.g., return a non-ignoring execution mode such as 'preserve' or the
existing mode you use for keeping scripts) instead of 'ignore', and ensure
preserved importmap nodes are inserted before dependent module scripts during
replay so module resolution remains correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44ee7baf-a288-42ba-b088-b315b68b3965
📒 Files selected for processing (2)
e2e/generated-ui-shell.spec.tspackages/worker/client/mcp-apps/kody-ui-utils.ts
Comment on lines
+999
to
+1007
| updateGeneratedUiRuntimeBootstrap(bootstrap) | ||
| installGeneratedUiRuntimeHooks({ | ||
| sendMessage: (text) => hostBridge.sendUserMessageWithFallback(text), | ||
| openLink: (url) => hostBridge.openLink(url), | ||
| requestDisplayMode, | ||
| executeCode: async (code, params) => { | ||
| const { appSession } = readGeneratedUiBootstrap() | ||
| const viaHttp = await executeCodeWithHttp( | ||
| bootstrap.appSession, | ||
| appSession, |
There was a problem hiding this comment.
executeCode is still reading the shell's initial bootstrap.
updateGeneratedUiRuntimeBootstrap() updates runtime state, but this hook still pulls appSession from window.__kodyGeneratedUiBootstrap. After the first activation, rerenders with a new appSession keep using stale bootstrap data, so the HTTP execute path can fall back incorrectly or use the wrong credentials. Read from a shared “latest bootstrap” ref or from runtime state instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 999 - 1007,
The executeCode hook currently calls readGeneratedUiBootstrap() and thus uses
the shell's initial window.__kodyGeneratedUiBootstrap (stale appSession); change
executeCode in installGeneratedUiRuntimeHooks to obtain the latest bootstrap
from the runtime’s shared ref/state that updateGeneratedUiRuntimeBootstrap
updates (or accept the current appSession via closure) before calling
executeCodeWithHttp so executeCodeWithHttp receives the up-to-date appSession;
update references to readGeneratedUiBootstrap() in the executeCode
implementation to use the runtime getter/shared ref instead.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/client/mcp-apps/kody-ui-utils.ts (1)
41-51:⚠️ Potential issue | 🟡 MinorAppSessionEnvelope type change allows undefined sessionId/expiresAt — verify type consistency across assignment chain.
The change from required to optional
sessionIdandexpiresAtfields creates a type mismatch:AppSessionEnvelopehas optional fields (can beundefined), but downstreamStorageContextexpectsstring | null. Ingenerated-ui-api.ts, these optional fields are assigned directly intostorageContextobjects (lines 234, 298, 337) andsessionExpiresAt(line 340) without explicit null coalescing. While most consumers ofStorageContextuse defensive patterns (optional chaining?.or?? null), the type definition inconsistency should be addressed: either normalize tonullat assignment time usingcontext.sessionId ?? null, or updateStorageContextto acceptstring | null | undefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 41 - 51, The AppSessionEnvelope now allows sessionId and expiresAt to be undefined but assignments into StorageContext (and into sessionExpiresAt) assume string|null; to fix, normalize those optional fields at the assignment sites by coalescing undefined to null (e.g., use sessionId ?? null and expiresAt ?? null when building storageContext and when setting sessionExpiresAt), or alternatively update the StorageContext type to accept undefined as well; update the assignment logic in generated-ui-api.ts where storageContext and sessionExpiresAt are set (look for uses of storageContext, sessionId, expiresAt, and sessionExpiresAt) and make the null-coalescing change so types align.
♻️ Duplicate comments (2)
packages/worker/client/mcp-apps/kody-ui-utils.ts (2)
1189-1196:⚠️ Potential issue | 🟠 MajorInline code renders still lack error handling.
The
inline_codebranch at line 1194 callsawait renderCode(...)without a try/catch. If script execution fails (e.g., network error loading an external script), the shell will be left in a partial state with no error feedback.This was flagged in a previous review and remains unaddressed.
💡 Proposed fix
if (envelope.mode === 'inline_code') { if (!envelope.code) { await renderError('The tool result did not include inline code.') return } - await renderCode(envelope.code, envelope.runtime ?? 'html') + try { + await renderCode(envelope.code, envelope.runtime ?? 'html') + } catch (error) { + if (latestEnvelope !== envelope) return + await renderError( + error instanceof Error ? error.message : 'Unknown render error.', + ) + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1189 - 1196, The inline_code branch currently calls renderCode(envelope.code, envelope.runtime) without error handling; wrap that await renderCode call in a try/catch inside the envelope.mode === 'inline_code' block, catch any thrown error, call await renderError(...) with a clear message that includes the caught error (or its message) so the shell reports the failure, and ensure you still return after handling the error; update references in this block (envelope, renderCode, renderError) only.
707-721:⚠️ Potential issue | 🟠 MajorImport maps are still being ignored, breaking module specifier resolution.
The logic at line 709 returns
'ignore'fortype="importmap", which drops import map scripts entirely. This was flagged in a previous review and remains unaddressed. Import maps must be processed before any module scripts that depend on their bare specifier mappings.Consider returning
'data'forimportmaptype so the script element is preserved in the document, or handle import maps specially to ensure they're inserted before module script execution begins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 707 - 721, The function getShellScriptExecutionMode currently drops import maps by returning 'ignore' for script.type === 'importmap'; change this so importmap scripts are preserved and processed (e.g., return 'data' for normalizedType === 'importmap' or add a special branch that flags import maps for pre-processing) so they are inserted and applied before module scripts run; update the branch in getShellScriptExecutionMode (and related logic that consumes its return values) instead of returning 'ignore', and ensure isClassicJavascriptScriptType is still used for other non-classic types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1217-1220: The message event listener using
globalThis.window.addEventListener currently calls
hostBridge.handleHostMessage(event.data) without origin validation; update the
listener to first check event.origin against an allowlist (or a trustedOrigin
constant) and only call hostBridge.handleHostMessage when the origin matches,
and apply the same origin-check pattern to the other unprotected listener that
dispatches to handleBridgeResponseMessage/handleLifecycleMessage so both entry
points enforce origin validation before processing event.data.
---
Outside diff comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 41-51: The AppSessionEnvelope now allows sessionId and expiresAt
to be undefined but assignments into StorageContext (and into sessionExpiresAt)
assume string|null; to fix, normalize those optional fields at the assignment
sites by coalescing undefined to null (e.g., use sessionId ?? null and expiresAt
?? null when building storageContext and when setting sessionExpiresAt), or
alternatively update the StorageContext type to accept undefined as well; update
the assignment logic in generated-ui-api.ts where storageContext and
sessionExpiresAt are set (look for uses of storageContext, sessionId, expiresAt,
and sessionExpiresAt) and make the null-coalescing change so types align.
---
Duplicate comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1189-1196: The inline_code branch currently calls
renderCode(envelope.code, envelope.runtime) without error handling; wrap that
await renderCode call in a try/catch inside the envelope.mode === 'inline_code'
block, catch any thrown error, call await renderError(...) with a clear message
that includes the caught error (or its message) so the shell reports the
failure, and ensure you still return after handling the error; update references
in this block (envelope, renderCode, renderError) only.
- Around line 707-721: The function getShellScriptExecutionMode currently drops
import maps by returning 'ignore' for script.type === 'importmap'; change this
so importmap scripts are preserved and processed (e.g., return 'data' for
normalizedType === 'importmap' or add a special branch that flags import maps
for pre-processing) so they are inserted and applied before module scripts run;
update the branch in getShellScriptExecutionMode (and related logic that
consumes its return values) instead of returning 'ignore', and ensure
isClassicJavascriptScriptType is still used for other non-classic types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08592f7e-7a2b-4f10-b155-3faa04d98a39
📒 Files selected for processing (2)
e2e/generated-ui-shell.spec.tspackages/worker/client/mcp-apps/kody-ui-utils.ts
Comment on lines
1217
to
1220
|
|
||
| globalThis.window.addEventListener('message', (event: MessageEvent) => { | ||
| hostBridge.handleHostMessage(event.data) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if handleHostMessage has origin validation
rg -A 30 "handleHostMessage" packages/worker/client/mcp-apps/widget-host-bridge.tsRepository: kentcdodds/kody
Length of output: 2015
🏁 Script executed:
rg "handleBridgeResponseMessage|handleLifecycleMessage" packages/worker/client/mcp-apps/widget-host-bridge.ts -A 15 -B 2Repository: kentcdodds/kody
Length of output: 1744
🏁 Script executed:
# Also check if there are any origin validations elsewhere in the codebase
rg "event\.origin|origin.*validate" packages/worker/client/mcp-apps/ -B 2 -A 2Repository: kentcdodds/kody
Length of output: 41
🏁 Script executed:
# Check the full context of where this listener is used - look at initializeShellHostDocument
rg -B 5 -A 10 "globalThis.window.addEventListener.*message" packages/worker/client/mcp-apps/kody-ui-utils.tsRepository: kentcdodds/kody
Length of output: 895
🏁 Script executed:
# Check if there's any parent window validation or cross-origin handling
rg "parent|cross.origin|window.opener" packages/worker/client/mcp-apps/ -B 2 -A 2Repository: kentcdodds/kody
Length of output: 6993
🏁 Script executed:
# Check what appInfo contains and how it's set - might indicate context
rg "appInfo.*=" packages/worker/client/mcp-apps/widget-host-bridge.ts -B 2 -A 5Repository: kentcdodds/kody
Length of output: 404
🏁 Script executed:
# Check if there are any comments about security or origin in the file
rg -i "security|origin|xss|cwe" packages/worker/client/mcp-apps/kody-ui-utils.ts -B 2 -A 2Repository: kentcdodds/kody
Length of output: 423
🏁 Script executed:
# Find where this origin validation code is located
rg -B 10 -A 5 "const origin = source.origin" packages/worker/client/mcp-apps/Repository: kentcdodds/kody
Length of output: 1277
🏁 Script executed:
# Search for all addEventListener('message') calls in the file to see the full context
rg -n "addEventListener.*message" packages/worker/client/mcp-apps/kody-ui-utils.tsRepository: kentcdodds/kody
Length of output: 216
🏁 Script executed:
# Get context around line 1044 to understand the first listener
sed -n '1030,1055p' packages/worker/client/mcp-apps/kody-ui-utils.tsRepository: kentcdodds/kody
Length of output: 762
🏁 Script executed:
# Get context around line 1218 to confirm it's the reviewed code
sed -n '1200,1230p' packages/worker/client/mcp-apps/kody-ui-utils.tsRepository: kentcdodds/kody
Length of output: 732
Add origin validation to the message event listener.
This listener lacks origin validation before processing messages from potentially malicious frames (CWE-346). While handleBridgeResponseMessage and handleLifecycleMessage validate message structure, they don't validate the origin. Note that there's a similar unprotected listener at line 1044 that should receive the same fix.
Add an origin check at the entry point:
Suggested fix
globalThis.window.addEventListener('message', (event: MessageEvent) => {
+ const expectedOrigin = new URL('/', import.meta.url).origin
+ if (event.origin !== expectedOrigin) return
hostBridge.handleHostMessage(event.data)
})🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 1217-1219: Message event listeners should validate the origin to prevent XSS attacks. Always check the event origin before processing the message.
Context: globalThis.window.addEventListener('message', (event: MessageEvent) => {
hostBridge.handleHostMessage(event.data)
})
Note: [CWE-346] Origin Validation Error [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html
- https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
(event-origin-validation)
[warning] 1217-1219: Message event listener without origin validation detected. Always validate the event.origin property in message event handlers to prevent XSS attacks from malicious frames or windows.
Context: globalThis.window.addEventListener('message', (event: MessageEvent) => {
hostBridge.handleHostMessage(event.data)
})
Note: [CWE-346] Origin Validation Error [REFERENCES]
- https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
- https://cwe.mitre.org/data/definitions/346.html
(message-listener-origin-check)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1217 - 1220,
The message event listener using globalThis.window.addEventListener currently
calls hostBridge.handleHostMessage(event.data) without origin validation; update
the listener to first check event.origin against an allowlist (or a
trustedOrigin constant) and only call hostBridge.handleHostMessage when the
origin matches, and apply the same origin-check pattern to the other unprotected
listener that dispatches to handleBridgeResponseMessage/handleLifecycleMessage
so both entry points enforce origin validation before processing event.data.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/worker/client/mcp-apps/kody-ui-utils.ts (4)
1190-1196:⚠️ Potential issue | 🟠 MajorGive inline-code renders the same error fallback as saved apps.
Line 1195 can reject when replaying or loading scripts, but this branch does not catch it. A bad inline render leaves the shell partially updated and skips
renderError(...).Suggested fix
if (envelope.mode === 'inline_code') { if (!envelope.code) { await renderError('The tool result did not include inline code.') return } - await renderCode(envelope.code, envelope.runtime ?? 'html') + try { + await renderCode(envelope.code, envelope.runtime ?? 'html') + } catch (error) { + if (isStaleRender(renderId)) return + await renderError( + error instanceof Error ? error.message : 'Unknown render error.', + ) + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1190 - 1196, The inline_code branch currently calls renderCode(envelope.code, envelope.runtime ?? 'html') without error handling, so failures can leave the UI in a partial state; wrap the renderCode call in a try/catch (in the same block that checks envelope.mode === 'inline_code') and on any exception call await renderError('The tool result did not include inline code.') or a more specific message, then return—ensuring the same fallback behavior as the saved-apps path and that renderError is always invoked if renderCode fails.
1029-1031:⚠️ Potential issue | 🟡 MinorValidate
postMessageorigin at both entry points.Both listeners forward every
messageevent straight intohostBridge.handleHostMessage(...). That means any other frame/window can hit these handlers unless the sender is filtered first.Suggested fix
+const trustedOrigin = new URL('/', import.meta.url).origin + globalThis.window.addEventListener('message', (event: MessageEvent) => { + if (event.origin !== trustedOrigin) return hostBridge.handleHostMessage(event.data) }) // ... globalThis.window.addEventListener('message', (event: MessageEvent) => { + if (event.origin !== trustedOrigin) return hostBridge.handleHostMessage(event.data) })Also applies to: 1222-1224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 1029 - 1031, The global message handlers (globalThis.window.addEventListener('message', ...) and the other listener around lines 1222-1224) currently forward all MessageEvent objects straight into hostBridge.handleHostMessage(event.data); restrict this by validating the event origin and sender before calling hostBridge: check event.origin against a configured allowlist (or expected origin string) and, when messages are expected from a specific iframe/window, verify event.source === expectedWindowReference (or another trusted check) and only then call hostBridge.handleHostMessage(event.data); also ensure you drop or log unexpected origins to avoid silently accepting messages from untrusted frames.
707-710:⚠️ Potential issue | 🟠 MajorPreserve
importmapscripts during replay.Line 709 still classifies
type="importmap"asignore, so the map never reaches the live document. Any replayed module that relies on bare specifiers will then resolve without that map and fail.Suggested fix
function getShellScriptExecutionMode(script: HTMLScriptElement) { const normalizedType = script.type.trim().toLowerCase() - if (normalizedType === 'importmap' || normalizedType === 'speculationrules') { + if (normalizedType === 'importmap') { + return 'data' as const + } + if (normalizedType === 'speculationrules') { return 'ignore' as const } if (normalizedType === 'module') { return 'module' as const }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 707 - 710, The function getShellScriptExecutionMode incorrectly treats scripts with type "importmap" as ignore, preventing import maps from being replayed; update the conditional in getShellScriptExecutionMode so that when normalizedType === 'importmap' it returns 'preserve' (keep 'speculationrules' as 'ignore' if desired), ensuring importmap scripts are forwarded to the live document during replay.
995-997:⚠️ Potential issue | 🟠 Major
executeCodeis still reading the initial bootstrap.Line 996 calls
readGeneratedUiBootstrap(), which readswindow.__kodyGeneratedUiBootstrapinstead of the bootstrap thatupdateGeneratedUiRuntimeBootstrap(...)is refreshing on rerenders. After the first activation, HTTP execute calls can still use a staleappSession.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/kody-ui-utils.ts` around lines 995 - 997, executeCode currently calls readGeneratedUiBootstrap() which returns the initial bootstrap and can be stale after rerenders; instead obtain appSession from the runtime-updated bootstrap that updateGeneratedUiRuntimeBootstrap updates (i.e. call the runtime bootstrap reader provided alongside updateGeneratedUiRuntimeBootstrap — e.g. readGeneratedUiRuntimeBootstrap or getRuntimeUiBootstrap) before calling executeCodeWithHttp so executeCode always uses the latest appSession; add a safe fallback to readGeneratedUiBootstrap if the runtime reader is unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 825-855: Inline module scripts (scriptDescriptor.executionMode ===
'module') are not being awaited because the code treats inline scripts the same
as non-module inline scripts and returns immediately after insertScript; update
the logic to compute a shouldAwaitLoad boolean (true for external scripts with
scriptDescriptor.src and for inline module scripts where executionMode ===
'module') and only return early for inline non-module scripts (e.g., when
!shouldAwaitLoad and !scriptDescriptor.src), otherwise create the loading
promise (using the existing loading event listeners) and await it after
insertScript; adjust handling around script.src, script.textContent,
wrapInlineClassicScriptForIsolation, insertScript, and executionMode so inline
modules and external scripts both wait for evaluation before proceeding.
---
Duplicate comments:
In `@packages/worker/client/mcp-apps/kody-ui-utils.ts`:
- Around line 1190-1196: The inline_code branch currently calls
renderCode(envelope.code, envelope.runtime ?? 'html') without error handling, so
failures can leave the UI in a partial state; wrap the renderCode call in a
try/catch (in the same block that checks envelope.mode === 'inline_code') and on
any exception call await renderError('The tool result did not include inline
code.') or a more specific message, then return—ensuring the same fallback
behavior as the saved-apps path and that renderError is always invoked if
renderCode fails.
- Around line 1029-1031: The global message handlers
(globalThis.window.addEventListener('message', ...) and the other listener
around lines 1222-1224) currently forward all MessageEvent objects straight into
hostBridge.handleHostMessage(event.data); restrict this by validating the event
origin and sender before calling hostBridge: check event.origin against a
configured allowlist (or expected origin string) and, when messages are expected
from a specific iframe/window, verify event.source === expectedWindowReference
(or another trusted check) and only then call
hostBridge.handleHostMessage(event.data); also ensure you drop or log unexpected
origins to avoid silently accepting messages from untrusted frames.
- Around line 707-710: The function getShellScriptExecutionMode incorrectly
treats scripts with type "importmap" as ignore, preventing import maps from
being replayed; update the conditional in getShellScriptExecutionMode so that
when normalizedType === 'importmap' it returns 'preserve' (keep
'speculationrules' as 'ignore' if desired), ensuring importmap scripts are
forwarded to the live document during replay.
- Around line 995-997: executeCode currently calls readGeneratedUiBootstrap()
which returns the initial bootstrap and can be stale after rerenders; instead
obtain appSession from the runtime-updated bootstrap that
updateGeneratedUiRuntimeBootstrap updates (i.e. call the runtime bootstrap
reader provided alongside updateGeneratedUiRuntimeBootstrap — e.g.
readGeneratedUiRuntimeBootstrap or getRuntimeUiBootstrap) before calling
executeCodeWithHttp so executeCode always uses the latest appSession; add a safe
fallback to readGeneratedUiBootstrap if the runtime reader is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e38f56e-5206-4bbf-afa9-f2f059955482
📒 Files selected for processing (1)
packages/worker/client/mcp-apps/kody-ui-utils.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 2ffd963. Configure here.
| element.setAttribute(attribute.name, attribute.value) | ||
| managedAttributes.add(attribute.name) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Managed attributes can erase original shell HTML attributes
Medium Severity
setManagedElementAttributes can permanently remove the shell document's original <html> or <body> attributes (e.g. lang) after a render sequence. When a preserveDocumentChrome: true render (full HTML or error) sets an attribute like lang="en" that already existed on the shell's element, it gets added to the managedAttributes tracking set. A subsequent preserveDocumentChrome: false render (HTML fragment or JS app) then removes all managed attributes — including the shell's original lang. Unlike defaultTitle, which is saved and restored in the GeneratedUiShellRenderState, no default html/body attributes are preserved.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2ffd963. Configure here.
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
document.writerender loop with a stable shell document that parses model HTML inertly and mounts user head/body content into shell-owned slotsSummary by CodeRabbit
Tests
Improvements