Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Rspack Bundle Analysis
Main CompilerSource:
Total emitted JS: 6.34 MB Runtime CompilerSource:
Total emitted JS: 5.37 MB Package Footprint (npm pack + npm install)
|
|
| Filename | Overview |
|---|---|
| packages/xmcp/src/runtime/utils/notifications.ts | New utility that registers user notification handlers on the MCP server, wrapping SDK-internal handlers for cancelled/progress via a hard-fail guard on _notificationHandlers; implementation is solid. |
| packages/xmcp/src/runtime/transports/http/stateless-streamable-http.ts | Adds onmessage dispatch for pure-notification POST batches so handlers fire before the 202 response is sent; logic is correct for both pure-notification and mixed batches. |
| packages/xmcp/src/define-notifications.ts | Maps user-facing shorthand keys to full MCP method strings and returns a typed NotificationsConfig; TypeScript generics correctly handle known keys vs custom string keys. |
| packages/xmcp/src/types/notification.ts | Defines NotificationParams, NotificationHandler, and NotificationsConfig; types are accurate and the paramless-handler distinction via conditional type is clean. |
| packages/xmcp/src/runtime/utils/server.ts | Integrates notifications loading into createServer and configureServer alongside existing tools/prompts/resources; pattern is consistent with the rest of the codebase. |
| packages/xmcp/src/compiler/index.ts | Watches ./src/notifications.ts with onAdd/onUnlink hooks following the same pattern as middleware; hasNotifications is correctly included in telemetry. |
| examples/notifications/test-notifications.ts | Test client for the notifications example; contains a minor documentation inconsistency in the expected output for the cancelled notification. |
| apps/website/content/docs/core-concepts/notifications.mdx | New documentation page covering client-to-server notifications, handler signatures, custom methods, SDK handler preservation, and error handling; content is accurate and well-structured. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: examples/notifications/test-notifications.ts
Line: 481-483
Comment:
**Expected output comment doesn't match handler output**
The comment on line 483 says the server will print `"Request req-42 was cancelled: User clicked cancel"`, but the actual handler in `src/notifications.ts` produces `"Request req-42 cancelled: User clicked cancel"` (no `"was"`). A developer running the example would see a different string than advertised.
```suggestion
console.log(
" Expected: [notification] Request req-42 cancelled: User clicked cancel\n"
);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "Asks and fixes" | Re-trigger Greptile
valebearzotti
left a comment
There was a problem hiding this comment.
notifications are not another element/concept of mcp servers, rather than interceptors. we shouldn't add them in another folder. it breaks the foundational DX entirely. should be handled separately if any under the middleware as interceptors. we're only console logging stuff there's no reason to make a syntax for all this.
if any we should export methods like notification.cancelled() and then add whatever we want to track inside it. but should be treated as interceptors. this should be significantly simpler.
|
@greptileai review |
ArchitectureNotifications no longer use a src/notifications/ directory with per-file auto-detection. Instead, they follow the same single-file pattern as DX improvements
|
|
gotta update to match the newer config style + can you add a screenshot of a custom notification and how it's tracked? |
notifications/directory convention #525