fix: narrow engine MCP import path for bundlers#149
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 16 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds explicit ESM exports for ChangesPackage Export Narrowing & Engine import
A2A parsing and artifact typing
Agent registration normalization
WebSocket resource events
UI, provider, reducer, and adapters: event & reaction typing
SDK typing and WS emission changes
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures subpath exports for the @relaycast/mcp package, exposing a new ./server entry point, and updates the Node MCP adapter to import from @relaycast/mcp/server. Feedback suggests adding a typesVersions mapping in package.json to maintain TypeScript compatibility for consumers using older module resolution settings.
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.js" | ||
| }, | ||
| "./server": { | ||
| "types": "./dist/server.d.ts", | ||
| "import": "./dist/server.js" | ||
| } | ||
| }, | ||
| "sideEffects": false, |
There was a problem hiding this comment.
For TypeScript consumers using 'moduleResolution': 'node' (or 'node10'), subpath exports defined in the 'exports' field are not automatically resolved. To ensure maximum compatibility and prevent compilation errors for consumers importing from '@relaycast/mcp/server', it is recommended to add a 'typesVersions' mapping in 'package.json'.
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js"
},
"./server": {
"types": "./dist/server.d.ts",
"import": "./dist/server.js"
}
},
"typesVersions": {
"*": {
"server": [
"./dist/server.d.ts"
]
}
},
"sideEffects": false,![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/react/src/reducer.ts (1)
229-240:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
prev.parent.reactionsbefore mapping or spreading it.The thread-parent path still assumes
prev.parent.reactionsis always an array. If a parent message was loaded without reactions, both add and remove events can crash in these branches.Suggested fix
- const parentReactions = prev.parent.reactions as ReactionGroupView[]; + const parentReactions = (prev.parent.reactions ?? []) as ReactionGroupView[]; const existing = parentReactions.find((r) => r.emoji === event.emoji); let newReactions: ReactionGroupView[]; if (existing) { if (existing.agents.includes(event.agentName)) return prev; newReactions = parentReactions.map((r) => r.emoji === event.emoji ? { ...r, count: r.count + 1, agents: [...r.agents, event.agentName] } : r, ); } else { - newReactions = [...prev.parent.reactions, { emoji: event.emoji, count: 1, agents: [event.agentName] }]; + newReactions = [...parentReactions, { emoji: event.emoji, count: 1, agents: [event.agentName] }]; }- const newReactions = (prev.parent.reactions as ReactionGroupView[]) + const newReactions = ((prev.parent.reactions ?? []) as ReactionGroupView[]) .map((r) => r.emoji === event.emoji ? { ...r, count: Math.max(0, r.count - 1), agents: r.agents.filter((a) => a !== event.agentName) } : r, )Also applies to: 280-286
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/reducer.ts` around lines 229 - 240, The code assumes prev.parent.reactions is always an array which can be undefined and cause crashes in the add/remove reaction branches; update the logic around prev.parent.reactions (used with ReactionGroupView, existing, newReactions, and the mapping/spread operations) to first normalize or guard it (e.g. const parentReactions = Array.isArray(prev.parent.reactions) ? prev.parent.reactions : []) and use that safe array for find, map and the spread when constructing newReactions so add/remove events won’t throw when reactions are missing.
🧹 Nitpick comments (1)
packages/mcp/src/resources/ws-bridge.ts (1)
97-108: ⚡ Quick winRefine the concern:
WsClientalready drops malformed WS messages, so the cast doesn’t need runtime Zod validationIn
packages/mcp/src/resources/ws-bridge.ts(97-108),WsClient.on('*', ...)is fedWsClientEventpayloads frompackages/sdk-typescript/src/ws.ts: malformed JSON is dropped, and wildcard emissions happen only when parsing succeeds or when the payload is an object with a stringtype(plusopen/close/error/reconnectingare emitted as{ type: ... }). So the “unchecked cast can allow null/non-object through at runtime” concern doesn’t match the SDK behavior.Optional: remove the
event as ResourceEventcast (or alignResourceEventwithWsClientEvent/maketyperequired) to improve type safety without adding extra Zod parsing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mcp/src/resources/ws-bridge.ts` around lines 97 - 108, The unchecked cast to ResourceEvent in the WsClient.on('*', ...) handler is unnecessary because WsClient already drops malformed messages; remove the runtime cast (the "event as ResourceEvent") and instead either (a) align the ResourceEvent type with WsClientEvent by making the type property required so TypeScript matches the payload, or (b) change the handler signature to accept WsClientEvent and pass that into eventToResourceUris; update references to ResourceEvent, WsClientEvent, WsClient.on('*', ...) and eventToResourceUris accordingly to preserve type safety without adding Zod validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/reducer.ts`:
- Around line 7-13: EventMessage currently omits the server timestamp so
createdAt is lost and handlers fall back to new Date(), causing time rewrites
and reordering; add createdAt?: string to the EventMessage type and update the
message construction in handleMessageCreated() and handleThreadReply() to use
event.message.createdAt when populating the local MessageWithMeta (instead of
generating a new timestamp), ensuring historical message times are preserved.
- Around line 174-185: In updateReactionsOnMessage ensure you never pass
undefined into the updater: when calling updater(updated[idx].reactions as
ReactionGroupView[]) default updated[idx].reactions to an empty array (e.g. use
updated[idx].reactions ?? []) so the updater always receives
ReactionGroupView[]; then assign the updater result back to
updated[idx].reactions (cast to MessageWithMeta['reactions'] if needed) and
return updated.
---
Outside diff comments:
In `@packages/react/src/reducer.ts`:
- Around line 229-240: The code assumes prev.parent.reactions is always an array
which can be undefined and cause crashes in the add/remove reaction branches;
update the logic around prev.parent.reactions (used with ReactionGroupView,
existing, newReactions, and the mapping/spread operations) to first normalize or
guard it (e.g. const parentReactions = Array.isArray(prev.parent.reactions) ?
prev.parent.reactions : []) and use that safe array for find, map and the spread
when constructing newReactions so add/remove events won’t throw when reactions
are missing.
---
Nitpick comments:
In `@packages/mcp/src/resources/ws-bridge.ts`:
- Around line 97-108: The unchecked cast to ResourceEvent in the
WsClient.on('*', ...) handler is unnecessary because WsClient already drops
malformed messages; remove the runtime cast (the "event as ResourceEvent") and
instead either (a) align the ResourceEvent type with WsClientEvent by making the
type property required so TypeScript matches the payload, or (b) change the
handler signature to accept WsClientEvent and pass that into
eventToResourceUris; update references to ResourceEvent, WsClientEvent,
WsClient.on('*', ...) and eventToResourceUris accordingly to preserve type
safety without adding Zod validation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d898285b-17d7-4d9c-8930-2f157d28f70e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.trajectories/index.jsonpackages/engine/src/engine/a2a.tspackages/engine/src/routes/agent.tspackages/mcp/package.jsonpackages/mcp/src/resources/ws-bridge.tspackages/observer-dashboard/src/components/DashboardLayout.tsxpackages/observer-dashboard/src/components/MessageCard.tsxpackages/react/src/adapters/messages.tspackages/react/src/provider.tsxpackages/react/src/reducer.tspackages/sdk-typescript/src/agent.tspackages/sdk-typescript/src/identity.tspackages/sdk-typescript/src/ws.tspackages/server/src/engine/a2a.tspackages/server/src/routes/agent.ts
💤 Files with no reviewable changes (1)
- .trajectories/index.json
✅ Files skipped from review due to trivial changes (1)
- packages/sdk-typescript/src/ws.ts
| type EventMessage = { | ||
| id: string; | ||
| agentName: string; | ||
| agentId?: string; | ||
| text: string; | ||
| attachments?: MessageWithMeta['attachments']; | ||
| }; |
There was a problem hiding this comment.
Keep the server timestamp in EventMessage.
createdAt drops out of the local event shape here, so handleMessageCreated() and handleThreadReply() fall back to new Date().toISOString(). That rewrites historical message times and can reorder messages after reconnects or delayed delivery.
Suggested fix
type EventMessage = {
id: string;
agentName: string;
agentId?: string;
text: string;
+ createdAt: string;
attachments?: MessageWithMeta['attachments'];
};Then use event.message.createdAt when constructing the local MessageWithMeta objects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/reducer.ts` around lines 7 - 13, EventMessage currently
omits the server timestamp so createdAt is lost and handlers fall back to new
Date(), causing time rewrites and reordering; add createdAt?: string to the
EventMessage type and update the message construction in handleMessageCreated()
and handleThreadReply() to use event.message.createdAt when populating the local
MessageWithMeta (instead of generating a new timestamp), ensuring historical
message times are preserved.
| function updateReactionsOnMessage( | ||
| messages: MessageWithMeta[], | ||
| messageId: string, | ||
| updater: (reactions: ReactionGroup[]) => ReactionGroup[], | ||
| updater: (reactions: ReactionGroupView[]) => ReactionGroupView[], | ||
| ): MessageWithMeta[] | null { | ||
| const idx = messages.findIndex((m) => m.id === messageId); | ||
| if (idx === -1) return null; | ||
| const updated = [...messages]; | ||
| updated[idx] = { ...updated[idx], reactions: updater(updated[idx].reactions) }; | ||
| updated[idx] = { | ||
| ...updated[idx], | ||
| reactions: updater(updated[idx].reactions as ReactionGroupView[]) as MessageWithMeta['reactions'], | ||
| }; |
There was a problem hiding this comment.
Default missing reactions to an empty array here.
MessageWithMeta.reactions is treated as nullable elsewhere in this stack, but this cast passes undefined through to the updater. The first .find()/.map() in the reaction handlers will then throw for any message hydrated without reactions.
Suggested fix
updated[idx] = {
...updated[idx],
- reactions: updater(updated[idx].reactions as ReactionGroupView[]) as MessageWithMeta['reactions'],
+ reactions: updater(((updated[idx].reactions ?? []) as ReactionGroupView[])) as MessageWithMeta['reactions'],
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/reducer.ts` around lines 174 - 185, In
updateReactionsOnMessage ensure you never pass undefined into the updater: when
calling updater(updated[idx].reactions as ReactionGroupView[]) default
updated[idx].reactions to an empty array (e.g. use updated[idx].reactions ?? [])
so the updater always receives ReactionGroupView[]; then assign the updater
result back to updated[idx].reactions (cast to MessageWithMeta['reactions'] if
needed) and return updated.
2 participants
Summary
createRelayMcpServerfrom@relaycast/mcp/serverinstead of the package root in the engine MCP adapter./serverexport in@relaycast/mcp@relaycast/mcpas side-effect-free so bundlers can drop unused entrypoints more aggressivelyWhy
This narrows the engine's dependency edge so Workers bundlers do not have to start from the
@relaycast/mcpbarrel entrypoint when they only need server creation. That should help keep dead MCP SDK example/client code out of reachable bundle graphs.Validation
node -e "JSON.parse(require('fs').readFileSync('packages/mcp/package.json','utf8')); console.log('package.json ok')"git diff --checkexamples/clientandnode:readlinein the emitted bundleCloses #144