Repository navigation
Support raw MCP content blocks from execute - #149
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 |
|
🔎 Preview deployed: https://kody-pr-149.kentcdodds.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/executor.ts`:
- Around line 216-224: extractRawContent currently only checks that
value.__mcpContent is an array but doesn't validate each ContentBlock, letting
malformed blocks propagate to consumers like prependToolMetadataContent; add a
validation step in extractRawContent that iterates the array and rejects the
whole payload (return null) if any block fails validation. Implement/invoke a
helper (e.g., validateContentBlock(block): boolean) that checks block.type and
required fields per type (for example: for 'text' ensure a non-empty text
string; for 'image' ensure data and mimeType are present and of correct types;
for other types validate their mandatory keys) and only return the cast
Array<ContentBlock> when every block passes validation. Ensure the helper is
used by extractRawContent before returning the array.
In `@packages/worker/src/mcp/tools/execute.ts`:
- Around line 193-201: The code is returning raw MCP content from
extractRawContent without enforcing a size limit, bypassing the truncation used
by formatExecutionOutput; update the block that builds the returned content
(where prependToolMetadataContent is called with resolvedConversationId and
rawContent from extractRawContent(result.result)) to detect oversized rawContent
(or individual items) and, when exceeding safe limits, replace/truncate it by
calling formatExecutionOutput(result) or otherwise trim the payload to the
bounded fallback used elsewhere so the response cannot exceed limits; ensure the
same guard is applied to the analogous path around line 206.
🪄 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: a2033dac-2bde-4bb7-81f2-f9bd875e1086
📒 Files selected for processing (4)
packages/worker/src/mcp/executor.node.test.tspackages/worker/src/mcp/executor.tspackages/worker/src/mcp/tools/execute.node.test.tspackages/worker/src/mcp/tools/execute.ts
| export function extractRawContent(value: unknown): Array<ContentBlock> | null { | ||
| if ( | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| '__mcpContent' in value && | ||
| Array.isArray((value as { __mcpContent: unknown }).__mcpContent) | ||
| ) { | ||
| return (value as { __mcpContent: Array<ContentBlock> }).__mcpContent | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/worker/src/mcp/executor.ts | head -n 230Repository: kentcdodds/kody
Length of output: 6376
🏁 Script executed:
rg "ContentBlock" packages/worker/src/mcp/ -A 2 -B 2Repository: kentcdodds/kody
Length of output: 5852
🏁 Script executed:
rg "extractRawContent" packages/worker/src/mcp/ -A 3 -B 1Repository: kentcdodds/kody
Length of output: 2780
🏁 Script executed:
cat packages/worker/src/mcp/executor.node.test.ts | grep -A 20 "extractRawContent returns MCP"Repository: kentcdodds/kody
Length of output: 512
🏁 Script executed:
cat packages/worker/src/mcp/tools/tool-response-content.ts | head -n 50Repository: kentcdodds/kody
Length of output: 749
🏁 Script executed:
rg "type ContentBlock|interface ContentBlock" --type tsRepository: kentcdodds/kody
Length of output: 725
Validate structure of MCP content blocks before returning from extractRawContent.
The function only verifies that __mcpContent is an array but does not validate individual block structure. Blocks missing required fields (e.g., text in text blocks, or data/mimeType in image blocks) will pass through unchecked and cause errors downstream when used by prependToolMetadataContent or other handlers.
Add a validation helper:
Proposed fix
+function isContentBlockArray(value: unknown): value is Array<ContentBlock> {
+ return (
+ Array.isArray(value) &&
+ value.every((block) => {
+ if (typeof block !== 'object' || block === null) return false
+ const candidate = block as Record<string, unknown>
+ if (candidate.type === 'text') return typeof candidate.text === 'string'
+ if (candidate.type === 'image') {
+ return (
+ typeof candidate.data === 'string' &&
+ typeof candidate.mimeType === 'string'
+ )
+ }
+ return false
+ })
+ )
+}
+
export function extractRawContent(value: unknown): Array<ContentBlock> | null {
if (
typeof value === 'object' &&
value !== null &&
- '__mcpContent' in value &&
- Array.isArray((value as { __mcpContent: unknown }).__mcpContent)
+ Object.hasOwn(value, '__mcpContent') &&
+ isContentBlockArray((value as { __mcpContent: unknown }).__mcpContent)
) {
return (value as { __mcpContent: Array<ContentBlock> }).__mcpContent
}
return null
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/executor.ts` around lines 216 - 224,
extractRawContent currently only checks that value.__mcpContent is an array but
doesn't validate each ContentBlock, letting malformed blocks propagate to
consumers like prependToolMetadataContent; add a validation step in
extractRawContent that iterates the array and rejects the whole payload (return
null) if any block fails validation. Implement/invoke a helper (e.g.,
validateContentBlock(block): boolean) that checks block.type and required fields
per type (for example: for 'text' ensure a non-empty text string; for 'image'
ensure data and mimeType are present and of correct types; for other types
validate their mandatory keys) and only return the cast Array<ContentBlock> when
every block passes validation. Ensure the helper is used by extractRawContent
before returning the array.
| const rawContent = extractRawContent(result.result) | ||
| return { | ||
| content: prependToolMetadataContent(resolvedConversationId, [ | ||
| { | ||
| type: 'text', | ||
| text: formatExecutionOutput(result), | ||
| }, | ||
| ...(rawContent ?? [ | ||
| { | ||
| type: 'text', | ||
| text: formatExecutionOutput(result), | ||
| }, | ||
| ]), |
There was a problem hiding this comment.
Add a size guard for raw MCP content responses.
Line 196 now bypasses the truncation path used by formatExecutionOutput, so very large __mcpContent payloads can exceed response limits and fail tool responses.
Suggested bounded fallback
const rawContent = extractRawContent(result.result)
+ const MAX_RAW_CONTENT_BYTES = 200_000
+ const rawContentJson =
+ rawContent === null ? null : JSON.stringify(rawContent)
+ const safeRawContent =
+ rawContentJson && rawContentJson.length <= MAX_RAW_CONTENT_BYTES
+ ? rawContent
+ : null
return {
content: prependToolMetadataContent(resolvedConversationId, [
- ...(rawContent ?? [
+ ...(safeRawContent ?? [
{
type: 'text',
text: formatExecutionOutput(result),
},
]),
...formatSurfacedMemoriesMarkdown(surfacedMemories),
]),
structuredContent: {
conversationId: resolvedConversationId,
- result: rawContent ? null : result.result,
+ result: safeRawContent ? null : result.result,
logs: result.logs ?? [],
...buildMemoryStructuredContent(surfacedMemories),
},
isError: false,
}Also applies to: 206-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/tools/execute.ts` around lines 193 - 201, The code is
returning raw MCP content from extractRawContent without enforcing a size limit,
bypassing the truncation used by formatExecutionOutput; update the block that
builds the returned content (where prependToolMetadataContent is called with
resolvedConversationId and rawContent from extractRawContent(result.result)) to
detect oversized rawContent (or individual items) and, when exceeding safe
limits, replace/truncate it by calling formatExecutionOutput(result) or
otherwise trim the payload to the bounded fallback used elsewhere so the
response cannot exceed limits; ensure the same guard is applied to the analogous
path around line 206.
Summary
extractRawContenthelper to detect__mcpContentsentinel results from execute sandbox codenullforstructuredContent.resultwhen raw MCP content is emittedTesting
npm exec -- vitest run --project node-unit packages/worker/src/mcp/executor.node.test.ts packages/worker/src/mcp/tools/execute.node.test.tsSummary by CodeRabbit
New Features
Refactor