Fix bin/setup failing on pnpm workspace member directories by justin808 · Pull Request #2477 · shakacode/react_on_rails

justin808 · 2026-02-23T00:22:10Z

Summary

bin/setup was failing on workspace member directories (e.g., spec/dummy) because it ran pnpm install --frozen-lockfile in directories without a pnpm-lock.yaml
Workspace members' dependencies are already installed by the root pnpm install, so they don't need their own install step
Now checks for the lockfile before running pnpm install, and prints a message that dependencies are managed by the workspace root

Test plan

Ran bin/setup --skip-pro locally — completes successfully
CI passes

🤖 Generated with Claude Code

Note

Low Risk
Small, localized change to setup scripting that only relaxes a directory-level precondition for running pnpm install.

Overview
Fixes bin/setup failures in pnpm workspace member directories by only running pnpm install --frozen-lockfile when a local pnpm-lock.yaml exists.

If a directory has a package.json but no lockfile (typical for workspace members like dummy apps), the script now skips the install step and reports that JS dependencies are managed by the workspace root.

^{Written by Cursor Bugbot for commit e3d38d3. Configure here.}

Summary by CodeRabbit

Chores
- Setup now installs JavaScript dependencies only when both a package manifest and a matching lockfile are present. If a manifest exists without a lockfile, installation is skipped and a clear notice indicates dependencies are managed at the workspace root, avoiding unnecessary local installs and improving setup clarity.

Workspace members like spec/dummy don't have their own pnpm-lock.yaml — their dependencies are managed by the root pnpm install. Skip running pnpm install --frozen-lockfile in directories without a lockfile. Co-Authored-By: Claude Opus 4.6 <[email protected]>

coderabbitai · 2026-02-23T00:22:28Z

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: a9a49d8a-3baa-4472-a955-439a1a7f2927

📥 Commits

Reviewing files that changed from the base of the PR and between 8b065f3 and 3f6032b.

📒 Files selected for processing (1)

bin/setup

Walkthrough

The setup script now runs pnpm install --frozen-lockfile only when both package.json and pnpm-lock.yaml exist. If the lockfile is missing, installation is skipped and a message indicates dependencies are managed by the workspace root.

Changes

Cohort / File(s)	Summary
Setup Script Logic `bin/setup`	Require both `package.json` and `pnpm-lock.yaml` before running `pnpm install --frozen-lockfile`; if lockfile is absent, skip local install and print an informational notice that dependencies are managed at the workspace root.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I checked the files before my run,
One lock, two files — then work's begun,
If lockfile hides, I skip the stride,
Workspace holds the npm tide,
Hopping on, I grin — all done!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing bin/setup to handle pnpm workspace member directories correctly.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch jg/fix-setup-pnpm-workspace-members

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 and usage tips.}

greptile-apps · 2026-02-23T00:23:52Z

Greptile Summary

Fixed bin/setup script failing when run on pnpm workspace member directories like spec/dummy. The script now only runs pnpm install --frozen-lockfile when a local pnpm-lock.yaml exists, and prints an informative message when dependencies are managed by the workspace root.

Modified the condition on line 160 to check for both package.json AND pnpm-lock.yaml
Added elif branch to print success message when package.json exists but lockfile doesn't
This correctly handles workspace members that don't have their own lockfiles since their dependencies are installed by the root workspace

The change is minimal, well-targeted, and aligns with pnpm workspace conventions where member packages don't maintain their own lockfiles.

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The change is a simple, focused fix that adds a single condition check before running pnpm install. The logic correctly handles pnpm workspace conventions, and the change has been tested locally. The fix prevents a known failure scenario without introducing new complexity or side effects.
No files require special attention

Important Files Changed

Filename	Overview
bin/setup	Added lockfile check before running `pnpm install` to skip workspace members; prints informative message instead

_{Last reviewed commit: e3d38d3}

claude · 2026-02-23T00:24:21Z

Review

The fix is correct and minimal. Workspace members (spec/dummy directories) are registered in the root pnpm-workspace.yaml and don't carry their own pnpm-lock.yaml, so running pnpm install --frozen-lockfile in them fails. This PR correctly skips that step for such directories.

A few observations:

1. Implicit assumption about what a missing lockfile means

The heuristic is: "if package.json exists but pnpm-lock.yaml doesn't, it must be a workspace member." That's true for the current directory list, but it's a negative-space check — if someone adds a standalone package.json directory (e.g., a new tool or example) without a lockfile, the script will silently print success and skip installation instead of surfacing the issue.

A more explicit alternative would be checking for pnpm-workspace.yaml in a parent directory, since that's the actual pnpm contract for workspace membership. The dummy apps already have their own pnpm-workspace.yaml (e.g. react_on_rails/spec/dummy/pnpm-workspace.yaml), so checking for it as a secondary signal could make intent clearer.

2. print_success for a skipped step may mislead

print_success (green checkmark) conventionally signals that something was done and verified. Here it marks a step that was deliberately skipped. A plain echo would be less likely to give users the impression that installation was actually confirmed. This is a minor UX nit, not a functional issue.

3. Ordering in main() protects against the skip

The script always calls install_dependencies "$ROOT_DIR" first (line 246), which runs the workspace-root pnpm install. The subsequent sub-directory calls then legitimately skip their own install. The ordering enforces correctness, so the skip is safe in the current call sequence.

Summary: Functionally correct. The lockfile heuristic works for the current directory set. Points 1 and 2 are worth noting for robustness as the repo evolves, but neither is a blocker.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bin/setup`:
- Around line 160-168: The current branch that detects workspace-managed
packages (elif [ -f "package.json" ]) silently prints success when
pnpm-lock.yaml is missing; change that to make skips visible and optionally
tighten detection: in the elif block replace print_success with print_warning
(so missing lockfile is surfaced), and if you want a stricter fix add an
explicit allowlist (e.g. WORKSPACE_ONLY_DIRS or WORKSPACE_MANAGED_DIRS) and
check membership before skipping the install instead of relying solely on
pnpm-lock.yaml; keep the pnpm install path (pnpm install --frozen-lockfile and
print_error on failure) unchanged.

justin808 · 2026-03-12T00:16:31Z

Addressed both open review points in bin/setup:

Changed the skip message from success to warning.
Clarified the skip condition as an assumption (no pnpm-lock.yaml => managed by workspace root).

This keeps behavior the same while avoiding false "success" signaling and making the heuristic explicit.

claude · 2026-03-12T00:18:12Z

The fix is correct and well-scoped. In a pnpm workspace, only the root directory has a pnpm-lock.yaml; member packages like spec/dummy are covered by the root install. Running pnpm install --frozen-lockfile inside a member directory would always fail because there is no lockfile to freeze against. This change properly guards against that. One minor note (see inline): the print_warning call on the skip path uses a Warning prefix and the hedge word assuming, which could confuse contributors into thinking something went wrong. Since skipping is the correct behavior for workspace members, a neutral informational echo message would be clearer. Everything else looks good: the AND condition on line 160 is the right approach, the root directory setup is unaffected since it has pnpm-lock.yaml, and the change is minimal without introducing new control-flow complexity.

justin808 · 2026-03-12T00:32:32Z

Updated as requested: the workspace-member case now uses plain informational output instead of warning semantics.

claude · 2026-03-12T00:34:50Z

Review

The fix is correct and well-scoped. Workspace members like spec/dummy use pnpm's workspace protocol (workspace:*) so their dependencies are resolved and installed by the root pnpm install — they never have their own pnpm-lock.yaml, which is exactly what this check tests.

A few observations:

Heuristic is appropriate — using pnpm-lock.yaml presence as a proxy for "is this a workspace root?" is idiomatic and reliable for pnpm projects. The root directory always has a lockfile; workspace members never do.

Minor: silent skip for unexpected missing lockfiles — If a directory has package.json but is not a workspace member and also lacks a lockfile (e.g., someone added a new package directory but forgot to run pnpm install), the script now silently skips the install and prints the workspace-managed message. For bin/setup, this is probably fine since that scenario shouldn't occur in this monorepo, but it's worth keeping in mind if new non-workspace package directories are added in the future.

Message style is consistent — the new echo message on line 168 matches the pattern used for other in-progress messages in install_dependencies (plain echo, not print_success/print_warning). ✓

Overall: clean, minimal fix. LGTM.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

…k cleanup (#2596) ## Summary - Added missing changelog entries for PRs #2439, #2477, #2554 - Stamped version header for 16.4.0.rc.9 (collapsed rc.8 into rc.9) - **Fixed rake task bug**: `collapse_prerelease_sections` removed section headers but left behind orphaned compare links at the bottom of CHANGELOG.md. Added `cleanup_collapsed_prerelease_links` to remove these links and update `[unreleased]` to compare from the last stable version. ## Changes 1. **CHANGELOG.md**: New entries + rc.9 stamp + removed orphaned `[16.4.0.rc.8]` link 2. **update_changelog.rake**: Added `cleanup_collapsed_prerelease_links` function, called from `prepare_changelog_for_auto_version` 3. **update_changelog_rake_helpers_spec.rb**: 3 new tests covering link cleanup (multi-prerelease chain, single prerelease, no prereleases) ## Skipped PRs (not user-visible) - #2593 — test only (lock instrumentation) - #2591 — test only (Jest clearMocks) - #2588 — internal refactoring (Thor shell output) - #2584 — docs restructuring - #2587 — internal tooling (release script) - #2589 — docs fix (JWT claim name) - #2586 — docs/CI (dead link removal) ## Test plan - [x] All 13 rake helper tests pass (3 new) - [ ] CI passes - [ ] After merge, run `rake release` to publish 16.4.0.rc.9 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Bug Fixes** * Fixed bin/setup failing in pnpm workspace member directories. * **New Features** * Added host configuration option for the Node Renderer Fastify worker. * **Improvements / Changed** * Automatic installation of react_on_rails_pro enabled for --rsc/--pro flags. * Clarified Pro-installation signaling related to immediate hydration warnings. * Updated changelog to include RC9 entries and ensure Unreleased links point to the correct base. * **Documentation** * Added startup warning and remediation guidance for unsafe compression middleware callbacks. * **Tests** * Added tests covering changelog prerelease-link cleanup behavior.  --------- Co-authored-by: Claude Opus 4.6 <[email protected]>

claude Bot reviewed Feb 23, 2026

View reviewed changes

Comment thread bin/setup Outdated

coderabbitai Bot requested changes Feb 23, 2026

View reviewed changes

Comment thread bin/setup Outdated

justin808 added the release:16.4.0-must-have Must-have for 16.4.0: critical bug/perf/usability label Feb 25, 2026

justin808 self-assigned this Mar 5, 2026

justin808 removed the release:16.4.0-must-have Must-have for 16.4.0: critical bug/perf/usability label Mar 6, 2026

justin808 mentioned this pull request Mar 8, 2026

Release 16.4.0 Triage: PRs, Issues, and Prioritization #2560

Closed

justin808 added bug P2 Backlog priority labels Mar 8, 2026

chore: clarify workspace-managed pnpm install skip

8b065f3

claude Bot reviewed Mar 12, 2026

View reviewed changes

Comment thread bin/setup Outdated

coderabbitai Bot approved these changes Mar 12, 2026

View reviewed changes

chore: use info output for workspace-managed js deps

3f6032b

justin808 merged commit 151d1d5 into master Mar 13, 2026
33 checks passed

justin808 deleted the jg/fix-setup-pnpm-workspace-members branch March 13, 2026 02:54

justin808 added a commit that referenced this pull request Mar 13, 2026

docs: add missing changelog entries for PRs #2439, #2477, #2554

cae4d7b

Co-Authored-By: Claude Opus 4.6 <[email protected]>

justin808 mentioned this pull request Mar 13, 2026

fix: update CHANGELOG for 16.4.0.rc.9 and fix orphaned prerelease link cleanup #2596

Merged

3 tasks

justin808 mentioned this pull request Mar 18, 2026

Fix bin/setup failing on pnpm workspace member directories #2722

Closed

This was referenced Mar 28, 2026

Revert "Eliminate double JSON.stringify in RSC payload embedding (#2835)" #2878

Merged

chore: security audit cleanup — CodeQL, workflow permissions, dependency overrides #2884

Merged

justin808 mentioned this pull request Apr 4, 2026

Fix bin/setup failing on pnpm workspace member directories #2976

Closed

This was referenced Apr 18, 2026

docs: address RSC pitfalls review follow-ups (#3155) #3156

Merged

refactor: unify renderer cache staging with mode: :copy / :symlink #3167

Merged

Fix Bencher reporting permanently broken on pushes to main #3148

Merged

This was referenced May 4, 2026

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management #3142

Open

feat: rolling_deploy_adapter for seeding previous bundle hashes #3173

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix bin/setup failing on pnpm workspace member directories#2477

Fix bin/setup failing on pnpm workspace member directories#2477
justin808 merged 3 commits intomasterfrom
jg/fix-setup-pnpm-workspace-members

justin808 commented Feb 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 23, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Feb 23, 2026

Uh oh!

claude Bot commented Feb 23, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

justin808 commented Mar 12, 2026

Uh oh!

Uh oh!

claude Bot commented Mar 12, 2026

Uh oh!

justin808 commented Mar 12, 2026

Uh oh!

claude Bot commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

justin808 commented Feb 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

greptile-apps Bot commented Feb 23, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

claude Bot commented Feb 23, 2026

Review

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

justin808 commented Mar 12, 2026

Uh oh!

Uh oh!

claude Bot commented Mar 12, 2026

Uh oh!

justin808 commented Mar 12, 2026

Uh oh!

claude Bot commented Mar 12, 2026

Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

justin808 commented Feb 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 23, 2026 •

edited

Loading