Repository navigation
Add hidden saved app search visibility flag - #113
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdded a persistent Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server as MCP Server
participant DB as Database
participant Vector as Vector Index
Client->>MCP_Server: ui_save_app(content, hidden=true)
MCP_Server->>DB: insertUiArtifact(row hidden=1)
DB-->>MCP_Server: created
MCP_Server->>Vector: deleteUiArtifactVector(appId)
Vector-->>MCP_Server: deleted
MCP_Server-->>Client: { appId, hidden: true }
Client->>MCP_Server: ui_save_app(appId, hidden=false)
MCP_Server->>DB: updateUiArtifact(hidden=0)
DB-->>MCP_Server: updated
MCP_Server->>Vector: upsertUiArtifactVector(appId)
Vector-->>MCP_Server: indexed
MCP_Server-->>Client: { appId, hidden: false }
sequenceDiagram
participant Client
participant MCP_Server as MCP Server
participant SearchTool as Search Tool
participant DB as Database
Client->>MCP_Server: search(query, detail=true)
MCP_Server->>SearchTool: loadUiArtifacts({ hidden: false })
SearchTool->>DB: listUiArtifactsByUserId(userId, { hidden: false })
DB-->>SearchTool: artifacts where hidden=0
SearchTool-->>MCP_Server: filtered artifacts
MCP_Server-->>Client: matches (excluding hidden apps)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
cursor
Bot
changed the title
kentcdodds
marked this pull request as ready for review
March 31, 2026 05:51
Contributor
|
🔎 Preview deployed: https://kody-pr-113.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Update without
hiddenresets visible apps to hidden- Updates now preserve the existing hidden flag unless explicitly provided, avoiding unintended visibility changes and vector deletions.
Preview (6a6fb30a14)
diff --git a/docs/agents/mcp-apps-spec-notes.md b/docs/agents/mcp-apps-spec-notes.md
--- a/docs/agents/mcp-apps-spec-notes.md
+++ b/docs/agents/mcp-apps-spec-notes.md
@@ -15,6 +15,9 @@
- The repo exposes a single generic shell via `open_generated_ui`.
- Saved apps are reopened by `app_id`; inline renders are ephemeral.
+- Saved apps are hidden from `search` by default; set
+ `hidden: false` in `ui_save_app` only for reusable apps that
+ should be discoverable.
- If an OAuth provider requires a callback URL, use a persisted hosted saved app
rather than an inline render.
- For secret-bearing requests and host approval policy, also read
diff --git a/packages/worker/migrations/0013-ui-artifact-search-visibility.sql b/packages/worker/migrations/0013-ui-artifact-search-visibility.sql
new file mode 100644
--- /dev/null
+++ b/packages/worker/migrations/0013-ui-artifact-search-visibility.sql
@@ -1,0 +1,2 @@
+ALTER TABLE ui_artifacts
+ADD COLUMN hidden INTEGER NOT NULL DEFAULT 1;
diff --git a/packages/worker/src/app/saved-ui-hosted-html.node.test.ts b/packages/worker/src/app/saved-ui-hosted-html.node.test.ts
--- a/packages/worker/src/app/saved-ui-hosted-html.node.test.ts
+++ b/packages/worker/src/app/saved-ui-hosted-html.node.test.ts
@@ -23,6 +23,7 @@
code: '<main>Hello</main>',
runtime: 'html',
parameters: null,
+ hidden: true,
created_at: '2026-03-27T00:00:00.000Z',
updated_at: '2026-03-27T00:00:00.000Z',
},
@@ -80,6 +81,7 @@
code: 'document.querySelector("[data-generated-ui-root]")?.append("hello")',
runtime: 'javascript',
parameters: null,
+ hidden: true,
created_at: '2026-03-27T00:00:00.000Z',
updated_at: '2026-03-27T00:00:00.000Z',
},
diff --git a/packages/worker/src/mcp/capabilities/apps/ui-get-app.ts b/packages/worker/src/mcp/capabilities/apps/ui-get-app.ts
--- a/packages/worker/src/mcp/capabilities/apps/ui-get-app.ts
+++ b/packages/worker/src/mcp/capabilities/apps/ui-get-app.ts
@@ -14,6 +14,7 @@
title: z.string(),
description: z.string(),
parameters: z.array(uiArtifactParameterSchema).nullable(),
+ hidden: z.boolean(),
code: z
.string()
.describe('Generated UI source code to render inside the generic shell.'),
@@ -54,6 +55,7 @@
title: row.title,
description: row.description,
parameters: parseUiArtifactParameters(row.parameters),
+ hidden: row.hidden,
code: row.code,
runtime: row.runtime,
created_at: row.created_at,
diff --git a/packages/worker/src/mcp/capabilities/apps/ui-list-apps.ts b/packages/worker/src/mcp/capabilities/apps/ui-list-apps.ts
--- a/packages/worker/src/mcp/capabilities/apps/ui-list-apps.ts
+++ b/packages/worker/src/mcp/capabilities/apps/ui-list-apps.ts
@@ -17,6 +17,7 @@
description: z.string(),
runtime: z.string(),
parameters: z.array(uiArtifactParameterSchema).nullable(),
+ hidden: z.boolean(),
created_at: z.string(),
updated_at: z.string(),
}),
@@ -45,6 +46,7 @@
description: row.description,
runtime: row.runtime,
parameters: parseUiArtifactParameters(row.parameters),
+ hidden: row.hidden,
created_at: row.created_at,
updated_at: row.updated_at,
})),
diff --git a/packages/worker/src/mcp/capabilities/apps/ui-load-app-source.ts b/packages/worker/src/mcp/capabilities/apps/ui-load-app-source.ts
--- a/packages/worker/src/mcp/capabilities/apps/ui-load-app-source.ts
+++ b/packages/worker/src/mcp/capabilities/apps/ui-load-app-source.ts
@@ -16,6 +16,7 @@
runtime: z.enum(['html', 'javascript']),
code: z.string(),
parameters: z.array(uiArtifactParameterSchema).nullable(),
+ hidden: z.boolean(),
})
export const uiLoadAppSourceCapability = defineDomainCapability(
@@ -52,6 +53,7 @@
runtime: row.runtime,
code: row.code,
parameters: parseUiArtifactParameters(row.parameters),
+ hidden: row.hidden,
}
},
},
diff --git a/packages/worker/src/mcp/capabilities/apps/ui-save-app.ts b/packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
--- a/packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
+++ b/packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
@@ -6,11 +6,15 @@
import { errorFields, logMcpEvent } from '#mcp/observability.ts'
import {
deleteUiArtifact,
+ getUiArtifactById,
insertUiArtifact,
updateUiArtifact,
} from '#mcp/ui-artifacts-repo.ts'
import { buildUiArtifactEmbedText } from '#mcp/ui-artifacts-embed.ts'
-import { upsertUiArtifactVector } from '#mcp/ui-artifacts-vectorize.ts'
+import {
+ deleteUiArtifactVector,
+ upsertUiArtifactVector,
+} from '#mcp/ui-artifacts-vectorize.ts'
import { requireMcpUser } from '#mcp/capabilities/meta/require-user.ts'
import {
normalizeUiArtifactParameters,
@@ -48,6 +52,12 @@
.describe(
'Optional parameter definitions for reusable saved apps. Resolved values are exposed at runtime on the imported `kodyWidget.params` helper from `@kody/ui-utils`.',
),
+ hidden: z
+ .boolean()
+ .optional()
+ .describe(
+ 'Whether this saved app should stay hidden from search results. Defaults to true so one-off apps stay private unless explicitly made discoverable.',
+ ),
})
const outputSchema = z.object({
@@ -55,6 +65,7 @@
runtime: z.enum(['html', 'javascript']),
hosted_url: z.string().url(),
parameters: z.array(uiArtifactParameterSchema).nullable(),
+ hidden: z.boolean(),
})
export const uiSaveAppCapability = defineDomainCapability(
@@ -77,6 +88,7 @@
const serializedParameters = parameters
? JSON.stringify(parameters)
: null
+ let hidden: boolean
if (isUpdate) {
const updated = await updateUiArtifact(
@@ -89,12 +101,27 @@
code: args.code,
runtime: args.runtime,
parameters: serializedParameters,
+ hidden: args.hidden,
},
)
if (!updated) {
throw new Error('Saved UI artifact not found for this user.')
}
+ if (args.hidden === undefined) {
+ const existing = await getUiArtifactById(
+ ctx.env.APP_DB,
+ user.userId,
+ appId,
+ )
+ if (!existing) {
+ throw new Error('Saved UI artifact not found for this user.')
+ }
+ hidden = existing.hidden
+ } else {
+ hidden = args.hidden
+ }
} else {
+ hidden = args.hidden ?? true
const now = new Date().toISOString()
await insertUiArtifact(ctx.env.APP_DB, {
id: appId,
@@ -104,23 +131,28 @@
code: args.code,
runtime: args.runtime,
parameters: serializedParameters,
+ hidden,
created_at: now,
updated_at: now,
})
}
try {
- await upsertUiArtifactVector(ctx.env, {
- appId,
- userId: user.userId,
- embedText: buildUiArtifactEmbedText({
- title: args.title,
- description: args.description,
- code: args.code,
- runtime: args.runtime,
- parameters,
- }),
- })
+ if (!hidden) {
+ await upsertUiArtifactVector(ctx.env, {
+ appId,
+ userId: user.userId,
+ embedText: buildUiArtifactEmbedText({
+ title: args.title,
+ description: args.description,
+ code: args.code,
+ runtime: args.runtime,
+ parameters,
+ }),
+ })
+ } else if (isUpdate) {
+ await deleteUiArtifactVector(ctx.env, appId)
+ }
} catch (cause) {
if (!isUpdate) {
await deleteUiArtifact(ctx.env.APP_DB, user.userId, appId)
@@ -156,6 +188,7 @@
runtime: args.runtime,
hosted_url: buildSavedUiUrl(ctx.callerContext.baseUrl, appId),
parameters,
+ hidden,
}
},
},
diff --git a/packages/worker/src/mcp/capabilities/unified-search.workers.test.ts b/packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
--- a/packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
+++ b/packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
@@ -43,6 +43,7 @@
required: true,
},
]),
+ hidden: false,
created_at: '2026-03-20T00:00:00.000Z',
updated_at: '2026-03-20T00:00:00.000Z',
}
diff --git a/packages/worker/src/mcp/index.ts b/packages/worker/src/mcp/index.ts
--- a/packages/worker/src/mcp/index.ts
+++ b/packages/worker/src/mcp/index.ts
@@ -42,7 +42,7 @@
- Never ask the user to paste secrets, tokens, API keys, passwords, OAuth codes, or client secrets into chat. Use saved secrets when available, or use 'open_generated_ui' to collect and save sensitive values instead.
- Use 'meta_save_skill' only for workflows that are reasonably repeatable—patterns you expect to run again with similar structure or inputs. Do not save one-off tasks, unique ad-hoc work, or highly bespoke requests as skills; run those with 'execute' instead. Use the optional 'collection' field to group related saved skills, and use 'meta_update_skill' to replace an existing skill's code in place.
- When a saved skill declares parameters, pass values via meta_run_skill params; the codemode can read them from the params variable.
-- Use 'ui_save_app' to persist reusable UI source for later reopening via 'app_id'. Saved apps are user-scoped UI artifacts, not codemode skills.
+ - Use 'ui_save_app' to persist reusable UI source for later reopening via 'app_id'. Saved apps are user-scoped UI artifacts, not codemode skills. They are hidden from search by default unless you explicitly set \`hidden: false\` for reusable apps.
- Use \`codemode.secret_list(args)\` during execute-time code to list secret metadata only; it does not return plaintext values.
- Use \`codemode.secret_set(args)\` only to persist secret values that are already available inside trusted execution, such as refreshed OAuth tokens. It returns metadata only and never returns plaintext values.
diff --git a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
--- a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
+++ b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
@@ -232,7 +232,110 @@
expect(executeResult?.hosted_url).toBe(
`${server.origin}/ui/${executeResult?.app_id}`,
)
+ expect(executeResult?.hidden).toBe(true)
+ const hiddenSearchResult = await mcpClient.client.callTool({
+ name: 'search',
+ arguments: {
+ query: 'Execute generated app',
+ detail: true,
+ },
+ })
+ const hiddenSearchStructured = (hiddenSearchResult as CallToolResult)
+ .structuredContent as
+ | {
+ result?: {
+ matches?: Array<{
+ type?: string
+ appId?: string
+ }>
+ }
+ }
+ | undefined
+ expect(
+ hiddenSearchStructured?.result?.matches?.some(
+ (match) =>
+ match.type === 'app' && match.appId === executeResult?.app_id,
+ ),
+ ).toBe(false)
+
+ const savedAppMetadata = await mcpClient.client.callTool({
+ name: 'execute',
+ arguments: {
+ code: `async () => {
+ return await codemode.ui_get_app({
+ app_id: ${JSON.stringify(executeResult?.app_id)},
+ })
+ }`,
+ },
+ })
+ const savedAppMetadataStructured = (savedAppMetadata as CallToolResult)
+ .structuredContent as
+ | {
+ result?: {
+ hidden?: boolean
+ }
+ }
+ | undefined
+ expect(savedAppMetadataStructured?.result?.hidden).toBe(true)
+
+ const searchableUpdateResult = await mcpClient.client.callTool({
+ name: 'execute',
+ arguments: {
+ code: `async () => {
+ return await codemode.ui_save_app({
+ app_id: ${JSON.stringify(executeResult?.app_id)},
+ title: 'Execute generated app',
+ description: 'Saved through execute.',
+ code: '<main><h1>Execute App</h1></main>',
+ hidden: false,
+ })
+ }`,
+ },
+ })
+ const searchableUpdateStructured = (
+ searchableUpdateResult as CallToolResult
+ ).structuredContent as
+ | {
+ result?: {
+ hidden?: boolean
+ }
+ }
+ | undefined
+ expect(searchableUpdateStructured?.result?.hidden).toBe(false)
+
+ const visibleSearchResult = await mcpClient.client.callTool({
+ name: 'search',
+ arguments: {
+ query: 'Execute generated app',
+ detail: true,
+ limit: 20,
+ maxResponseSize: 20_000,
+ },
+ })
+ const visibleSearchStructured = (visibleSearchResult as CallToolResult)
+ .structuredContent as
+ | {
+ result?: {
+ matches?: Array<{
+ type?: string
+ appId?: string
+ }>
+ }
+ }
+ | undefined
+ expect(
+ visibleSearchStructured?.result?.matches?.find(
+ (match) =>
+ match.type === 'app' && match.appId === executeResult?.app_id,
+ ),
+ ).toEqual(
+ expect.objectContaining({
+ type: 'app',
+ appId: executeResult?.app_id,
+ }),
+ )
+
const contextResult = await mcpClient.client.callTool({
name: 'execute',
arguments: {
diff --git a/packages/worker/src/mcp/observability.workers.test.ts b/packages/worker/src/mcp/observability.workers.test.ts
--- a/packages/worker/src/mcp/observability.workers.test.ts
+++ b/packages/worker/src/mcp/observability.workers.test.ts
@@ -146,7 +146,6 @@
{
title: 'Observed app',
description: 'Observation test app.',
- keywords: ['observability'],
code: 'document.querySelector("#app")!.innerHTML = "<h1>Observed app</h1>"',
},
{
@@ -175,6 +174,7 @@
},
)
expect(typeof (result as { app_id: string }).app_id).toBe('string')
+ expect((result as { hidden: boolean }).hidden).toBe(true)
} finally {
console.info = originalInfo
}
@@ -201,6 +201,7 @@
title: 'Observed app',
description: 'Observation test app.',
code: '<main><h1>Observed app</h1></main>',
+ hidden: false,
},
{
env: {
diff --git a/packages/worker/src/mcp/tools/search.node.test.ts b/packages/worker/src/mcp/tools/search.node.test.ts
--- a/packages/worker/src/mcp/tools/search.node.test.ts
+++ b/packages/worker/src/mcp/tools/search.node.test.ts
@@ -19,6 +19,7 @@
code: '<div />',
runtime: 'html',
parameters: null,
+ hidden: false,
created_at: '2026-03-24T00:00:00.000Z',
updated_at: '2026-03-24T00:00:00.000Z',
},
diff --git a/packages/worker/src/mcp/tools/search.ts b/packages/worker/src/mcp/tools/search.ts
--- a/packages/worker/src/mcp/tools/search.ts
+++ b/packages/worker/src/mcp/tools/search.ts
@@ -269,7 +269,9 @@
loadSkills: () =>
listMcpSkillsByUserId(agent.getEnv().APP_DB, userId!),
loadUiArtifacts: () =>
- listUiArtifactsByUserId(agent.getEnv().APP_DB, userId!),
+ listUiArtifactsByUserId(agent.getEnv().APP_DB, userId!, {
+ hidden: false,
+ }),
loadUserSecrets: () =>
listUserSecretsForSearch({
env: agent.getEnv(),
diff --git a/packages/worker/src/mcp/ui-artifacts-repo.ts b/packages/worker/src/mcp/ui-artifacts-repo.ts
--- a/packages/worker/src/mcp/ui-artifacts-repo.ts
+++ b/packages/worker/src/mcp/ui-artifacts-repo.ts
@@ -16,8 +16,8 @@
.prepare(
`INSERT INTO ui_artifacts (
id, user_id, title, description, source_code, source_type,
- parameters, created_at, updated_at
- ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+ parameters, hidden, created_at, updated_at
+ ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
)
.bind(
row.id,
@@ -27,6 +27,7 @@
row.code,
row.runtime,
row.parameters ?? null,
+ row.hidden ? 1 : 0,
row.created_at ?? now,
row.updated_at ?? now,
)
@@ -41,7 +42,7 @@
const result = await db
.prepare(
`SELECT id, user_id, title, description, source_code, source_type,
- parameters, created_at, updated_at
+ parameters, hidden, created_at, updated_at
FROM ui_artifacts WHERE id = ? AND user_id = ?`,
)
.bind(artifactId, userId)
@@ -62,7 +63,7 @@
const result = await db
.prepare(
`SELECT id, user_id, title, description, source_code, source_type,
- parameters, created_at, updated_at
+ parameters, hidden, created_at, updated_at
FROM ui_artifacts
WHERE id = ? AND user_id IN (${placeholders})
LIMIT 1`,
@@ -92,7 +93,12 @@
updates: Partial<
Pick<
UiArtifactRow,
- 'title' | 'description' | 'code' | 'runtime' | 'parameters'
+ | 'title'
+ | 'description'
+ | 'code'
+ | 'runtime'
+ | 'parameters'
+ | 'hidden'
>
>,
): Promise<boolean> {
@@ -118,6 +124,9 @@
if (updates.parameters !== undefined) {
addAssignment('parameters', updates.parameters ?? null)
}
+ if (updates.hidden !== undefined) {
+ addAssignment('hidden', updates.hidden ? 1 : 0)
+ }
addAssignment('updated_at', new Date().toISOString())
@@ -133,14 +142,21 @@
export async function listUiArtifactsByUserId(
db: D1Database,
userId: string,
+ options?: { hidden?: boolean },
): Promise<Array<UiArtifactRow>> {
+ const hidden = options?.hidden
const { results } = await db
.prepare(
`SELECT id, user_id, title, description, source_code, source_type,
- parameters, created_at, updated_at
- FROM ui_artifacts WHERE user_id = ?`,
+ parameters, hidden, created_at, updated_at
+ FROM ui_artifacts
+ WHERE user_id = ?
+ ${hidden === undefined ? '' : 'AND hidden = ?'}`,
)
- .bind(userId)
+ .bind(
+ userId,
+ ...(hidden === undefined ? [] : [hidden ? 1 : 0]),
+ )
.all<Record<string, unknown>>()
return (results ?? []).map(mapRow)
}
@@ -154,6 +170,10 @@
code: String(row['source_code']),
runtime: String(row['source_type']) as UiArtifactRow['runtime'],
parameters: row['parameters'] == null ? null : String(row['parameters']),
+ hidden:
+ row['hidden'] === 1 ||
+ row['hidden'] === '1' ||
+ row['hidden'] === true,
created_at: String(row['created_at']),
updated_at: String(row['updated_at']),
}
diff --git a/packages/worker/src/mcp/ui-artifacts-types.ts b/packages/worker/src/mcp/ui-artifacts-types.ts
--- a/packages/worker/src/mcp/ui-artifacts-types.ts
+++ b/packages/worker/src/mcp/ui-artifacts-types.ts
@@ -8,6 +8,7 @@
code: string
runtime: UiArtifactRuntime
parameters: string | null
+ hidden: boolean
created_at: string
updated_at: string
}You can send follow-ups to this agent here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/mcp/tools/search.ts (1)
267-274:⚠️ Potential issue | 🟡 MinorExpose the discoverability rule in the
searchtool contract.This filter changes the behavior from “saved apps are searchable” to “only apps saved with
hidden: falseare searchable,” but the tool description still advertises saved-app search generically. Without that hint, missing hits read like a search failure rather than intended visibility.✏️ Suggested wording
-Search Kody **builtin capabilities**, your saved **skills** (meta domain), your saved **apps** (apps domain), and your reusable **user secret references** by natural language before calling `execute` or opening a UI. +Search Kody **builtin capabilities**, your saved **skills** (meta domain), your saved discoverable **apps** (apps domain; only apps saved with `hidden: false`), and your reusable **user secret references** by natural language before calling `execute` or opening a UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/tools/search.ts` around lines 267 - 274, The search tool's contract description currently implies all saved apps are searchable but the implementation filters UI artifacts with hidden: false in loadOptionalSearchRows; update the "search" tool contract (the user-facing description for the search tool) to explicitly state that only saved apps marked discoverable/hidden: false are included in search results and mention the discoverability rule so callers understand why some saved apps won't appear; locate the contract/description for the search tool (referenced by the "search" tool name and the loadOptionalSearchRows call) and amend its text to reference "only apps saved with hidden: false (discoverable) are returned" and, if applicable, add a short note on how to change visibility via the UI/API.
🧹 Nitpick comments (1)
packages/worker/src/mcp/observability.workers.test.ts (1)
204-204: Minor formatting nit: inconsistent indentation.The
hidden: falseproperty appears to have extra leading whitespace compared to surrounding properties.♻️ Suggested fix
{ app_id: 'app-1', title: 'Observed app', description: 'Observation test app.', code: '<main><h1>Observed app</h1></main>', - hidden: false, + hidden: false, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/observability.workers.test.ts` at line 204, Adjust the indentation of the object property "hidden: false" so it lines up with the surrounding properties in the same object literal (remove the extra leading whitespace); locate the "hidden: false" occurrence in the observability workers test and make its indentation consistent with adjacent properties to match the file's formatting style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 267-274: The search tool's contract description currently implies
all saved apps are searchable but the implementation filters UI artifacts with
hidden: false in loadOptionalSearchRows; update the "search" tool contract (the
user-facing description for the search tool) to explicitly state that only saved
apps marked discoverable/hidden: false are included in search results and
mention the discoverability rule so callers understand why some saved apps won't
appear; locate the contract/description for the search tool (referenced by the
"search" tool name and the loadOptionalSearchRows call) and amend its text to
reference "only apps saved with hidden: false (discoverable) are returned" and,
if applicable, add a short note on how to change visibility via the UI/API.
---
Nitpick comments:
In `@packages/worker/src/mcp/observability.workers.test.ts`:
- Line 204: Adjust the indentation of the object property "hidden: false" so it
lines up with the surrounding properties in the same object literal (remove the
extra leading whitespace); locate the "hidden: false" occurrence in the
observability workers test and make its indentation consistent with adjacent
properties to match the file's formatting style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19e6b523-6931-4e50-af1b-b7622f83f418
📒 Files selected for processing (15)
docs/agents/mcp-apps-spec-notes.mdpackages/worker/migrations/0013-ui-artifact-search-visibility.sqlpackages/worker/src/app/saved-ui-hosted-html.node.test.tspackages/worker/src/mcp/capabilities/apps/ui-get-app.tspackages/worker/src/mcp/capabilities/apps/ui-list-apps.tspackages/worker/src/mcp/capabilities/apps/ui-load-app-source.tspackages/worker/src/mcp/capabilities/apps/ui-save-app.tspackages/worker/src/mcp/capabilities/unified-search.workers.test.tspackages/worker/src/mcp/index.tspackages/worker/src/mcp/mcp-server.mcp-e2e.test.tspackages/worker/src/mcp/observability.workers.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/mcp/ui-artifacts-repo.tspackages/worker/src/mcp/ui-artifacts-types.ts
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/src/mcp/capabilities/apps/ui-save-app.ts (1)
140-192:⚠️ Potential issue | 🟠 MajorDon't silently succeed when a visible app fails to reindex.
When
hiddenresolves tofalseon Line 141, a failingupsertUiArtifactVectorfalls into the Line 156 catch, gets logged, and the handler still returns success on Line 186. For apps being unhidden, that leaves D1 saying “discoverable” while search can still miss the app entirely until some later repair.At minimum, bubble the error whenever the final state is searchable; if you want compensation instead, capture the pre-update flag before the write and roll it back here.
🛠️ Minimal fail-fast fix
} catch (cause) { if (!isUpdate) { await deleteUiArtifact(ctx.env.APP_DB, user.userId, appId) throw cause } const { errorName, errorMessage } = errorFields(cause) logMcpEvent({ category: 'mcp', tool: 'capability', capabilityName: 'ui_save_app', domain: capabilityDomainNames.apps, outcome: 'failure', durationMs: 0, baseUrl: ctx.callerContext.baseUrl, hasUser: true, failurePhase: 'handler', message: 'Failed to refresh saved app vector index after in-place update.', errorName, errorMessage, cause, context: { userId: user.userId, appId, isUpdate, }, }) + + if (!hidden) { + throw cause + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/capabilities/apps/ui-save-app.ts` around lines 140 - 192, The catch currently only rethrows when !isUpdate, which lets failures from upsertUiArtifactVector (called when hidden is false) on updates silently succeed; change the error handling so that if the intended final visibility is searchable (hidden === false) you rethrow the caught error after logging instead of returning success—i.e., in the catch block check the resolved hidden flag (or capture its pre-write value) and if hidden is false, after calling logMcpEvent (and any cleanup like deleteUiArtifactVector/deleteUiArtifact if desired) throw the original cause; this ensures failures in upsertUiArtifactVector are bubbled to the caller rather than returning a successful response from the handler.
🤖 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/src/mcp/capabilities/apps/ui-save-app.ts`:
- Around line 55-60: Update the description for the "hidden" field in the schema
in ui-save-app.ts to clarify that the default-to-true behavior applies only when
creating a new app; when updating an existing app, omitting "hidden" preserves
the stored value. Locate the "hidden" field (the
z.boolean().optional().describe(...) entry) and replace the misleading text with
a concise note that defaults only on create and that patch/update requests that
omit the field do not change the stored hidden flag.
---
Outside diff comments:
In `@packages/worker/src/mcp/capabilities/apps/ui-save-app.ts`:
- Around line 140-192: The catch currently only rethrows when !isUpdate, which
lets failures from upsertUiArtifactVector (called when hidden is false) on
updates silently succeed; change the error handling so that if the intended
final visibility is searchable (hidden === false) you rethrow the caught error
after logging instead of returning success—i.e., in the catch block check the
resolved hidden flag (or capture its pre-write value) and if hidden is false,
after calling logMcpEvent (and any cleanup like
deleteUiArtifactVector/deleteUiArtifact if desired) throw the original cause;
this ensures failures in upsertUiArtifactVector are bubbled to the caller rather
than returning a successful response from the handler.
🪄 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: fa7e759e-76ef-47b1-9e85-1230aa23eb62
📒 Files selected for processing (1)
packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
Comment on lines
+55
to
+60
| hidden: z | ||
| .boolean() | ||
| .optional() | ||
| .describe( | ||
| 'Whether this saved app should stay hidden from search results. Defaults to true so one-off apps stay private unless explicitly made discoverable.', | ||
| ), |
There was a problem hiding this comment.
Clarify that hidden only defaults on create.
This description says the field “defaults to true,” but omitted hidden values on updates actually preserve the stored flag. That mismatch can mislead callers/agents when patching an existing app.
✏️ Suggested wording
hidden: z
.boolean()
.optional()
.describe(
- 'Whether this saved app should stay hidden from search results. Defaults to true so one-off apps stay private unless explicitly made discoverable.',
+ 'Whether this saved app should stay hidden from search results. New apps default to true so one-off apps stay private; updates preserve the current value unless hidden is explicitly provided.',
),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/capabilities/apps/ui-save-app.ts` around lines 55 -
60, Update the description for the "hidden" field in the schema in
ui-save-app.ts to clarify that the default-to-true behavior applies only when
creating a new app; when updating an existing app, omitting "hidden" preserves
the stored value. Locate the "hidden" field (the
z.boolean().optional().describe(...) entry) and replace the misleading text with
a concise note that defaults only on create and that patch/update requests that
omit the field do not change the stored hidden flag.
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
hiddenhidden: true, and make apps discoverable only when saved or updated withhidden: falseapp_idwhile excluding hidden apps fromsearchTesting
npm exec vitest run --project node-unit packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/app/saved-ui-hosted-html.node.test.tsnpm exec vitest run --project workers-unit packages/worker/src/mcp/capabilities/unified-search.workers.test.ts packages/worker/src/mcp/observability.workers.test.tsnpm exec vitest run --project mcp-e2e packages/worker/src/mcp/mcp-server.mcp-e2e.test.tsSummary by CodeRabbit
New Features
Documentation
Chores
Tests