Core: Fix WebSocket connection for StackBlitz/WebContainers#34281
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe pull request adds support for detecting web container environments and introduces conditional validation bypass in WebSocket upgrade handlers. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code/core/src/core-server/utils/__tests__/server-channel.test.ts (1)
312-366: Good test coverage for theskipValidationbehavior.The tests correctly verify that:
- Connections without tokens are accepted when validation is disabled
- Connections with invalid origins are accepted when validation is disabled
Both tests follow the established patterns and properly assert that
handleUpgradeis called while the socket is not destroyed.Consider adding a test case for missing
originheader withskipValidation: trueto mirror the existing "rejects connections without origin header" test, ensuring full parity in coverage.📝 Optional test case
it('accepts connections without origin header when validation is disabled', () => { const server = new EventEmitter() as any as Server; const socket = new EventEmitter() as any; socket.write = vi.fn(); socket.destroy = vi.fn(); const destroySpy = vi.spyOn(socket, 'destroy'); const handleUpgradeSpy = vi.fn(); const transport = new ServerChannelTransport(server, webContainerOptions); // `@ts-expect-error` (accessing private property) transport.socket.handleUpgrade = handleUpgradeSpy; const request = { url: '/storybook-server-channel', headers: {}, } as any; const head = Buffer.from(''); server.listeners('upgrade')[0](request, socket, head); expect(socket.write).not.toHaveBeenCalled(); expect(destroySpy).not.toHaveBeenCalled(); expect(handleUpgradeSpy).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` around lines 312 - 366, Add a test that mirrors the existing "accepts connections without token/with invalid origin" cases but uses a request with no origin header to verify skipValidation behavior; create the same setup (EventEmitter server, socket with write/destroy spies), instantiate ServerChannelTransport (same webContainerOptions used in the other tests), stub transport.socket.handleUpgrade and trigger server.listeners('upgrade')[0](request, socket, head) where request.headers is an empty object, then assert socket.write was not called, socket.destroy was not called, and handleUpgrade was called (test title: "accepts connections without origin header when validation is disabled").code/core/package.json (1)
237-237: Verify whether an exact version constraint is needed for@webcontainer/env.The dependency is correctly added to
dependenciesfor runtime use. The package is valid and version1.1.1is the latest available. However, consider pinning an exact version (1.1.1) instead of the caret constraint (^1.1.1) if stricter build reproducibility is required across environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/package.json` at line 237, Decide and apply the desired version constraint for the `@webcontainer/env` dependency in package.json: if you need strict reproducibility, replace the caret constraint "^1.1.1" with an exact "1.1.1" for the "@webcontainer/env" entry; otherwise, leave the caret to allow compatible patch/minor updates. Locate the "@webcontainer/env" key in the dependencies section of package.json and update its version string accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/core/package.json`:
- Line 237: Decide and apply the desired version constraint for the
`@webcontainer/env` dependency in package.json: if you need strict
reproducibility, replace the caret constraint "^1.1.1" with an exact "1.1.1" for
the "@webcontainer/env" entry; otherwise, leave the caret to allow compatible
patch/minor updates. Locate the "@webcontainer/env" key in the dependencies
section of package.json and update its version string accordingly.
In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts`:
- Around line 312-366: Add a test that mirrors the existing "accepts connections
without token/with invalid origin" cases but uses a request with no origin
header to verify skipValidation behavior; create the same setup (EventEmitter
server, socket with write/destroy spies), instantiate ServerChannelTransport
(same webContainerOptions used in the other tests), stub
transport.socket.handleUpgrade and trigger
server.listeners('upgrade')[0](request, socket, head) where request.headers is
an empty object, then assert socket.write was not called, socket.destroy was not
called, and handleUpgrade was called (test title: "accepts connections without
origin header when validation is disabled").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b1058aa-d232-4eb7-8abb-23aa0369655a
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
code/core/package.jsoncode/core/src/common/utils/envs.tscode/core/src/core-server/build-dev.tscode/core/src/core-server/utils/__tests__/server-channel.test.tscode/core/src/core-server/utils/get-server-channel.ts
ghengeveld
added
bug
patch:yes
|
View your CI Pipeline Execution ↗ for commit d910f6e
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit d910f6e ☁️ Nx Cloud last updated this comment at |
ghengeveld
commented
Mar 23, 2026
AriPerkkio
reviewed
Mar 23, 2026
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 50 | 🚨 +1 🚨 |
| Self size | 20.46 MB | 20.46 MB | 🎉 -780 B 🎉 |
| Dependency size | 16.54 MB | 16.55 MB | 🚨 +10 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 650 KB | 650 KB | 0 B |
| Dependency size | 60.20 MB | 59.95 MB | 🎉 -248 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 92 | 92 | 0 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -36 B 🎉 |
| Dependency size | 22.72 MB | 22.48 MB | 🎉 -248 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 121 | 121 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.79 MB | 23.54 MB | 🎉 -248 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 82 | 82 | 0 |
| Self size | 35 KB | 35 KB | 🚨 +36 B 🚨 |
| Dependency size | 20.51 MB | 20.26 MB | 🎉 -248 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 24 KB | 24 KB | 🎉 -12 B 🎉 |
| Dependency size | 44.81 MB | 44.56 MB | 🎉 -248 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 58 | 58 | 0 |
| Self size | 1.44 MB | 1.19 MB | 🎉 -248 KB 🎉 |
| Dependency size | 13.21 MB | 13.21 MB | 🎉 -6 B 🎉 |
| Bundle Size Analyzer | Link | Link |
yannbf
approved these changes
Mar 23, 2026
Member
yannbf
left a comment
yannbf
left a comment
There was a problem hiding this comment.
I agree with Ari's concerns, otherwise LGTM.
This was referenced Mar 23, 2026
Merged
valentinpalkovic
pushed a commit
that referenced
this pull request
Apr 2, 2026
Core: Fix WebSocket connection for StackBlitz/WebContainers (cherry picked from commit a5658d3)
Closed
19 tasks
github-actions
Bot
added
the
patch:done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34226
What I did
@webcontainer/envChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-34281-sha-d910f6e6. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34281-sha-d910f6e6 sandboxor in an existing project withnpx storybook@0.0.0-pr-34281-sha-d910f6e6 upgrade.More information
0.0.0-pr-34281-sha-d910f6e6fix-stackblitz-websocketd910f6e61774280029)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34281Summary by CodeRabbit
New Features
Tests