Skip to content

Build: Persist root node_modules to the CI workspace#34886

Merged
Sidnioulz merged 3 commits into
nextfrom
valentin/ci-persist-root-node-modules
May 22, 2026
Merged

Build: Persist root node_modules to the CI workspace#34886
Sidnioulz merged 3 commits into
nextfrom
valentin/ci-persist-root-node-modules

Conversation

@valentinpalkovic
Copy link
Copy Markdown
Contributor

@valentinpalkovic valentinpalkovic commented May 22, 2026

What I'm changing

The CircleCI Build (linux) job persists per-package code/<pkg>/dist and code/<pkg>/node_modules to the pipeline workspace, but the workspace-root node_modules (plus code/node_modules and scripts/node_modules) reaches downstream jobs only through the shared save_cache — which is gated on isTrustedAuthor().

Yarn's node-modules linker hoists shared/singleton dependencies — oxc-parser, vitest, type-fest, … — into the root node_modules, not the per-package folders. So for community/fork PRs (untrusted authors) build_linux never writes that cache, and when restore_cache finds nothing the downstream jobs run with a freshly-built dist but no root node_modules.

That produces failures unrelated to the PR's diff, e.g.:

  • Cannot find package 'oxc-parser' — every sandbox *--create job
  • command not found: vitesttests--linux / tests-stories--linux
  • type-fest has no exported member 'OmitIndexSignature'typescript-validation

CI is currently red across community PRs (e.g. #34534, #34883) for exactly this reason.

How I'm changing it

  • build_linux now also persists node_modules, code/node_modules and scripts/node_modules to the pipeline workspace. persist_to_workspace is pipeline-scoped and not trust-gated, so every PR — community/fork PRs included — receives the exact node_modules the build produced.
  • workflow.restoreLinux now attaches the workspace after restoring the shared cache, so the freshly-built output is authoritative and a stale cache entry can't shadow it.

The shared save_cache/restore_cache is left in place as a (now largely redundant) optimization; removing it could be a follow-up.

Manual testing

  • Regenerated the CircleCI config locally (scripts/ci/main.ts --workflow=normal --gh-trusted-author=false) and confirmed the Build (linux) job's persist_to_workspace now lists project/node_modules, project/code/node_modules and project/scripts/node_modules, and that downstream jobs restore_cache before attach_workspace.
  • scripts typecheck (yarn check) and oxfmt --check pass on the changed files.
  • The full proof is CI on this PR (downstream jobs exercise the new persist/attach path) and re-running a community PR such as CLI: Upgrade jscodeshift to 17.3.0 #34534 afterwards.

Unblocks #34534.

Summary by CodeRabbit

  • Chores
    • Optimized CI/CD pipeline caching and workspace persistence to improve build reliability and ensure consistent dependency availability across all pull request types.

Review Change Stack

CircleCI downstream jobs received the workspace-root node_modules --
where Yarn hoists shared/singleton deps like oxc-parser, vitest and
type-fest -- only via the shared save_cache, which is gated on
isTrustedAuthor(). Community/fork PRs therefore ran with freshly-built
dist output but no root node_modules, causing failures unrelated to
their diff (e.g. "Cannot find package 'oxc-parser'", "command not
found: vitest", stale type-fest typecheck errors).

Persist node_modules, code/node_modules and scripts/node_modules to the
pipeline-scoped workspace in build_linux, and attach the workspace after
restoring the cache so the fresh build output is authoritative.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@valentinpalkovic valentinpalkovic added build Internal-facing build tooling & test updates ci:normal labels May 22, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 910258f1-25ac-4571-a4c2-93b4b123a9bf

📥 Commits

Reviewing files that changed from the base of the PR and between af1cdb0 and 9a53ebe.

📒 Files selected for processing (3)
  • scripts/ci/common-jobs.ts
  • scripts/ci/sandboxes.ts
  • scripts/ci/utils/helpers.ts
💤 Files with no reviewable changes (1)
  • scripts/ci/sandboxes.ts

📝 Walkthrough

Walkthrough

The PR coordinates CI cache handling by reordering when shared caches are attached relative to workspace restoration, implementing that pattern in the Linux build job with expanded node_modules persistence for downstream jobs, and removing redundant cache logic from the chromatic job.

Changes

CI Cache Ordering and Workspace Updates

Layer / File(s) Summary
Restore step ordering in CI helper
scripts/ci/utils/helpers.ts
workflow.restoreLinux reorders steps to attach shared cache before workspace attachment, with comments documenting that downstream jobs should consume pipeline workspace outputs to avoid stale cache interference.
Linux build job cache attachment and persistence
scripts/ci/common-jobs.ts
Linux build_linux job adds cache.attach(CACHE_KEYS()) before installation and extends workspace.persist to include workspace-root node_modules (shared/hoisted root and per-package directories) so dependencies propagate to downstream jobs.
Chromatic job cache cleanup
scripts/ci/sandboxes.ts
Chromatic job removes cache.attach(CACHE_KEYS()) call and cleans up associated unused imports (CACHE_KEYS, cache) from ./utils/helpers.ts.

Possibly Related PRs

  • storybookjs/storybook#34853: Directly modifies scripts/ci/common-jobs.ts cache behavior in the Linux build_linux job with related workspace dependency handling.

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@Sidnioulz
Copy link
Copy Markdown
Member

@copilot I have the following requirements for data flows in CI:

  • all jobs that need to fetch pre-computed data, computed earlier in the pipeline, exclusively use the workspace persistence which is designed for exactly this
  • jobs that refresh/compute data should
    • read from a cache to save time initially
    • perform refresh to update what's missing in cache
    • write back to cache for other future pipelines, if legally allowed
    • always persist to workspace for future jobs to have access to the data

Your task is:

  1. Validate my requirements; if there are security flaws in my reasoning, shout at me and correct course
  2. Look at the data flows in our CircleCI pipeline and tell me if the PR makes those flows respect the elicited requirements
  3. If we fall short of the requirements, make necessary adjustments

Copilot AI and others added 2 commits May 22, 2026 14:19
…f needed

Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/261ad771-2035-454b-b268-561ccbdd8c2c

Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/261ad771-2035-454b-b268-561ccbdd8c2c

Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

@copilot I have the following requirements for data flows in CI:

  • all jobs that need to fetch pre-computed data, computed earlier in the pipeline, exclusively use the workspace persistence which is ...

I validated your requirements and they’re directionally correct. Security nuance: workspace data is pipeline-scoped (good for same-pipeline handoff), while shared cache is cross-pipeline and should stay trust-gated for writes; downstream PR jobs should not rely on that cache for correctness.

I checked the flow and found gaps: downstream Linux jobs still restored shared cache via workflow.restoreLinux(), and sandbox chromatic also restored cache after workspace attach.

Fixed in 9a53ebe:

  • build_linux now restores cache first (cache.attach) before refresh/compute.
  • Downstream workflow.restoreLinux() now uses workspace only (removed cache.attach).
  • Removed sandbox chromatic cache.attach so precomputed inputs come from workspace only.

Validation after changes passed:

  • yarn --cwd scripts check
  • yarn --cwd scripts lint:js:cmd ci/common-jobs.ts ci/utils/helpers.ts ci/sandboxes.ts --fix
  • yarn --cwd scripts test
  • parallel validation (Code Review + CodeQL)

Copilot AI requested a review from Sidnioulz May 22, 2026 14:26
@Sidnioulz Sidnioulz merged commit 1411339 into next May 22, 2026
136 of 138 checks passed
@Sidnioulz Sidnioulz deleted the valentin/ci-persist-root-node-modules branch May 22, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants