Skip to content

Conversation

@kasperpeulen
Copy link
Contributor

@kasperpeulen kasperpeulen commented Nov 28, 2025

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Infrastructure

    • CI workspace roots, artifact/cache locations, and executor resource classes reorganized; corepack enabled in CI
    • Added GitHub Actions workflow for NX runs; updated Node.js version to 22.21.1
  • Build System

    • Project targets restructured for clearer compile/check flows; yarn cache keys simplified
    • Build/install command patterns standardized
  • Development Experience

    • Simplified local install/start commands and environment defaults; new .env flags for sandbox behavior

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2025

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","build","dependencies"]

🚫 PR title must be in the format of "Area: Summary", With both Area and Summary starting with a capital letter Good examples: - "Docs: Describe Canvas Doc Block" - "Svelte: Support Svelte v4" Bad examples: - "add new api docs" - "fix: Svelte 4 support" - "Vue: improve docs"

Generated by 🚫 dangerJS against c289d52

@nx-cloud
Copy link

nx-cloud bot commented Nov 28, 2025

View your CI Pipeline Execution ↗ for commit c289d52

Command Status Duration Result
nx run-many -t check -c production --parallel=7 ✅ Succeeded 1s View ↗
nx run-many -t compile -c production --parallel=3 ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-28 15:14:39 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

📝 Walkthrough

Walkthrough

The PR migrates CI/workspace references to a unified root layout, consolidates Yarn caching to the top-level yarn.lock, enables corepack in CI steps, adds/rewrites extensive Nx workspace and sandbox project configurations, removes many per-package npm scripts and $schema entries, and updates assorted scripts, docs, and small code fixes.

Changes

Cohort / File(s) Change Summary
CircleCI config and jobs
.circleci/config.yml, .circleci/src/jobs/*.yml
Attach workspace roots changed from . to /tmp, many persist/store paths moved under storybook/ or /tmp, added corepack enable in sandbox/install steps, adjusted artifact/cache paths and some executor classes (resource sizes).
GitHub Actions & setup action
.github/workflows/nx.yml, .github/actions/setup-node-and-install/action.yml
New nx workflow for PR/push with label-based tag filtering; simplified Yarn cache key to yarn.lock and updated action restore/restore-keys accordingly.
Root monorepo and yarn configs
package.json, nx.json, .nvmrc, .yarnrc.yml, yarn.lock (implicit)
Converted repo to monorepo roots/workspaces, added root nx configuration, bumped Node in .nvmrc to 22.21.1, adjusted root Yarn settings (enableGlobalCache, logFilters, etc.).
NX workspace (global)
nx.json, .nx/workflows/*.yaml
Added comprehensive nx.json with targetDefaults, namedInputs, and many nx reusable workflow templates; new distribution/agent configs under .nx/workflows.
CircleCI -> GitHub environment / env files
.env, code/.env, .gitignore, .ignore
Added .env with Storybook sandbox flags; removed NX_ADD_PLUGINS from code/.env; updated ignore rules and removed single-line .ignore entry.
Scripts and CI helpers
scripts/build-package.ts, .cursor/environment.json, scripts/.yarnrc.yml
Reworked build-package workspace discovery to getCodeWorkspaces(), centralized build invocation via shared script, simplified environment install command, and removed scripts-level Yarn rc.
Per-package package.json removals (scripts)
code/**/*/package.json (many — addons, builders, frameworks, renderers, presets, lib, etc.)
Removed scripts blocks (commonly check and prep) from many package.json files across code/* packages.
Per-package NX project.json updates
code/**/project.json (many)
Removed $schema entries and replaced build targets with compile and check targets; added "tags": ["ci:normal"] where applicable; numerous sandbox projects gained richer target sets and implicitDependencies.
Sandbox project manifests
code/sandbox/*/project.json (added/updated/removed many)
Large set of sandbox project JSONs were added or converted: description added, implicitDependencies expanded (cli + addons), sandbox/build targets now include options.dir, and many new targets were added (prepare-sandbox, prepare-build-sandbox, dev, vitest-integration, chromatic, serve, e2e/test-runner variants); some sandbox project files were deleted.
Code changes — small fixes & test tweaks
code/addons/vitest/src/vitest-plugin/index.ts, code/lib/cli-storybook/src/link.ts, code/e2e-tests/*.ts, code/renderers/*/src/*
Introduced commonConfig root to Vite final config type, changed CLI root package name check to @storybook/code, CI-specific test cleanup and SANDBOX_DIRECTORY usage in tests, minor TS typing and import cleanups.
Docs/snippets and contributing
docs/_snippets/*.md, CONTRIBUTING.md, CONTRIBUTING/RELEASING.md, .github/copilot-instructions.md
Fixed small syntax/whitespace issues, converted demo template bindings from quoted to expression form (label="${label}"label=${label}), updated docs and setup commands (yarn vs yarn i) and Node version notes.
Other workspace changes
code/nx.json (deleted), code/project.json, code/.nxignore, code/.yarnrc.yml (deleted)
Removed legacy code/nx.json, added code/project.json with lint/test/knip targets, removed code-level yarnrc and nxignore patterns to centralize config at repo root.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Areas needing extra attention:

  • CircleCI changes (attach path → /tmp, persisted artifact locations, corepack enable) — verify concurrency and workspace consistency.
  • Nx workspace/nx.json and .github/workflows/nx.yml — validate targetDefaults, namedInputs, task dependencies, and PR label/tag filtering logic.
  • Sandbox project additions/edits — ensure no circular implicitDependencies and that new targets map to correct directories.
  • Removal of many per-package scripts and deletion of code/nx.json/per-package yarnrc — confirm CI and local workflows still find required commands and that the centralized resolutions/devDependencies cover all cases.
  • scripts/build-package.ts and related build invocation changes — ensure build command semantics and flags (e.g., --prod) remain correct for all packages.

Possibly related PRs


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7aa225e and c289d52.

📒 Files selected for processing (1)
  • .nx/workflows/distribution-config.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .nx/workflows/distribution-config.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: merged
  • GitHub Check: nx

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
code/lib/cli-storybook/src/link.ts (1)

18-24: Root package.json name mismatch—the code change will fail validation.

The verification reveals a critical issue: the code in link.ts checks for @storybook/code, but the root package.json still identifies itself as @storybook/root. This means the validation will always fail, throwing "Expected to run link from the root of the storybook monorepo" when the link command runs.

Additionally, scripts/utils/workspace.ts:22 still explicitly filters by the old @storybook/root name, indicating incomplete migration.

Required fixes:

  • Update root package.json to use "name": "@storybook/code"
  • Update scripts/utils/workspace.ts:22 to reference the new name or verify the filter logic is still correct after the rename
.github/copilot-instructions.md (1)

150-156: Fix duplicate/contradictory compile commands in “For Code Changes”

Step 2 recommends yarn task --task compile from the repo root, but step 4 repeats compilation as cd code && yarn task --task compile. That’s both redundant and potentially wrong if task is only wired at the root.

I’d suggest making step 4 either a no-op (remove it) or use the same root-level command:

-4. Compile packages with `cd code && yarn task --task compile`
+4. Recompile packages if needed: `yarn task --task compile`
.circleci/config.yml (1)

365-366: Inconsistent corepack enable: some use sudo, others don't.

Line 366 uses sudo corepack enable in the create-sandboxes job, but other jobs (lines 182, 285, 327, 407, 438, 1031, 1104) use unprefixed corepack enable. If the build environment requires sudo for corepack, all invocations should use it; if not, none should. Inconsistency could cause failures in jobs where corepack enable silently fails due to permissions. Standardize this across all jobs or document why some require sudo.

♻️ Duplicate comments (6)
code/lib/create-storybook/project.json (1)

5-8: Consistent NX target pattern with html renderer.

This follows the same structure as code/renderers/html/project.json with empty compile and check targets. Ensure central NX configuration handles these consistently across all packages. See related verification request in the html project review.

code/core/project.json (1)

5-8: Consistent NX configuration pattern across packages.

This follows the same systematic pattern as code/renderers/html/project.json and code/lib/create-storybook/project.json. The empty compile and check targets should be verified to execute correctly via central NX configuration (see related verification request in the html renderer review).

code/addons/pseudo-states/project.json (1)

1-9: Same verification applies as code/addons/vitest/project.json.

This file follows the same restructuring pattern (compile/check targets, $schema removal, ci:normal tag). The verification script from the vitest addon applies here as well.

code/presets/create-react-app/project.json (1)

1-9: Same verification applies as code/addons/vitest/project.json.

This file follows the same restructuring pattern. The verification script from the vitest addon applies here as well.

code/frameworks/nextjs/project.json (1)

1-9: Same verification applies as code/addons/vitest/project.json.

This file follows the same restructuring pattern. The verification script from the vitest addon applies here as well.

code/lib/cli-storybook/project.json (1)

1-9: Same verification applies as code/addons/vitest/project.json.

This file follows the same restructuring pattern. The verification script from the vitest addon applies here as well.

🧹 Nitpick comments (13)
code/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.ts (1)

14-17: Consider a minimal local channel type instead of any

Using any here drops type safety around emit/once. If the goal is just to decouple from storybook/internal/channels, you can keep strict-ish typings with a local type:

type AddonsChannel = {
  emit: (event: string, ...args: any[]) => void;
  once: (event: string, cb: (...args: any[]) => void) => void;
};

declare global {
  // eslint-disable-next-line no-var
  var __STORYBOOK_ADDONS_CHANNEL__: AddonsChannel;
}

This avoids importing internal types while still giving consumers editor help and basic checks. Please verify the shape matches the real channel API used in this template.

code/e2e-tests/addon-onboarding.spec.ts (1)

21-25: CI‑only cleanup is reasonable; verify no unintended cross‑test side effects

The CI‑gated removal of ~/.storybook/settings.json before running the onboarding flow makes sense to guarantee a clean first‑run state, and { force: true } avoids failures if the file doesn’t exist. The eslint disable for playwright/no-conditional-in-test is justified here.

Please double‑check that:

  • No other E2E specs rely on ~/.storybook/settings.json persisting across tests, and
  • This spec won’t race with other workers that might read/write the same file when tests run in parallel in CI.

If there are such dependencies, it might be safer to serialize those tests or move them to a separate project that uses an isolated home directory.

code/sandbox/ember-3-js/project.json (1)

3-3: Ensure auto-generated notice is accurate.

The description states the file is auto-generated. Verify that this file should not be manually edited, and that the generation script (scripts/create-nx-sandbox-projects.ts) creates semantically correct NX configurations for all sandbox projects.

code/renderers/web-components/src/docs/web-components-properties.test.ts (1)

43-44: Consider adding spy: true option to package mocks.

Per coding guidelines, Vitest mocks should use the spy: true option for package mocks. This is a pre-existing pattern in the file, not introduced by this PR.

Apply this diff to align with the mocking guidelines:

-vi.mock('lit', () => ({ default: {} }));
-vi.mock('lit/directive-helpers.js', () => ({ default: {} }));
+vi.mock('lit', { spy: true });
+vi.mock('lit/directive-helpers.js', { spy: true });
.yarnrc.yml (1)

3-3: Verify enableGlobalCache behavior in CI.

Enabling enableGlobalCache: true allows Yarn to use a shared cache across the system. Ensure this is appropriate for your CI environment and doesn't cause conflicts between concurrent builds or different branches.

scripts/build-package.ts (1)

14-26: Unify path imports for consistency

You’re importing join from 'node:path' and resolve from 'path'. They’re equivalent here, but mixing both forms is slightly noisy. Consider consolidating to a single module (e.g., all from 'node:path') for clarity.

code/addons/vitest/src/vitest-plugin/index.ts (1)

138-157: commonConfig.root wiring into viteFinal looks correct; consider using ViteUserConfig for typing

Passing commonConfig = { root: resolve(finalOptions.configDir, '..') } into:

presets.apply<{ plugins?: Plugin[]; root: string }>('viteFinal', commonConfig)

nicely gives viteFinal the project root context builder‑vite expects, and keeps your later mergeConfig/withoutVitePlugins calls behaving as before.

If you want slightly more future‑proof typing, you could use ViteUserConfig instead of a bespoke { plugins?: Plugin[]; root: string }, so any additional fields returned by viteFinal stay properly typed:

const commonConfig: Pick<ViteUserConfig, 'root'> = { root: resolve(finalOptions.configDir, '..') };

const viteConfigFromStorybook = await presets.apply<ViteUserConfig>('viteFinal', commonConfig);

Purely optional; current code is functionally fine.

package.json (1)

44-64: Review extensive dependency resolutions for conflicts.

Lines 44–64 introduce a large resolutions block pinning many transitive dependencies. While this improves stability, verify that these pinned versions:

  • Don't conflict with specific package requirements in code/, scripts/, or workspace packages
  • Are compatible with the Node >=22.0.0 requirement
  • Are documented and maintained as dependencies evolve

Consider adding a comment explaining the rationale for these pins.

code/project.json (1)

4-29: Compilation dependency on all packages may cause performance bottleneck.

Each target (lint, test, knip) depends on the compile target of all projects ("projects": ["*"]). This means:

  • Every lint run must recompile every package in the monorepo
  • Every test run must recompile every package
  • This could significantly slow down CI and local development

Consider whether this dependency is necessary. Options:

  1. Depend only on direct/package dependencies: If code/ depends on specific packages, depend only on their compile targets
  2. Skip compilation for certain targets: If linting and testing don't require compiled artifacts, remove the dependsOn constraint
  3. Separate compile step in CI: Compile once in CI before running lint/test in parallel
  4. Use NX affected: Optimize with nx affected to only compile/test changed packages

Example (if only direct dependencies need compiling):

-      "dependsOn": [{ "projects": ["*"], "target": "compile" }],
+      "dependsOn": [{ "projects": ["dependencies"], "target": "compile" }],

Or, if compilation isn't needed:

-      "dependsOn": [{ "projects": ["*"], "target": "compile" }],
+      "dependsOn": [],

Can you clarify whether the full compilation dependency is intentional?

.github/workflows/nx.yml (1)

35-49: Add default/fallback behavior for pull requests without CI labels.

The workflow defines label-based conditionals for PRs (lines 36-44) but has no fallback case if a PR doesn't have any of the ci:normal, ci:merged, or ci:daily labels. This could cause the workflow to silently skip task execution for unlabeled PRs.

Consider adding a default behavior:

      # --- PRs ---
      - if: github.event_name == 'pull_request' &&
          contains(github.event.pull_request.labels.*.name, 'ci:normal')
        run: yarn nx run-many -t $ALL_TASKS -c production -p="tag:ci:normal"
      - if: github.event_name == 'pull_request' &&
          contains(github.event.pull_request.labels.*.name, 'ci:merged')
        run: yarn nx run-many -t $ALL_TASKS -c production -p="tag:ci:normal,tag:ci:merged"
      - if: github.event_name == 'pull_request' &&
          contains(github.event.pull_request.labels.*.name, 'ci:daily')
        run: yarn nx run-many -t $ALL_TASKS -c production -p="tag:ci:normal,tag:ci:merged,tag:ci:daily"
+     - if: github.event_name == 'pull_request' &&
+         !contains(github.event.pull_request.labels.*.name, 'ci:normal') &&
+         !contains(github.event.pull_request.labels.*.name, 'ci:merged') &&
+         !contains(github.event.pull_request.labels.*.name, 'ci:daily')
+       run: yarn nx run-many -t $ALL_TASKS -c production -p="tag:ci:normal"

This ensures that PRs without explicit labels still run ci:normal tasks by default.

.nx/workflows/agents.yaml (1)

28-71: High code duplication in init-steps anchors.

Lines 3-27 (linux-init-steps) and lines 28-71 (linux-browsers-init-steps) are nearly identical, with only the browser-specific cache and installation steps differing. Consider refactoring to reduce duplication: define a base anchor and extend it selectively, or use YAML merge keys more aggressively to avoid repeating checkout, Node install, and yarn install logic.

nx.json (1)

57-64: Clarify sandbox output path configuration.

The sandbox target (line 57-64) uses both {projectName} in the command template and {options.dir} in the outputs path. For a project named "nextjs/15-ts" with options.dir "nextjs-15-ts", ensure the output path correctly resolves to {workspaceRoot}/sandbox/nextjs-15-ts (not {workspaceRoot}/sandbox/nextjs/15-ts). Add a comment or validation to make this mapping explicit, as template name format may cause confusion.

.circleci/config.yml (1)

1-1149: Schema validation and feature flags: useInferencePlugins and analyzeSourceFiles.

The config references large files and numerous Nx operations. The nx.json specifies useInferencePlugins: false (nx.json line 12) and analyzeSourceFiles: false (nx.json line 8), which disables Nx's automatic plugin inference. Ensure that all necessary plugins are explicitly configured and that project configurations fully define their targets. A missing target definition could silently be skipped rather than failing loudly, leading to incomplete CI runs.

- run:
command: |
corepack enable
cp /tmp/storybook-sandboxes /tmp/storybook/sandbox -r --remove-destination
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's check the file size and read around lines 328-333
wc -l .circleci/config.yml
sed -n '320,340p' .circleci/config.yml

Repository: storybookjs/storybook

Length of output: 1201


🏁 Script executed:

#!/bin/bash
# Look for agents.yaml and check line 75
fd -name agents.yaml -type f

Repository: storybookjs/storybook

Length of output: 295


🏁 Script executed:

#!/bin/bash
# Search for STORYBOOK_SANDBOX_ROOT references across the codebase
rg -n "STORYBOOK_SANDBOX_ROOT" -C 3

Repository: storybookjs/storybook

Length of output: 728


🏁 Script executed:

#!/bin/bash
# Get more context from constants.ts - find ROOT_DIRECTORY
head -20 scripts/utils/constants.ts

Repository: storybookjs/storybook

Length of output: 878


🏁 Script executed:

#!/bin/bash
# Search for when/why this STORYBOOK_SANDBOX_ROOT override was added
git log --oneline -S "STORYBOOK_SANDBOX_ROOT=./sandbox" -- .circleci/config.yml | head -10

Repository: storybookjs/storybook

Length of output: 79


🏁 Script executed:

#!/bin/bash
# Look for any comments or documentation about sandbox directory setup
rg -i "sandbox.*root|storybook-sandboxes" --type-list | head -20
rg -i "storybook.*sandbox" -C 2 --type md

Repository: storybookjs/storybook

Length of output: 2468


🏁 Script executed:

#!/antml/bash
# Check if there are any build/setup scripts that explain the directory structure
fd -name "*.sh" -o -name "setup*" -o -name "prepare*" | head -20
rg -i "tmp/storybook|storybook-sandboxes" -C 2

Repository: storybookjs/storybook

Length of output: 3936


🏁 Script executed:

#!/bin/bash
# Find agents.yaml
fd -name "agents.yaml" -type f

Repository: storybookjs/storybook

Length of output: 295


🏁 Script executed:

#!/bin/bash
# Look at the git commit that modified this
git show 7aa225ee7f --stat

Repository: storybookjs/storybook

Length of output: 50378


🏁 Script executed:

#!/bin/bash
# Check if there are references to agents.yaml in the codebase
rg -i "agents\.yaml" -C 2

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check the full context of the chromatic-sandboxes job in config.yml
sed -n '310,345p' .circleci/config.yml

Repository: storybookjs/storybook

Length of output: 1677


🏁 Script executed:

#!/bin/bash
# Check agents.yaml and see line 75
head -80 .nx/workflows/agents.yaml | tail -10

Repository: storybookjs/storybook

Length of output: 297


🏁 Script executed:

#!/bin/bash
# Get more context from agents.yaml - look for sandbox references
rg -C 3 "sandbox" .nx/workflows/agents.yaml

Repository: storybookjs/storybook

Length of output: 266


🏁 Script executed:

#!/bin/bash
# Search for all STORYBOOK_SANDBOX_ROOT references in the full config
grep -n "STORYBOOK_SANDBOX_ROOT" .circleci/config.yml | head -20

Repository: storybookjs/storybook

Length of output: 397


🏁 Script executed:

#!/antml/bash
# Check if the create-sandboxes job or other setup jobs explain the path choice
sed -n '1,100p' .circleci/config.yml | grep -A 20 "create-sandboxes"

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

#!/bash
# Look for where /tmp/storybook/sandbox is created or populated
rg -n "tmp/storybook/sandbox|tmp/storybook-sandboxes" .circleci/config.yml

Repository: storybookjs/storybook

Length of output: 163


🏁 Script executed:

#!/bin/bash
# Find create-sandboxes job
rg -n "create-sandboxes" .circleci/config.yml | head -5

Repository: storybookjs/storybook

Length of output: 259


🏁 Script executed:

#!/bash
# Look at create-sandboxes job definition
sed -n '/create-sandboxes:/,/^    [a-z]/p' .circleci/config.yml | head -60

Repository: storybookjs/storybook

Length of output: 2360


🏁 Script executed:

#!/bash
# Check attach_workspace and save_cache patterns to understand sandbox movement
grep -n "attach_workspace\|save_cache" .circleci/config.yml | grep -A 2 -B 2 "sandboxes\|/tmp"

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

#!/bash
# Verify the complete workspace and path setup
sed -n '310,345p' .circleci/config.yml | grep -E "at:|root:|paths:|cp /tmp"

Repository: storybookjs/storybook

Length of output: 184


🏁 Script executed:

#!/bash
# Check what ROOT_DIRECTORY resolves to in the build context
pwd
echo "---"
cat scripts/utils/constants.ts | head -25

Repository: storybookjs/storybook

Length of output: 1005


STORYBOOK_SANDBOX_ROOT override in chromatic-sandboxes masks a path inconsistency that should be addressed in base configuration.

The chromatic-sandboxes job (lines 328-333) copies sandboxes from /tmp/storybook-sandboxes to /tmp/storybook/sandbox, then overrides STORYBOOK_SANDBOX_ROOT=./sandbox to make the code find them. However, agents.yaml (line 75) sets the default as STORYBOOK_SANDBOX_ROOT: ../storybook-sandboxes, reflecting the expectation that sandboxes are a sibling directory. CircleCI's workspace persists them to /tmp/storybook-sandboxes at the workspace root, not as a sibling—the copy and job-level override compensate for this mismatch.

Rather than relying on workarounds, align the base configuration: either adjust where create-sandboxes persists the workspace to match the expected relative path, or update scripts/utils/constants.ts to handle the CircleCI layout natively.

🤖 Prompt for AI Agents
In .circleci/config.yml around line 328, the chromatic-sandboxes job currently
copies /tmp/storybook-sandboxes into /tmp/storybook/sandbox and overrides
STORYBOOK_SANDBOX_ROOT to ./sandbox to mask a path mismatch with agents.yaml
(which expects ../storybook-sandboxes); fix by aligning the base configuration
instead of using the copy+override workaround: either change the
create-sandboxes/workspace persistence so the sandboxes are written as a sibling
directory matching agents.yaml (../storybook-sandboxes) or update
scripts/utils/constants.ts to detect and prefer the CircleCI workspace layout
(/tmp/storybook-sandboxes) when present, then remove the cp step and the
job-level STORYBOOK_SANDBOX_ROOT override so both local and CI runs use the same
canonical path.

Comment on lines +19 to +22
corepack enable
cp /tmp/storybook-sandboxes /tmp/storybook/sandbox -r --remove-destination
TEMPLATE=$(yarn get-template --cadence << pipeline.parameters.workflow >> --task chromatic)
cd sandbox/$(yarn get-sandbox-dir --template $TEMPLATE) && yarn
cd $(STORYBOOK_SANDBOX_ROOT=./sandbox yarn get-sandbox-dir --template $TEMPLATE) && yarn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

cp options are in the wrong place and will break sandbox copying

The cp invocation is currently:

cp /tmp/storybook-sandboxes /tmp/storybook/sandbox -r --remove-destination

On GNU cp, options must come before the first non‑option argument; as written, -r/--remove-destination are treated as paths, so this command will fail and the sandbox won’t be copied.

Recommend:

-        cp /tmp/storybook-sandboxes /tmp/storybook/sandbox -r --remove-destination
+        cp -r --remove-destination /tmp/storybook-sandboxes /tmp/storybook/sandbox
🤖 Prompt for AI Agents
In .circleci/src/jobs/chromatic-sandboxes.yml around lines 19 to 22, the cp
command passes options after the source/destination so GNU cp treats them as
paths and fails; reorder the invocation so options precede non-option arguments
(e.g., place -r and --remove-destination immediately after cp) to ensure the
sandbox directory is copied correctly.

Comment on lines 20 to 24
- store_test_results:
path: test-results
path: storybook/test-results
- store_artifacts:
path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/
path: storybook/test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/
destination: playwright
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the full CircleCI config file to understand the working_directory and paths
cat -n ".circleci/src/jobs/e2e-ui-vitest-3.yml"

Repository: storybookjs/storybook

Length of output: 1046


🏁 Script executed:

#!/bin/bash
# Look for similar e2e CircleCI config files to understand the pattern
find .circleci/src/jobs -name "e2e-ui-vitest*.yml" -type f | sort

Repository: storybookjs/storybook

Length of output: 103


🏁 Script executed:

#!/bin/bash
# Check for all CircleCI job files and search for storybook/ references
find .circleci/src/jobs -name "*.yml" -type f | head -20

Repository: storybookjs/storybook

Length of output: 799


🏁 Script executed:

#!/bin/bash
# Check e2e-ui.yml for comparison
cat -n ".circleci/src/jobs/e2e-ui.yml"

Repository: storybookjs/storybook

Length of output: 1019


🏁 Script executed:

#!/bin/bash
# Check build.yml to understand the workspace structure
head -50 ".circleci/src/jobs/build.yml"

Repository: storybookjs/storybook

Length of output: 1378


🏁 Script executed:

#!/bin/bash
# Search for where test-storybooks directories are created
rg -n "test-storybooks" ".circleci/src/jobs/" -A 2 -B 2

Repository: storybookjs/storybook

Length of output: 5021


🏁 Script executed:

#!/bin/bash
# Look for main CircleCI config
find .circleci -name "config.yml" -o -name "config.yaml" | head -5

Repository: storybookjs/storybook

Length of output: 85


🏁 Script executed:

#!/bin/bash
# Search for e2e-ui-vitest-3 in the main config
rg -n "e2e-ui-vitest-3" ".circleci/config.yml" -A 5 -B 5

Repository: storybookjs/storybook

Length of output: 1609


🏁 Script executed:

#!/bin/bash
# Search for where test-storybooks is created or persisted
rg -n "persist_to_workspace" ".circleci/src/jobs/" -A 20 | grep -E "(persist_to_workspace|test-storybooks|portable-stories)" -A 3 -B 1

Repository: storybookjs/storybook

Length of output: 800


🏁 Script executed:

#!/bin/bash
# Search for test-storybooks directory in the repository
find . -maxdepth 2 -type d -name "test-storybooks" 2>/dev/null

Repository: storybookjs/storybook

Length of output: 82


🏁 Script executed:

#!/antml/bin/bash
# Check what git-shallow-clone does and understand the checkout flow
rg -n "git-shallow-clone" ".circleci/src/" -A 5 -B 2 | head -30

Repository: storybookjs/storybook

Length of output: 1783


🏁 Script executed:

#!/bin/bash
# Let's look for vitest config to see where test results are actually generated
find . -name "vitest.config.*" -type f | grep -i "test-storybooks\|portable-stories" | head -5

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Search for vitest configs
find . -name "vitest.config.*" -type f

Repository: storybookjs/storybook

Length of output: 1922


🏁 Script executed:

#!/bin/bash
# Check the structure of test-storybooks
ls -la test-storybooks/ | head -20

Repository: storybookjs/storybook

Length of output: 571


🏁 Script executed:

#!/bin/bash
# Check portable-stories-kitchen-sink structure
ls -la test-storybooks/portable-stories-kitchen-sink/

Repository: storybookjs/storybook

Length of output: 453


🏁 Script executed:

#!/bin/bash
# Check react-vitest-3 structure and package.json
ls -la test-storybooks/portable-stories-kitchen-sink/react-vitest-3/

Repository: storybookjs/storybook

Length of output: 1174


Working directory paths must include storybook/ prefix to align with artifact storage paths.

The CircleCI configuration has a critical path mismatch:

  • Lines 13, 19 (working_directory): test-storybooks/portable-stories-kitchen-sink/react-vitest-3
  • Lines 21, 23 (store_test_results/store_artifacts): storybook/test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/

The working directory paths are missing the storybook/ prefix. Since the workspace is attached at /tmp with all files under /tmp/storybook/, the working_directory should be:

working_directory: storybook/test-storybooks/portable-stories-kitchen-sink/react-vitest-3

This same issue exists in .circleci/src/jobs/e2e-ui.yml (lines 13, 19). Update both files to include the storybook/ prefix in working_directory paths so test results are generated in the correct location for artifact storage.

🤖 Prompt for AI Agents
In .circleci/src/jobs/e2e-ui-vitest-3.yml around lines 20-24 (and also update
.circleci/src/jobs/e2e-ui.yml around lines 13 and 19), the working_directory is
missing the required storybook/ prefix so test output is written to the wrong
path; change the working_directory values to prepend "storybook/" (e.g.,
working_directory:
storybook/test-storybooks/portable-stories-kitchen-sink/react-vitest-3) so that
the job writes test results under the same storybook/test-results/artifacts
paths used by store_test_results and store_artifacts.

Comment on lines 10 to +17
- run:
command: |
cd code
TEST_FILES=$(circleci tests glob "**/*.{stories}.{ts,tsx,js,jsx,cjs}" | sed "/^e2e-tests\//d" | sed "/^node_modules\//d")
echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose
name: Run tests
- store_test_results:
path: test-results
path: storybook/test-results
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🚨 Critical: Test results path mismatch between output and store location.

Line 14 outputs test results to ../test-results/junit-${CIRCLE_NODE_INDEX}.xml (relative to working directory code), but line 17 expects them at storybook/test-results.

With the workspace attached at /tmp, the relative path ../test-results resolves to /tmp/../test-results (outside the workspace), while CircleCI will look for results in /tmp/storybook/test-results.

Update the test output path to match the new store location:

-        echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose
+        echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../storybook/test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose

Or, if the output directory structure should be different, update the store_test_results path to match.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- run:
command: |
cd code
TEST_FILES=$(circleci tests glob "**/*.{stories}.{ts,tsx,js,jsx,cjs}" | sed "/^e2e-tests\//d" | sed "/^node_modules\//d")
echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose
name: Run tests
- store_test_results:
path: test-results
path: storybook/test-results
- run:
command: |
cd code
TEST_FILES=$(circleci tests glob "**/*.{stories}.{ts,tsx,js,jsx,cjs}" | sed "/^e2e-tests\//d" | sed "/^node_modules\//d")
echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../storybook/test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose
name: Run tests
- store_test_results:
path: storybook/test-results
🤖 Prompt for AI Agents
In .circleci/src/jobs/stories-tests.yml around lines 10 to 17, the CircleCI job
writes test output to ../test-results/junit-${CIRCLE_NODE_INDEX}.xml from inside
the code directory but configures store_test_results to look in
storybook/test-results, causing a path mismatch; fix by making the test command
write into the store_test_results directory (e.g., output to
storybook/test-results/junit-${CIRCLE_NODE_INDEX}.xml relative to the workspace)
or alternatively change store_test_results.path to ../test-results to match the
current output—ensure both paths match and are inside the workspace so CircleCI
can find and store the junit files.

