Core: Fix EEXIST race condition in static file copying during build#34499
Conversation
Add `force: true` to all `fs.cp()` calls in the static build pipeline. The preview build (Vite) and static file copies run in parallel via `Promise.all`, which can cause EEXIST errors when both attempt to create the same subdirectory simultaneously — especially on CI runners with slow I/O.
📝 WalkthroughWalkthroughThe static build now copies Storybook browser assets to the output directory with fs/promises.cp using { recursive: true, force: true }, enabling overwrites of existing destination targets. ChangesStatic Build File Copy
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/core-server/build-static.ts (1)
115-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
disableTelemetryguard on telemetry sends.Line 115 and Line 211 currently send telemetry even when users disable it via
core.disableTelemetry. Please add the opt-out guard back in both blocks.Proposed fix
- if (invokedBy) { + if (invokedBy && !core?.disableTelemetry) { // NOTE: we don't await this event to avoid slowing things down. // This could result in telemetry events being lost. telemetry('test-run', { runner: invokedBy, watch: false }, { configDir: options.configDir }); } @@ - if (!options.test) { + if (!options.test && !core?.disableTelemetry) { try { const generator = await storyIndexGeneratorPromise; const storyIndex = await generator?.getIndex(); const payload: any = { precedingUpgrade: await getPrecedingUpgrade(), };Also applies to: 211-229
🤖 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 `@code/core/src/core-server/build-static.ts` around lines 115 - 119, The telemetry sends are missing the opt-out check; wrap both places that call telemetry(...) (the block that runs when invokedBy is set and the other block later that also calls telemetry) with a guard checking core.disableTelemetry (e.g. only call telemetry if core.disableTelemetry is falsy), so use the existing disableTelemetry flag (core.disableTelemetry) to skip the telemetry('test-run', ...) invocation and the other telemetry invocation in the file.
🤖 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.
Outside diff comments:
In `@code/core/src/core-server/build-static.ts`:
- Around line 115-119: The telemetry sends are missing the opt-out check; wrap
both places that call telemetry(...) (the block that runs when invokedBy is set
and the other block later that also calls telemetry) with a guard checking
core.disableTelemetry (e.g. only call telemetry if core.disableTelemetry is
falsy), so use the existing disableTelemetry flag (core.disableTelemetry) to
skip the telemetry('test-run', ...) invocation and the other telemetry
invocation in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d47a5492-5e88-4256-82b6-3272a481325f
📒 Files selected for processing (1)
code/core/src/core-server/build-static.ts
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 79 KB | 79 KB | 🎉 -48 B 🎉 |
| Dependency size | 33.35 MB | 33.36 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 0 B |
| Dependency size | 30.73 MB | 30.75 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 30.07 MB | 30.08 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 662 KB | 662 KB | 0 B |
| Dependency size | 61.51 MB | 61.52 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 23 KB | 23 KB | 0 B |
| Dependency size | 46.05 MB | 46.06 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 16 KB | 16 KB | 0 B |
| Dependency size | 34.61 MB | 34.63 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 164 | 164 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 32.35 MB | 32.37 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Closes #18686
What I did
Added
force: trueto allfs.cp()calls in the static build pipeline.In
buildStaticStandalone, the preview build (Vite/Webpack) and static file copies run concurrently viaPromise.all:When both operations write to the same
outputDir, they can attempt tomkdirthe same subdirectory simultaneously. Withoutforce: true,fs.cpinternally callsmkdir(dest)(notmkdir(dest, { recursive: true })), so the second call fails withEEXIST.This is timing-dependent and primarily affects CI environments where I/O is slower, causing the copy and build to overlap for longer periods.
Changes
code/core/src/core-server/utils/copy-all-static-files.ts: Addforce: trueto bothcopyAllStaticFilesandcopyAllStaticFilesRelativeToMaincode/core/src/core-server/build-static.ts: Addforce: trueto the core server public dir copyHow to test
staticDirspointing to a directory with nested subdirectoriesstorybook buildrepeatedly in a CI-like environment (constrained I/O)EEXIST: file already exists, mkdirSummary by CodeRabbit