Repository navigation
Add MCP conversation and memory context plumbing - #103
Conversation
Co-authored-by: me <me@kentcdodds.com>
Co-authored-by: me <me@kentcdodds.com>
📝 WalkthroughWalkthroughThis pull request adds optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
kentcdodds
left a comment
There was a problem hiding this comment.
@cursoragent Let's keep casing consistent by changing it to memoryContext
Summary
Testing
|
Co-authored-by: me <me@kentcdodds.com>
Co-authored-by: me <me@kentcdodds.com>
Co-authored-by: me <me@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-103.kentcdodds.workers.dev Worker: Mocks:
|
Co-authored-by: me <me@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts (1)
258-260: Strengthen generatedconversationIdassertion.Line 258–260 currently verifies only “non-empty string”. Consider asserting expected format/length so generator regressions are caught earlier.
Suggested test hardening
expect(typeof structuredResult?.conversationId).toBe('string') -expect((structuredResult?.conversationId ?? '').length).toBeGreaterThan(0) +expect(structuredResult?.conversationId).toMatch( + /^[0-9abcdefghjkmnpqrstvwxyz]{12}$/, +) expect(structuredResult?.result?.ok).toBe(true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts` around lines 258 - 260, The test currently only checks that structuredResult?.conversationId is a non-empty string; update the assertions in the test (the expectations around structuredResult and conversationId) to validate a stricter format—e.g., assert it matches the expected pattern/length such as a UUID v4 regex or a fixed-length alphanumeric pattern and/or a minimum length (instead of only .length > 0), so change the checks around structuredResult?.conversationId to use a regex match and/or explicit length equality in addition to the existing type check while leaving the structuredResult?.result?.ok assertion intact.
🤖 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/mcp-server.mcp-e2e.test.ts`:
- Around line 258-260: The test currently only checks that
structuredResult?.conversationId is a non-empty string; update the assertions in
the test (the expectations around structuredResult and conversationId) to
validate a stricter format—e.g., assert it matches the expected pattern/length
such as a UUID v4 regex or a fixed-length alphanumeric pattern and/or a minimum
length (instead of only .length > 0), so change the checks around
structuredResult?.conversationId to use a regex match and/or explicit length
equality in addition to the existing type check while leaving the
structuredResult?.result?.ok assertion intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25faa22f-30e6-4aaa-8a40-bedc889f886f
📒 Files selected for processing (7)
packages/worker/src/mcp/index.tspackages/worker/src/mcp/mcp-server.mcp-e2e.test.tspackages/worker/src/mcp/tools/execute.node.test.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/mcp/tools/open-generated-ui.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/mcp/tools/tool-call-context.ts


Summary
conversationIdvalues and structured optionalmemoryContextsearch,execute, andopen_generated_uito accept those fields and always returnstructuredContent.conversationIdTesting
npm run test -- packages/worker/src/mcp/tools/execute.node.test.tsnpm run test:mcp -- packages/worker/src/mcp/mcp-server.mcp-e2e.test.tsSummary by CodeRabbit
Release Notes
New Features
search,execute,open_generated_ui) now accept optionalconversationIdandmemoryContextparametersTests