Comment on lines 11 to +12
- attach_workspace:
at: .
at: /tmp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all workspace save/attach operations in CircleCI configs
echo "=== PERSIST_TO_WORKSPACE operations ==="
rg -n "persist_to_workspace" .circleci/ -A 3

echo ""
echo "=== ATTACH_WORKSPACE operations ==="
rg -n "attach_workspace" .circleci/ -A 3

Repository: storybookjs/storybook

Length of output: 13307


🏁 Script executed:

#!/bin/bash
# Check context around line 225 to identify the job
echo "=== Job context around persist_to_workspace without root (line 225) ==="
sed -n '215,235p' .circleci/config.yml

echo ""
echo "=== Check test-init-empty-windows job definition and dependencies ==="
rg -n "test-init-empty-windows" .circleci/ -B 5 -A 20

Repository: storybookjs/storybook

Length of output: 8952


Workspace attachment path mismatch confirmed—the build job must be updated to persist to /tmp.

The test-init-empty-windows job depends on the build job, which persists its workspace to the default root (.) without specifying a root parameter. However, test-init-empty-windows now attempts to attach the workspace from /tmp. This path mismatch will cause the job to fail.

Update .circleci/config.yml line 225 to add root: /tmp to the persist_to_workspace block in the build job:

- persist_to_workspace:
    root: /tmp
    paths:
      - storybook/node_modules
      - storybook/code/node_modules
      - storybook/code/addons
      - storybook/scripts/node_modules
      - storybook/code/bench
      - storybook/code/examples
      - storybook/code/frameworks
      - storybook/code/lib
      - storybook/code/core

This aligns with the monorepo refactoring pattern where all other jobs consistently use attach_workspace: at: /tmp.

🤖 Prompt for AI Agents
In .circleci/src/jobs/test-init-empty-windows.yml around lines 11 to 12, the job
is attaching the workspace at /tmp but the build job currently persists the
workspace to the default root (.), causing a path mismatch; update
.circleci/config.yml at line 225 to add root: /tmp to the persist_to_workspace
block for the build job so the persisted paths are stored under /tmp (keeping
the listed paths unchanged), ensuring attach_workspace: at: /tmp in
test-init-empty-windows can find the workspace contents.

Comment on lines +1 to +3
STORYBOOK_DISABLE_TELEMETRY=true
IN_STORYBOOK_SANDBOX=true
STORYBOOK_SANDBOX_ROOT=../storybook-sandboxes No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix environment variable key ordering and add trailing newline.

The dotenv-linter flagged two issues:

  1. Keys should be in alphabetical order: IN_STORYBOOK_SANDBOX should come before STORYBOOK_DISABLE_TELEMETRY
  2. Missing blank line at end of file

Apply this diff to fix both issues:

-STORYBOOK_DISABLE_TELEMETRY=true
-IN_STORYBOOK_SANDBOX=true
+IN_STORYBOOK_SANDBOX=true
+STORYBOOK_DISABLE_TELEMETRY=true
 STORYBOOK_SANDBOX_ROOT=../storybook-sandboxes
+
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
STORYBOOK_DISABLE_TELEMETRY=true
IN_STORYBOOK_SANDBOX=true
STORYBOOK_SANDBOX_ROOT=../storybook-sandboxes
IN_STORYBOOK_SANDBOX=true
STORYBOOK_DISABLE_TELEMETRY=true
STORYBOOK_SANDBOX_ROOT=../storybook-sandboxes
🧰 Tools
🪛 dotenv-linter (4.0.0)

[warning] 2-2: [UnorderedKey] The IN_STORYBOOK_SANDBOX key should go before the STORYBOOK_DISABLE_TELEMETRY key

(UnorderedKey)


[warning] 3-3: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)

🤖 Prompt for AI Agents
In .env around lines 1 to 3, the environment variable keys are not alphabetized
and the file lacks a trailing newline; reorder the lines so keys are
alphabetical (place IN_STORYBOOK_SANDBOX before STORYBOOK_DISABLE_TELEMETRY) and
ensure the file ends with a single blank line (add a trailing newline
character).

Comment on lines +91 to +95
windows-js:
resource-class: 'windows/medium'
image: 'windows-2022'
# TODO init-steps:
env: *common-env-vars No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's locate and examine the agents.yaml file
find . -name "agents.yaml" -type f | head -20

Repository: storybookjs/storybook

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Read the agents.yaml file to see the full context and structure
cat -n ./.nx/workflows/agents.yaml

Repository: storybookjs/storybook

Length of output: 3979


🏁 Script executed:

#!/bin/bash
# Search for any references to windows-js or Windows init-steps
rg -n "windows-js|windows.*init" --type yaml --type md

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Search more broadly for windows-js references
rg -n "windows-js" --type-list | head -20
rg -n "windows-js" -C 3

Repository: storybookjs/storybook

Length of output: 454


🏁 Script executed:

#!/bin/bash
# Search for windows-js in all files
rg -n "windows-js" -C 2

# Also search for TODO comments in the workflows directory
echo "--- Searching for TODOs in workflows ---"
rg -n "TODO" ./.nx/workflows/

Repository: storybookjs/storybook

Length of output: 159


🏁 Script executed:

#!/bin/bash
# Check git log for the agents.yaml file
git log --oneline -n 20 -- ./.nx/workflows/agents.yaml

Repository: storybookjs/storybook

Length of output: 79


🏁 Script executed:

#!/bin/bash
# Search for any workflow files that might use the agents template
find . -name "*.yaml" -o -name "*.yml" | grep -i workflow | head -20

# Also search for references to agents or launch-templates
echo "--- Searching for launch-templates references ---"
rg -n "launch-template|agents\.yaml" -C 2 | head -50

Repository: storybookjs/storybook

Length of output: 898


🏁 Script executed:

#!/bin/bash
# Search for Windows-related setup or documentation
rg -n -i "windows" -C 2 | head -100

# Also check if there's a README or documentation in .nx directory
find ./.nx -type f -name "*.md" -o -name "*.txt" -o -name "README*"

Repository: storybookjs/storybook

Length of output: 7387


Based on my verification of the codebase, I can now provide the rewritten review comment:


Windows template initialization is incomplete and requires implementation.

Line 94 contains a commented-out TODO init-steps: without any Windows-specific initialization steps. The windows-js template is missing critical setup steps that are defined in comparable templates (linux-js and linux-browsers-js), including checkout, cache restoration, Node installation, and dependency installation. This gap could cause Windows agents to skip necessary initialization when invoked. Implement Windows init-steps following the pattern of the Linux templates, adapting commands as needed for Windows environments (e.g., PowerShell compatibility, path handling).

🤖 Prompt for AI Agents
In .nx/workflows/agents.yaml around lines 91 to 95, the windows-js template
contains a commented TODO for init-steps but lacks Windows-specific
initialization, causing Windows agents to miss checkout, cache restoration, Node
installation, and dependency installation; implement an init-steps block
mirroring linux-js/linux-browsers-js but adjusted for Windows (use
PowerShell-compatible commands/paths and Windows cache keys), include steps to
checkout the repo, restore/save dependency cache, install/setup the required
Node version (or use nvm-windows if applicable), then run npm/yarn install (or
the equivalent) so the Windows agent performs the same initialization as the
Linux templates.

@@ -1,8 +1,9 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's check the current state of the file under review
echo "=== Current content of code/frameworks/react-native-web-vite/project.json (first 30 lines) ==="
head -30 "code/frameworks/react-native-web-vite/project.json"

echo -e "\n=== Checking for $schema presence in project.json files ==="
fd -path "*/project.json" -type f | head -30

Repository: storybookjs/storybook

Length of output: 545


🏁 Script executed:

#!/bin/bash
# Fixed script to check $schema presence in project.json files

echo "=== Finding all project.json files in the repository ==="
fd "project\.json" | head -50

echo -e "\n=== Checking $schema presence in project.json files ==="
echo "Files WITH \$schema:"
fd "project\.json" -x grep -l '\$schema' {} \; 2>/dev/null | sort

echo -e "\nFiles WITHOUT \$schema:"
fd "project\.json" -exec sh -c 'grep -q "\$schema" "$1" || echo "$1"' _ {} \; 2>/dev/null | sort

echo -e "\n=== Summary count ==="
echo "Total project.json files:"
fd "project\.json" | wc -l

echo "With \$schema:"
fd "project\.json" -x grep -l '\$schema' {} \; 2>/dev/null | wc -l

echo "Without \$schema:"
fd "project\.json" -exec sh -c 'grep -q "\$schema" "$1" || echo "$1"' _ {} \; 2>/dev/null | wc -l

Repository: storybookjs/storybook

Length of output: 2274


🏁 Script executed:

#!/bin/bash
# Let's directly check the content of several project.json files

echo "=== Sample project.json files content ==="
echo "--- code/core/project.json (first 10 lines) ---"
head -10 "code/core/project.json"

echo -e "\n--- code/frameworks/react-vite/project.json (first 10 lines) ---"
head -10 "code/frameworks/react-vite/project.json"

echo -e "\n--- code/addons/docs/project.json (first 10 lines) ---"
head -10 "code/addons/docs/project.json"

echo -e "\n=== Using jq to check for \$schema field ==="
echo "Checking which files have \$schema field using jq:"
fd "project\.json" -x sh -c 'if jq ".[\"\$schema\"]" "$1" > /dev/null 2>&1; then echo "$1: HAS \$schema"; else echo "$1: NO \$schema"; fi' _ {} \; | head -30

echo -e "\n=== Count with jq ==="
echo "Files with \$schema:"
fd "project\.json" -x sh -c 'jq ".[\"\$schema\"]" "$1" > /dev/null 2>&1 && echo "$1"' _ {} \; 2>/dev/null | wc -l

echo "Files without \$schema:"
fd "project\.json" -x sh -c 'jq ".[\"\$schema\"]" "$1" > /dev/null 2>&1 || echo "$1"' _ {} \; 2>/dev/null | wc -l

Repository: storybookjs/storybook

Length of output: 2774


🏁 Script executed:

#!/bin/bash
# Let's see the complete structure of a project.json file with $schema
echo "=== Complete project.json from code/core/project.json ==="
cat "code/core/project.json"

echo -e "\n=== Check the $schema value in a few files ==="
jq '."$schema"' "code/core/project.json"
jq '."$schema"' "code/frameworks/react-vite/project.json" 2>/dev/null || echo "Not found"

echo -e "\n=== Verify the file under review one more time ==="
cat "code/frameworks/react-native-web-vite/project.json"

echo -e "\n=== Check if there are any comments or documentation about schema in NX config ==="
find . -maxdepth 2 -name "nx.json" -o -name ".nxrc*" -o -name "project.json" | head -5
head -20 nx.json 2>/dev/null || echo "No root nx.json found"

Repository: storybookjs/storybook

Length of output: 1098


🏁 Script executed:

#!/bin/bash
# Let's verify the actual pattern more thoroughly
echo "=== Checking ALL project.json files for $schema (complete check) ==="
fd "project\.json" -x sh -c 'echo "=== $1 ==="; jq "has(\"\$schema\")" "$1"' _ {} \; | head -100

echo -e "\n=== Double-check: list first 15 project.json files and their complete content ==="
fd "project\.json" | head -5 | while read f; do
  echo "--- $f ---"
  cat "$f"
  echo ""
done

Repository: storybookjs/storybook

Length of output: 4322


🏁 Script executed:

#!/bin/bash
# Let's verify this is the intended NX pattern by checking NX documentation references
echo "=== Searching for any NX schema references or documentation in the repo ==="
rg -n "schema.*project\.json" -i | head -20

echo -e "\n=== Check if there are any comments about project.json structure ==="
rg -n "project\.json" --type-add 'comments:^.*(//|#|\*|<!--)' | head -30

echo -e "\n=== Look for any migration guides or documentation ==="
fd "README\|MIGRATION\|CHANGELOG" -i | head -10

Repository: storybookjs/storybook

Length of output: 1689


🌐 Web query:

What is the standard NX monorepo pattern for $schema in project.json files

💡 Result:

Short answer: use a relative path from the project.json location to the workspace node_modules schema, e.g.

  • libs (typical): "$schema": "../../node_modules/nx/schemas/project-schema.json"
  • apps or deeper folders: adjust the number of ../ segments so the path reaches /node_modules/nx/schemas/project-schema.json

