Skip to content

CLI: Restore pnpm minimumReleaseAge prechecks - #36352

Merged
valentinpalkovic merged 3 commits into
storybookjs:nextfrom
ia319:bug/36351-pnpm-release-age-precheck
Oct 12, 2026
Merged

valentinpalkovic merged 3 commits into
storybookjs:nextfrom
ia319:bug/36351-pnpm-release-age-precheck

Conversation

@ia319

@ia319 ia319 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Closes #36351

What I did

  • Query the full time map and select the requested version locally.
  • Update parsePackageTimeMap to accept a map wrapped in a single-element array.

Reasons

Its native field lookup does not support time[version], returning empty output and causing Storybook to skip the release-age precheck.

  • npm 12.0.0 changed view --json to always return an array.

The parser needs to handle this format, including queries forwarded by pnpm < 11.

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

Manual testing

  1. With pnpm 12.4.2, create a React + Vite project and install its dependencies before configuring the waiting period:

    pnpm create vite release-age-repro --template react-ts
    cd release-age-repro
    pnpm install
  2. Configure a waiting period longer than the target Storybook release’s age:

    echo 'minimumReleaseAge: 23040' > pnpm-workspace.yaml

    23040 means 16 days. Increase it if the tested release is older.

  3. Run the CLI built from this branch:

    node /path/to/storybook/code/lib/create-storybook/dist/bin/index.js

    Confirm the release-age prompt appears before project files are modified. Choose “Stop now and rerun…” and confirm it prints a command for an allowed older release.

  4. Repeat in a fresh project with pnpm < 11 that supports minimumReleaseAge, alongside npm 12.0.0, to verify compatibility with npm’s array response.

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

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • 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 - 🚫 Not run

This PR does not have a canary release associated.

In-repo PRs: add the ci:canary label. Later pushes republish while the label remains.

Fork PRs: the label does nothing (a later push must not auto-publish). A maintainer publishes from this repository with Run workflow and the pr input. branch and sha are optional; if more than one is set, they must be the same commit. The fork author does not need to do anything.

@ia319 ia319 added bug cli ci:normal Run our default set of CI jobs (choose this for most PRs). labels Sep 17, 2026
@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3b8ae158-9a7a-4dc1-9bab-35189aa16f6c
📥 Commits

Reviewing files that changed from the base of the PR and between fd09159 and 13ee7dd.

📒 Files selected for processing (2)
  • code/core/src/common/js-package-manager/PNPMProxy.test.ts
  • code/core/src/common/js-package-manager/PNPMProxy.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The change supports pnpm 12 release-time responses wrapped in a single-element array. PNPM prechecks now look up versions from the complete release-time map. Tests cover recent, old, and missing release times.

Changes

PNPM release-time handling

Layer / File(s) Summary
Release-time map parsing
code/core/src/common/js-package-manager/util.ts, code/core/src/common/js-package-manager/util.test.ts
parsePackageTimeMap accepts a single-element array containing a release-time map. Tests verify valid fields and reject unsupported array shapes and invalid values.
Release-time precheck integration
code/core/src/common/js-package-manager/PNPMProxy.ts, code/core/src/common/js-package-manager/PNPMProxy.test.ts
getPackageReleaseTime parses the complete pnpm release-time map and looks up the requested version. Tests cover prompts and MinimumReleaseAgeHandledError for recent releases, plus no prompt or side effects for old and missing release times.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 13ee7

The change restores the minimumReleaseAge precheck for pnpm and accepts array-wrapped responses. No actionable merge-blocking risk remains.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

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

@storybook-app-bot

storybook-app-bot Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Package Benchmarks

Commit: 13ee7dd, ran on 6 October 2026 at 18:59:58 UTC

No significant changes detected, all good. 👏

@storybook-contribution-review storybook-contribution-review Bot added pnpm sev:S3 Medium priority. Fix within months if possible. review:1-architecture In automated review pipeline: awaiting stage 1 architecture review labels Sep 18, 2026

@storybook-contribution-review storybook-contribution-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 I am the Storybook contribution review bot; a human maintainer approved this message before it was posted.
Review phase: Final verdict - architecture, thermo-nuclear code quality, and runtime verification complete

We are trying out something new: a three-stage automated review - architecture, then code quality, then runtime verification - that runs on a PR before a human maintainer looks at it closely. A maintainer still reads this and still makes the call; the aim is that by the time they do, the reachability and evidence work is already done. Tell us how it lands for you, that feedback is genuinely useful to us right now.

Hi @ia319,

Thank you for restoring pnpm's minimum-release-age check at the shared package-time-map boundary. The full pipeline passes at head fd0915962d94d6619f56ef40c91595e67b5cc676.

The regression tests failed six times on baseline 4f66ceeb2410de4bcefe3ba3919267495cc40f37 and passed all 41 tests at the PR head. With pnpm 11.0.0, the baseline continued without warning for storybook@10.4.0-alpha.17; the head stopped with MinimumReleaseAgeHandledError and printed npx create-storybook@10.3.5.

The head also passed mature and absent releases, updated all four exclusion patterns in non-interactive mode, and printed rerun guidance with pnpm 10.17.1 and npm 12.0.0. Direct npm 12 returned the time map in a one-element array, which the parser test covers.

The run used seeded package-manager fixtures and selected the real prompt through a test override instead of terminal keystrokes. These limits do not block this command-line fix.

Final verdict: approve.

No patch will be applied because the PR already contains the regression tests.

@storybook-contribution-review storybook-contribution-review Bot added review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review review:3-verification Stage 2 passed: awaiting stage 3 proof-of-work verification review:verified Three-step review passed: architecture, code quality, runtime verification and removed review:1-architecture In automated review pipeline: awaiting stage 1 architecture review review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review review:3-verification Stage 2 passed: awaiting stage 3 proof-of-work verification labels Sep 21, 2026
@valentinpalkovic valentinpalkovic self-assigned this Oct 6, 2026
@valentinpalkovic valentinpalkovic added qa:needed Pull Requests that will need manual QA prior to release. ci:normal Run our default set of CI jobs (choose this for most PRs). and removed ci:normal Run our default set of CI jobs (choose this for most PRs). labels Oct 6, 2026
@valentinpalkovic
valentinpalkovic merged commit 823f26f into storybookjs:next Oct 12, 2026
160 of 164 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 12, 2026
2 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). cli pnpm qa:needed Pull Requests that will need manual QA prior to release. review:verified Three-step review passed: architecture, code quality, runtime verification sev:S3 Medium priority. Fix within months if possible.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Missing minimumReleaseAge prompt when initializing Storybook with pnpm 12

2 participants