Fix createStorybookMcpHandler not forwarding the sources option#194
Fix createStorybookMcpHandler not forwarding the sources option#194
createStorybookMcpHandler not forwarding the sources option#194Conversation
🦋 Changeset detectedLatest commit: eb0ea73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
+ Coverage 73.28% 73.91% +0.63%
==========================================
Files 42 42
Lines 1179 1177 -2
Branches 333 330 -3
==========================================
+ Hits 864 870 +6
+ Misses 198 192 -6
+ Partials 117 115 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR fixes createStorybookMcpHandler() so handler-level sources are forwarded into the per-request transport context, enabling multi-source manifest behavior to work when configured at handler creation time.
Changes:
- Forward all handler-level
StorybookContextoptions (includingsources) intotransport.respond()by default. - Keep
onSessionInitializeas a handler-level-only option (not part of per-request context). - Add tests covering handler-level
sourcesforwarding and per-request override behavior, plus a changeset.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/mcp/src/index.ts | Changes context construction/merging so handler-level options (incl. sources) flow into the transport context. |
| packages/mcp/src/index.test.ts | Adds regression tests for forwarding sources and for per-request override semantics. |
| .changeset/fair-pianos-smile.md | Documents the patch-level fix for consumers. |
You can also share your feedback on Copilot code review. Take the survey.
| return await transport.respond(req, { | ||
| ...defaultContext, | ||
| ...context, | ||
| request: req, | ||
| manifestProvider: context?.manifestProvider ?? options.manifestProvider, | ||
| onListAllDocumentation: context?.onListAllDocumentation ?? options.onListAllDocumentation, | ||
| onGetDocumentation: context?.onGetDocumentation ?? options.onGetDocumentation, | ||
| }); |
There was a problem hiding this comment.
Spreading ...context over ...defaultContext changes the precedence semantics compared to the previous nullish-coalescing merge: if a caller passes a per-request context where fields like manifestProvider, sources, onListAllDocumentation, or onGetDocumentation are present but undefined (e.g. via object-spread), this will now overwrite the handler-level defaults and can fall back to the default provider unexpectedly. Consider merging per-field with nullish coalescing (or a merge helper that ignores undefined) so handler-level options remain the default unless the request explicitly supplies a defined override.
There was a problem hiding this comment.
I think this is okay actually.
Bundle ReportChanges will decrease total bundle size by 170 bytes (-0.34%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @storybook/mcp-esmAssets Changed:
Files in
|
This docs-PR comment surfaced a problem: #172 (comment)
The
createStorybookMcpHandlerAPI didn't forward thesourcesoption to the request context, so it didn't apply in that scenario (only in the scenario where the individual tool-adders are used).Now the logic is changed so that all options are always forwarded and merged correctly.