This is the pattern Nx generators use (they set the correct relative path automatically); you generally should keep the generated $schema value. [1][2][3]

Sources:

  • Nx docs / recipe examples showing "$schema": "../../node_modules/nx/schemas/project-schema.json". [1]
  • Discussion/examples noting the relative-path $schema entries and IDE behaviour. [2]
  • Real project examples (project.json files) using the same relative path pattern. [3]

🏁 Script executed:

#!/bin/bash
# Check if there's documentation about why $schema is not used in this monorepo
echo "=== Search for NX configuration or comments about schema ==="
rg "\$schema" -i --type json | head -20

echo -e "\n=== Check for any Nx configuration documentation ==="
find . -maxdepth 2 -name "*.md" | xargs grep -l "project.json\|nx.json" 2>/dev/null | head -5

echo -e "\n=== Check the root package.json for NX version ==="
jq '.devDependencies | {nx, "@nx/core", "@nx/workspace"}' package.json 2>/dev/null || echo "Checking dependencies..."
grep -A 2 -B 2 '"nx"' package.json | head -20

Repository: storybookjs/storybook

Length of output: 723


Based on my verification, I have found critical issues with the review comment's factual accuracy:

The review comment's core claim about inconsistency is incorrect. I verified that:

  1. ALL 97 project.json files in the monorepo are missing $schema — not just the file under review. This is a monorepo-wide pattern, not an inconsistency.

  2. Web search confirmed that NX best practices recommend including $schema in each project.json with a relative path like "../../node_modules/nx/schemas/project-schema.json".

  3. The monorepo uses NX ^22.0.4, a recent version where schema support is fully functional, with no documented reason for this deviation.

While the review comment correctly identifies that $schema should be present (supporting IDE validation and auto-completion), the verification claim about inconsistency across the monorepo is factually wrong — the file is perfectly consistent with all other project.json files in the repository.


Restore the $schema field for proper NX project validation and IDE support.

This monorepo omits $schema from all project.json files, deviating from NX best practices. The recommended pattern is "$schema": "../../node_modules/nx/schemas/project-schema.json" (adjusting the path depth as needed). This enables IDE auto-completion, type hints, and schema validation for NX project configuration. Since all project.json files lack this field, consider adding it across the monorepo for consistency with NX conventions and to unlock tooling benefits.

🤖 Prompt for AI Agents
In code/frameworks/react-native-web-vite/project.json lines 1-1 the project.json
is missing the NX $schema field (this is a monorepo-wide omission across 97
project.json files); add the $schema entry at the top of this file using the NX
schema relative path (e.g. "../../node_modules/nx/schemas/project-schema.json",
adjusting the ../ depth for this file), and apply the same change consistently
across all project.json files in the repository so IDE validation,
auto-completion and NX schema support are enabled.

Comment on lines +1 to +25
{
"name": "solid-vite/default-js",
"projectType": "application",
"implicitDependencies": [
"core",
"cli",
"addon-a11y",
"addon-docs",
"addon-vitest",
"addon-onboarding"
],
"targets": {
"sandbox": { "options": { "dir": "solid-vite-default-js" } },
"prepare-sandbox": {},
"dev": {},
"build": { "options": { "dir": "solid-vite-default-js" } },
"prepare-build-sandbox": {},
"chromatic": {},
"serve": {},
"e2e-tests-dev": {},
"test-runner": {},
"test-runner-dev": {}
},
"tags": []
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's look at the companion file mentioned in the review
echo "=== solid-vite-default-ts/project.json ===" 
fd -t f 'project\.json' 'code/sandbox/solid-vite-default-ts' 2>/dev/null && cat "code/sandbox/solid-vite-default-ts/project.json"

echo ""
echo "=== All sandbox project.json files ===" 
# Find all sandbox project.json files
fd -t f 'project\.json' 'code/sandbox/'

Repository: storybookjs/storybook

Length of output: 186


🏁 Script executed:

# Compare the two companion files side-by-side
diff -u code/sandbox/solid-vite-default-ts/project.json code/sandbox/solid-vite-default-js/project.json

Repository: storybookjs/storybook

Length of output: 1082


🏁 Script executed:

# Check all sandbox project.json files for consistency
find code/sandbox -name 'project.json' -type f -exec sh -c 'echo "=== {} ===" && jq "{description, tags, has_e2e_tests_dev: (.targets | has(\"e2e-tests-dev\"))}" {}' \;

Repository: storybookjs/storybook

Length of output: 10383


Add missing description field and populate tags array; remove e2e-tests-dev target to match companion configuration.

The verification confirms all three inconsistencies. The companion solid-vite-default-ts/project.json has:

  • "description": "Do not edit. Auto generated by scripts/create-nx-sandbox-projects.ts."
  • "tags": ["ci:merged", "ci:daily"]
  • No e2e-tests-dev target

This file lacks both the description and tags, and incorrectly includes e2e-tests-dev. Scanning all 43 sandbox projects shows solid-vite-default-js is the only file missing the description field—all others include the auto-generation notice. The presence of e2e-tests-dev differs between the two files: the TS version (properly auto-generated) excludes it, while the JS version includes it.

Required changes:

  1. Add "description": "Do not edit. Auto generated by scripts/create-nx-sandbox-projects.ts." at line 3 (after "name")
  2. Change "tags": [] to "tags": ["ci:merged", "ci:daily"] at line 24
  3. Remove "e2e-tests-dev": {} from targets (line 20)
🤖 Prompt for AI Agents
In code/sandbox/solid-vite-default-js/project.json around lines 1 to 25, add the
missing description field and update tags to match the companion TS file, and
remove the extraneous target: insert "description": "Do not edit. Auto generated
by scripts/create-nx-sandbox-projects.ts." immediately after the "name"
property, replace "tags": [] with "tags": ["ci:merged", "ci:daily"], and remove
the "e2e-tests-dev": {} entry from the "targets" object so the targets match the
companion configuration.

@storybook-app-bot
Copy link

storybook-app-bot bot commented Nov 28, 2025

Package Benchmarks

Commit: c289d52, ran on 28 November 2025 at 15:17:20 UTC

No significant changes detected, all good. 👏

@kasperpeulen kasperpeulen added ci:merged Run the CI jobs that normally run when merged. and removed ci:normal labels Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:merged Run the CI jobs that normally run when merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants