Skip to content

Refine MCP search output and embeddings - #115

Merged
kentcdodds merged 7 commits into
mainfrom
cursor/kody-mcp-skill-for-github-docs-bd7d
Mar 31, 2026
Merged

kentcdodds merged 7 commits into
mainfrom
cursor/kody-mcp-skill-for-github-docs-bd7d

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Mar 31, 2026 •

Copy link
Copy Markdown
Member

Summary

  • simplify skill and saved-app embedding sources, including a new app reindex maintenance path
  • replace search detail payloads with compact markdown plus slim structured results and exact entity lookups
  • add hosted URLs for saved app search results and update MCP instructions, maintenance docs, and focused tests for the new contract
  • fix the latest pulled branch state so npm run typecheck passes again

Testing

  • npm run typecheck
  • npm run build
  • npm run test -- packages/worker/src/mcp/skills/skill-embed-and-flags.node.test.ts packages/worker/src/mcp/ui-artifacts-embed.node.test.ts packages/worker/src/mcp/capabilities/unified-search.workers.test.ts packages/worker/src/mcp/tools/search.node.test.ts
  • npm run test:mcp -- packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features

    • View a focused entity by reference in search (e.g., "id:app" or "id:skill").
    • New maintenance endpoint to reindex saved apps and rebuild app embeddings.
  • Changes

    • Search now returns slimmer, compact results by default (previous "detail" mode removed).
    • App results include a hosted URL in structured output.
    • Saved app source code is excluded from embedding text; parameter hints preserved.

cursoragent and others added 2 commits March 31, 2026 06:46
Co-authored-by: me <me@kentcdodds.com>
Co-authored-by: me <me@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Mar 31, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds POST /__maintenance/reindex-apps and its handler to re-embed visible saved UI artifacts (excluding source code) and upsert ui_artifact_<uuid> vectors; removes detailed search hits in favor of compact summaries, adds search-format utilities and entity-detail lookup via an entity parameter, and updates related tests and docs.

Changes

Cohort / File(s) Summary
Documentation
docs/agents/mcp-skills.md, docs/setup-manifest.md
Documented new POST /__maintenance/reindex-apps and expanded CAPABILITY_REINDEX_SECRET scope to include discoverable saved apps.
Routing & HTTP handler
packages/worker/src/index.ts, packages/worker/src/ui-artifact-maintenance.ts
Added route for /__maintenance/reindex-apps and handleUiArtifactReindexRequest which validates Bearer token and calls reindex logic, returning JSON status and errors.
Reindex implementation
packages/worker/src/mcp/ui-artifact-reindex.ts
New reindexUiArtifactVectors(env) that loads visible UI artifacts per user, collects app secrets map, batches embedding requests to AI model, validates response length, and upserts vectors with ids ui_artifact_<uuid>.
Embedding input changes
packages/worker/src/mcp/ui-artifacts-embed.ts, packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
Removed code from UI artifact embed input and stopped including stored app source in embedding text; DB still persists code on save.
Search formatting & entity detail
packages/worker/src/mcp/tools/search-format.ts, packages/worker/src/mcp/tools/search.ts
Added comprehensive search-format utilities/types and formatters; refactored search tool to accept optional entity ({id}:{type}) for focused entity detail, removed detail flag, and introduced markdown + structured output and trimming helpers.
Unified/search hit shape changes
packages/worker/src/mcp/capabilities/unified-search.ts, packages/worker/src/mcp/ui-artifacts-search.ts
Removed detail variants for skill/secret/ui-artifact hits (summary-only hits), added baseUrl param to produce hostedUrl for apps, simplified row-to-hit logic and removed conditional detail branches.
Skill embed simplification
packages/worker/src/mcp/skills/skill-embed-and-flags.ts, packages/worker/src/mcp/skills/skill-embed-and-flags.node.test.ts
Removed denormalized capability-text block generation; replaced with single-line inferred capabilities: ... summary and updated tests to match.
Tests & examples
packages/worker/src/mcp/*.test.ts, packages/worker/src/mcp/ui-artifacts-embed.node.test.ts
Updated tests to pass baseUrl, remove detail: true, adjust expectations to id/hostedUrl and new text outputs, and added UI artifact embed test ensuring parameters appear and source code is excluded.
MCP metadata docs
packages/worker/src/mcp/index.ts
Updated server metadata/instructions to remove detail guidance and add entity lookup examples using {id}:{type}.
Capability summary change
packages/worker/src/mcp/capabilities/capability-search.ts
Added description to exported CapabilitySummaryRow and populated it from spec descriptions.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Worker
  participant DB
  participant AI as AI_Embed_Model
  participant Vector as VectorIndex

  Client->>Worker: POST /__maintenance/reindex-apps + Bearer token
  Worker->>Worker: validate CAPABILITY_REINDEX_SECRET
  alt not configured
    Worker-->>Client: 503 Saved app reindex is not configured
  else authorized
    Worker->>DB: query visible ui_artifacts (grouped by user)
    DB-->>Worker: ui artifacts per user
    Worker->>DB: list app secrets for appIds per user
    DB-->>Worker: app secrets map
    loop batches of artifacts
      Worker->>AI_Embed_Model: run embedding for batch texts
      AI_Embed_Model-->>Worker: embeddings array
      Worker->>Vector: upsert vectors (ui_artifact_<uuid>, metadata)
      Vector-->>Worker: upsert ack
    end
    Worker-->>Client: 200 { ok: true, upserted: N }
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I nibbled through titles, sniffed each saved app bright,

Left the secret source alone — tucked code out of sight.
I hopped and batched the embeddings, sent them off to learn,
Then upserted tidy vectors so search returns will turn.
A little hop, a gentle thump — reindex done just right!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Refine MCP search output and embeddings' directly and accurately summarizes the main changes: simplifying embeddings, refining search output format, and replacing detail payloads with compact markdown and entity lookups.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/kody-mcp-skill-for-github-docs-bd7d

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.

❤️ Share

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

Co-authored-by: me <me@kentcdodds.com>
@kentcdodds
kentcdodds marked this pull request as ready for review March 31, 2026 13:26
@github-actions

github-actions Bot commented Mar 31, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-115.kentcdodds.workers.dev

Worker: kody-pr-115
D1: kody-pr-115-db
KV: kody-pr-115-oauth-kv

Mocks:

Comment thread packages/worker/src/mcp/capabilities/unified-search.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/worker/src/mcp/capabilities/unified-search.workers.test.ts (1)

57-63: ⚠️ Potential issue | 🔴 Critical

Remove the stale detail argument from this test call.

Line 62 still passes detail: false, but searchUnified no longer accepts that property. This object literal should fail npm run typecheck.

Suggested fix
 	const result = await searchUnified({
 		env,
 		baseUrl: 'http://localhost',
 		query: 'open pull requests for cursor agents',
 		limit: 5,
-		detail: false,
 		specs,
 		userId: 'user-123',
 		skillRows: [skillRow],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/unified-search.workers.test.ts` around
lines 57 - 63, The test calls searchUnified with a stale property; remove the
no-longer-supported detail field from the object passed to searchUnified (the
call where result is assigned) so the argument object matches the current
searchUnified signature; update the invocation that includes env, baseUrl,
query, limit, and specs to omit detail entirely.
🧹 Nitpick comments (1)
packages/worker/src/mcp/ui-artifact-reindex.ts (1)

13-25: Avoid the per-user D1 fan-out in the full reindex job.

Lines 13-25 fetch distinct users and then reload artifacts per user, and Lines 44-66 do another secrets lookup per user. On a large saved-app corpus that is 2 * users + 1 DB round-trips plus full in-memory materialization before embedding work, which is a risky shape for a deploy-time Worker task. A paginated scan of visible apps with batched secret lookups by app id would scale much better.

Also applies to: 44-66

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/setup-manifest.md`:
- Line 93: Replace the single word "builtin" with the hyphenated form "built-in"
in the sentence that mentions the endpoint `POST /__maintenance/reindex-apps` so
it reads "built-in capabilities"; update the phrase near that endpoint reference
(`POST /__maintenance/reindex-apps`) accordingly to use "built-in".

In `@packages/worker/src/mcp/skills/skill-embed-and-flags.ts`:
- Around line 61-63: Filter inferred capability names using an own-property
check and remove duplicates: replace the current filter on
input.inferredCapabilities that uses input.specs[name] != null with a check
using Object.prototype.hasOwnProperty.call(input.specs, name) to ensure only own
keys of input.specs are accepted, deduplicate the list (e.g., via a Set) and
then sort it; refer to inferredCapabilityNames, input.inferredCapabilities,
input.specs and the existing mergeInferredCapabilityNames pattern for
consistency.

In `@packages/worker/src/mcp/tools/search-format.ts`:
- Around line 42-49: The new slim app shape dropped the app-bound secret
metadata: update the app result shape (the object with type: 'app' that
currently lists id, title, description, usage, hostedUrl) to include the
availableSecrets field and ensure every place that maps search hits into that
shape (including searchUnified result mapping and the markdown renderers that
build app outputs) copies availableSecrets from the original hit into the new
shape; apply the same addition to the other mapping sites you noted (lines
~90-101, ~211-230, ~239-275, ~412-451) so ranked results and entity: "...:app"
details continue to expose the app's availableSecrets.

In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 330-333: The code is using the best-effort list
input.searchRows.userSecretRows to resolve an exact secret (ref.id), which hides
transient loader failures from loadOptionalSearchRows; instead replace this
lookup with a direct fetch (call the secret loader / fetch-by-name function
rather than searching userSecretRows) or detect and rethrow the loader error
produced by loadOptionalSearchRows so we don’t convert backend failures into
"Secret not found for this user."; update the branch around the lookups that
reference input.searchRows.userSecretRows and the thrown message "Secret not
found for this user." to use a direct secret-by-name fetch or propagate the
original loader error.

In `@packages/worker/src/mcp/ui-artifact-reindex.ts`:
- Around line 39-42: The current reindex only calls listAllVisibleUiArtifacts
and upserts visible items (see listAllVisibleUiArtifacts and the upsert loop
around lines 94-104), so stale vector rows named like ui_artifact_<uuid> for
hidden/deleted apps are never removed; modify the routine to perform a delete
phase: either (A) fetch existing vector IDs from the vector store (matching
prefix ui_artifact_), compute the set difference against the visible rows
returned by listAllVisibleUiArtifacts, and delete those stale ids before/after
upserting, or (B) perform a full rebuild by removing all ui_artifact_* vectors
first and then upserting the current visible rows—implement the chosen strategy
and ensure the code references listAllVisibleUiArtifacts, the upsert logic, and
the vector-store delete operation so stale vectors are removed.

In `@packages/worker/src/mcp/ui-artifacts-embed.node.test.ts`:
- Around line 4-26: The test "buildUiArtifactEmbedText excludes source code and
keeps parameter hints" is passing a stale code property that no longer exists on
buildUiArtifactEmbedText's input type; remove the code: '<main>…</main>' field
from the test payload so the object matches the current input shape for
buildUiArtifactEmbedText, leaving title, description, runtime and parameters
as-is (keep assertions unchanged), and if you still want to assert that source
code is never embedded move that coverage to the ui_save_app or reindex call
sites that still accept a code field.

---

Outside diff comments:
In `@packages/worker/src/mcp/capabilities/unified-search.workers.test.ts`:
- Around line 57-63: The test calls searchUnified with a stale property; remove
the no-longer-supported detail field from the object passed to searchUnified
(the call where result is assigned) so the argument object matches the current
searchUnified signature; update the invocation that includes env, baseUrl,
query, limit, and specs to omit detail entirely.
🪄 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: 06124774-80cb-4599-8106-f13e548569bb

📥 Commits

Reviewing files that changed from the base of the PR and between edf5135 and 4a93c1d.

📒 Files selected for processing (17)
  • docs/agents/mcp-skills.md
  • docs/setup-manifest.md
  • packages/worker/src/index.ts
  • packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
  • packages/worker/src/mcp/capabilities/unified-search.ts
  • packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
  • packages/worker/src/mcp/index.ts
  • packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
  • packages/worker/src/mcp/skills/skill-embed-and-flags.node.test.ts
  • packages/worker/src/mcp/skills/skill-embed-and-flags.ts
  • packages/worker/src/mcp/tools/search-format.ts
  • packages/worker/src/mcp/tools/search.ts
  • packages/worker/src/mcp/ui-artifact-reindex.ts
  • packages/worker/src/mcp/ui-artifacts-embed.node.test.ts
  • packages/worker/src/mcp/ui-artifacts-embed.ts
  • packages/worker/src/mcp/ui-artifacts-search.ts
  • packages/worker/src/ui-artifact-maintenance.ts
💤 Files with no reviewable changes (2)
  • packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
  • packages/worker/src/mcp/ui-artifacts-embed.ts

Comment thread docs/setup-manifest.md
and all user skills into Vectorize)
`POST /__maintenance/reindex-capabilities`,
`POST /__maintenance/reindex-skills`, and
`POST /__maintenance/reindex-apps` to embed and upsert builtin capabilities,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use built-in here.

Line 93 reads more naturally as built-in capabilities.

🧰 Tools
🪛 LanguageTool

[grammar] ~93-~93: Ensure spelling is correct
Context: ...nance/reindex-apps` to embed and upsert builtin capabilities, all user skills, and di...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/setup-manifest.md` at line 93, Replace the single word "builtin" with
the hyphenated form "built-in" in the sentence that mentions the endpoint `POST
/__maintenance/reindex-apps` so it reads "built-in capabilities"; update the
phrase near that endpoint reference (`POST /__maintenance/reindex-apps`)
accordingly to use "built-in".

Comment on lines +61 to +63
const inferredCapabilityNames = input.inferredCapabilities
.filter((name) => input.specs[name] != null)
.sort()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify current membership checks in this module.
rg -n -C2 "input\.specs\[name\]|hasOwnProperty|Object\.hasOwn" packages/worker/src/mcp/skills/skill-embed-and-flags.ts

Repository: kentcdodds/kody

Length of output: 221


🏁 Script executed:

# Read the full context of the function containing this code
cat -n packages/worker/src/mcp/skills/skill-embed-and-flags.ts | head -100

Repository: kentcdodds/kody

Length of output: 3690


🏁 Script executed:

# Check if input.specs could have prototype-inherited keys
# Search for where input is defined and its type
rg -n "input\s*[:=]|inferredCapabilities|specs" packages/worker/src/mcp/skills/skill-embed-and-flags.ts | head -30

Repository: kentcdodds/kody

Length of output: 678


🏁 Script executed:

# Look for similar property access patterns in the codebase
rg -n "\[name\]\s*!=\s*null|\[.*\]\s*!=\s*undefined" packages/worker/src/mcp/skills/ | head -20

Repository: kentcdodds/kody

Length of output: 164


🏁 Script executed:

# Check how specs and inferredCapabilities are created/sourced in the codebase
rg -n "inferredCapabilities\s*[:=]|specs\s*[:=]" packages/worker/src/mcp/skills/ --max-count=20

Repository: kentcdodds/kody

Length of output: 1242


🏁 Script executed:

# Check if there are other places where input.specs[name] pattern is used
rg -n "\.specs\[" packages/worker/src/mcp/skills/skill-embed-and-flags.ts

Repository: kentcdodds/kody

Length of output: 108


🏁 Script executed:

# Check the rest of the file to see deriveTrustFlags function fully
sed -n '79,115p' packages/worker/src/mcp/skills/skill-embed-and-flags.ts

Repository: kentcdodds/kody

Length of output: 965


Use own-property checks when filtering inferred capability names.

At line 62, input.specs[name] != null can accept prototype-inherited keys (for example toString) as valid capability names, introducing bogus inferred capability tokens into embeddings. This inconsistency also appears alongside the safer Object.keys() pattern used in mergeInferredCapabilityNames in the same file. Use Object.prototype.hasOwnProperty.call(input.specs, name) to check only own properties, and deduplicate the input list:

Proposed fix
-	const inferredCapabilityNames = input.inferredCapabilities
-		.filter((name) => input.specs[name] != null)
-		.sort()
+	const inferredCapabilityNames = [...new Set(input.inferredCapabilities)]
+		.filter((name) =>
+			Object.prototype.hasOwnProperty.call(input.specs, name),
+		)
+		.sort()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const inferredCapabilityNames = input.inferredCapabilities
.filter((name) => input.specs[name] != null)
.sort()
const inferredCapabilityNames = [...new Set(input.inferredCapabilities)]
.filter((name) =>
Object.prototype.hasOwnProperty.call(input.specs, name),
)
.sort()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/skills/skill-embed-and-flags.ts` around lines 61 -
63, Filter inferred capability names using an own-property check and remove
duplicates: replace the current filter on input.inferredCapabilities that uses
input.specs[name] != null with a check using
Object.prototype.hasOwnProperty.call(input.specs, name) to ensure only own keys
of input.specs are accepted, deduplicate the list (e.g., via a Set) and then
sort it; refer to inferredCapabilityNames, input.inferredCapabilities,
input.specs and the existing mergeInferredCapabilityNames pattern for
consistency.

Comment on lines +42 to +49
| {
type: 'app'
id: string
title: string
description: string
usage: string
hostedUrl: string
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve app-bound secret metadata in the new search output.

searchUnified still attaches availableSecrets to app hits, but the new slim app shapes and markdown renderers drop that field completely here. After this change, neither ranked search results nor entity: "...:app" detail tells callers which app secrets exist, which regresses the "nest app secrets on apps" behavior and makes saved-app discovery less useful.

Also applies to: 90-101, 211-230, 239-275, 412-451

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/tools/search-format.ts` around lines 42 - 49, The new
slim app shape dropped the app-bound secret metadata: update the app result
shape (the object with type: 'app' that currently lists id, title, description,
usage, hostedUrl) to include the availableSecrets field and ensure every place
that maps search hits into that shape (including searchUnified result mapping
and the markdown renderers that build app outputs) copies availableSecrets from
the original hit into the new shape; apply the same addition to the other
mapping sites you noted (lines ~90-101, ~211-230, ~239-275, ~412-451) so ranked
results and entity: "...:app" details continue to expose the app's
availableSecrets.

Comment on lines +330 to +333
const row = input.searchRows.userSecretRows.find((secret) => secret.name === ref.id)
if (!row) {
throw new Error('Secret not found for this user.')
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't resolve exact secret entities from the best-effort search list.

This branch depends on searchRows.userSecretRows, but loadOptionalSearchRows(...) intentionally downgrades secret-loader failures into warnings. In that case, Line 331 throws "Secret not found for this user." for a transient backend failure instead of surfacing the real problem. Exact entity: "...:secret" lookups should fetch directly by name or propagate the loader error.

🤖 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 330 - 333, The code is
using the best-effort list input.searchRows.userSecretRows to resolve an exact
secret (ref.id), which hides transient loader failures from
loadOptionalSearchRows; instead replace this lookup with a direct fetch (call
the secret loader / fetch-by-name function rather than searching userSecretRows)
or detect and rethrow the loader error produced by loadOptionalSearchRows so we
don’t convert backend failures into "Secret not found for this user."; update
the branch around the lookups that reference input.searchRows.userSecretRows and
the thrown message "Secret not found for this user." to use a direct
secret-by-name fetch or propagate the original loader error.

Comment on lines +39 to +42
const rows = await listAllVisibleUiArtifacts(env)
if (rows.length === 0) {
return { upserted: 0 }
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This reindex path never removes stale app vectors.

Because it only scans hidden = 0 rows and then calls upsert, any ui_artifact_<uuid> already left behind for a now-hidden or deleted app survives this maintenance run. That means the endpoint cannot fully repair D1/Vectorize drift; it needs a delete phase or rebuild strategy for ids that are no longer visible.

Also applies to: 94-104

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/ui-artifact-reindex.ts` around lines 39 - 42, The
current reindex only calls listAllVisibleUiArtifacts and upserts visible items
(see listAllVisibleUiArtifacts and the upsert loop around lines 94-104), so
stale vector rows named like ui_artifact_<uuid> for hidden/deleted apps are
never removed; modify the routine to perform a delete phase: either (A) fetch
existing vector IDs from the vector store (matching prefix ui_artifact_),
compute the set difference against the visible rows returned by
listAllVisibleUiArtifacts, and delete those stale ids before/after upserting, or
(B) perform a full rebuild by removing all ui_artifact_* vectors first and then
upserting the current visible rows—implement the chosen strategy and ensure the
code references listAllVisibleUiArtifacts, the upsert logic, and the
vector-store delete operation so stale vectors are removed.

Comment thread packages/worker/src/mcp/ui-artifacts-embed.node.test.ts Outdated
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/mcp/capabilities/unified-search.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/unified-search.ts (1)

361-366: ⚠️ Potential issue | 🟡 Minor

Keep a description on capability summary hits.

Line 365 forces searchCapabilities onto the summary row. The summary shape in packages/worker/src/mcp/capabilities/capability-search.ts omits description, and packages/worker/src/mcp/tools/search-format.ts falls back to an empty string when that field is missing. The new compact search output will therefore render builtin capability matches without descriptions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/unified-search.ts` around lines 361 -
366, searchCapabilities is being called with detail: false in unified-search.ts
which returns the compact summary shape (from capability-search.ts) that omits
description, causing search-format.ts to fall back to an empty string for
builtin capabilities; change the call to searchCapabilities({ ..., detail: true,
... }) so the returned hits include the description field, or otherwise ensure
the summary includes description by requesting the detailed shape in
unified-search.ts (referencing searchCapabilities, capability-search.ts, and
search-format.ts).
🤖 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/capabilities/unified-search.ts`:
- Around line 361-366: searchCapabilities is being called with detail: false in
unified-search.ts which returns the compact summary shape (from
capability-search.ts) that omits description, causing search-format.ts to fall
back to an empty string for builtin capabilities; change the call to
searchCapabilities({ ..., detail: true, ... }) so the returned hits include the
description field, or otherwise ensure the summary includes description by
requesting the detailed shape in unified-search.ts (referencing
searchCapabilities, capability-search.ts, and search-format.ts).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcb3e4d3-f2f9-4480-a9b1-f1ba803f3f39

📥 Commits

Reviewing files that changed from the base of the PR and between 4a93c1d and 332aa39.

📒 Files selected for processing (1)
  • packages/worker/src/mcp/capabilities/unified-search.ts

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
packages/worker/src/mcp/capabilities/capability-search.ts (2)

143-145: Redundant description assignment in toDetail.

toSummary(spec) now returns description, so the explicit description: spec.description on line 145 overwrites the same value. Safe to remove.

♻️ Proposed cleanup
 	const row: CapabilityDetailRow = {
 		...toSummary(spec),
-		description: spec.description,
 		keywords: spec.keywords,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/capability-search.ts` around lines 143 -
145, The toDetail function constructs a CapabilityDetailRow by spreading
toSummary(spec) which already includes description; remove the redundant
explicit assignment description: spec.description from the object literal in
toDetail so the description from toSummary(spec) is not overwritten and behavior
remains the same (update the object in toDetail that builds CapabilityDetailRow,
referencing toSummary and spec).

106-116: Consider removing redundant description from CapabilityDetailRow.

Since CapabilitySummaryRow now includes description, the explicit description: string declaration in CapabilityDetailRow (line 107) is redundant—it's already inherited via the & intersection.

♻️ Proposed cleanup
 export type CapabilityDetailRow = CapabilitySummaryRow & {
-	description: string
 	keywords: Array<string>
 	readOnly: boolean
 	idempotent: boolean
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/capability-search.ts` around lines 106 -
116, CapabilityDetailRow redundantly redeclares description already provided by
CapabilitySummaryRow; remove the explicit "description: string" property from
the CapabilityDetailRow type declaration so it simply extends
CapabilitySummaryRow (& { keywords: Array<string>; readOnly: boolean;
idempotent: boolean; destructive: boolean; inputSchema: unknown; outputSchema?:
unknown; inputFields?: Array<string>; outputFields?: Array<string> }) and run
type checks to ensure no other code expects the duplicate declaration.
🤖 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/src/mcp/capabilities/capability-search.ts`:
- Around line 143-145: The toDetail function constructs a CapabilityDetailRow by
spreading toSummary(spec) which already includes description; remove the
redundant explicit assignment description: spec.description from the object
literal in toDetail so the description from toSummary(spec) is not overwritten
and behavior remains the same (update the object in toDetail that builds
CapabilityDetailRow, referencing toSummary and spec).
- Around line 106-116: CapabilityDetailRow redundantly redeclares description
already provided by CapabilitySummaryRow; remove the explicit "description:
string" property from the CapabilityDetailRow type declaration so it simply
extends CapabilitySummaryRow (& { keywords: Array<string>; readOnly: boolean;
idempotent: boolean; destructive: boolean; inputSchema: unknown; outputSchema?:
unknown; inputFields?: Array<string>; outputFields?: Array<string> }) and run
type checks to ensure no other code expects the duplicate declaration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7325f6ce-dcd0-42b5-bcfb-ce68219533aa

📥 Commits

Reviewing files that changed from the base of the PR and between 332aa39 and cd31816.

📒 Files selected for processing (1)
  • packages/worker/src/mcp/capabilities/capability-search.ts

Comment thread packages/worker/src/mcp/ui-artifacts-embed.node.test.ts Outdated
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

return [`## Skill — ${match.title}`, '', match.description]
}
if (match.type === 'app') {
const hostedUrl = buildSavedUiUrl(baseUrl, match.appId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Redundant hostedUrl recomputation ignores existing match field

Low Severity

Both formatMatchBlock and toSlimStructuredMatches call buildSavedUiUrl(baseUrl, match.appId) to compute the hosted URL, but after narrowing match.type === 'app', the UiArtifactSearchHit already carries a pre-computed hostedUrl field. Using match.hostedUrl directly would avoid the redundant computation and reduce coupling to buildSavedUiUrl in these formatting functions.

Additional Locations (1)
Fix in Cursor Fix in Web

Co-authored-by: me <me@kentcdodds.com>
@kentcdodds
kentcdodds merged commit 7de2a93 into main Mar 31, 2026
8 of 9 checks passed
@kentcdodds
kentcdodds deleted the cursor/kody-mcp-skill-for-github-docs-bd7d branch March 31, 2026 14:08
@coderabbitai coderabbitai Bot mentioned this pull request Jul 13, 2026
4 of 5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants