-
Notifications
You must be signed in to change notification settings - Fork 11
GET handler serving HTML, updated manifest format #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 51eea40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Bundle ReportChanges will increase total bundle size by 3.74kB (11.03%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: @storybook/addon-mcp-esmAssets Changed:
Files in
view changes for bundle: @storybook/mcp-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
- Coverage 84.50% 81.91% -2.59%
==========================================
Files 14 15 +1
Lines 813 846 +33
Branches 157 159 +2
==========================================
+ Hits 687 693 +6
- Misses 118 145 +27
Partials 8 8 ☔ View full report in Codecov by Sentry. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the component manifest format to include a required path field, adds an optional error field, makes several fields truly optional (switching from exactOptional to optional), and introduces a GET handler for the /mcp endpoint that serves an HTML page with conditional redirection to the component manifest viewer.
Key changes:
- Required
pathfield added toComponentManifestschema and all test fixtures - Changed from
v.exactOptional()tov.optional()for various manifest fields - Added optional
errorfield toBaseManifestfor error reporting - Made
Example.snippetoptional and added guard to skip examples without snippets - Introduced GET handler for
/mcpthat serves HTML to browsers and plain text to API clients - Extracted
isManifestAvailableutility function for reusability
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp/src/types.ts | Updated schema to add required path field, optional error field, and changed exactOptional to optional |
| packages/mcp/src/utils/format-manifest.ts | Added guard to skip examples without snippets |
| packages/mcp/src/utils/format-manifest.test.ts | Added path field to all test fixtures |
| packages/mcp/src/utils/get-manifest.test.ts | Added path field to test fixtures and improved formatting |
| packages/mcp/fixtures/*.fixture.json | Added path field to all component fixtures |
| packages/addon-mcp/src/tools/is-manifest-available.ts | Extracted utility function for checking manifest availability |
| packages/addon-mcp/src/preset.ts | Changed to async function, added GET handler for /mcp with HTML response and conditional redirect |
| packages/addon-mcp/src/mcp-handler.ts | Refactored to use extracted isManifestAvailable utility |
| packages/addon-mcp/CHANGELOG.md | Added changelog entries for versions 0.0.1-0.0.5 |
| CHANGELOG.md | Removed duplicate changelog content (moved to package-specific file) |
| .changeset/*.md | Added changesets for manifest format update and GET handler addition |
shilman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine we'll want to redesign this but this is a great first iteration.
Longer term I worry that people might not know what the component manifest is and so it might be better to have a list of toolsets and statuses, rather than a redirect:
Dev [description] [disabled]
Docs [description] [view manifest]
Yeah I can see how that makes sense. I'd want to avoid duplication between this page and the documentation (to avoid getting them out of sync), so we should try and link out to the docs on the matter as much as possible. |
This PR adds a GET handler to
/mcpin@storybook/addon-mcpthat shows the addon is successfully running. When component manifests exists, it automatically redirects to human readable component manifests. See storybookjs/storybook#32882Recording of the experience of visiting
/mcpnow:mcp-het.mp4
It also updates the manifest format to account for errors and missing snippets. See storybookjs/storybook#32855