Fix bin/setup failing on pnpm workspace member directories

[justin808]

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
  • Improved setup process to conditionally handle JavaScript dependency installation based on lockfile presence, providing clearer messaging when dependencies are managed at the workspace root level.

[coderabbitai]

Walkthrough

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

Changes

Cohort / File(s)	Summary
Setup Script Logic bin/setup	Modified JavaScript dependency installation to check for both package.json and pnpm-lock.yaml before running pnpm install; skips installation and displays informational message when lockfile is missing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A setup script now thinks twice,
Before installing, checking ice—
If lockfile's gone, it gently skips,
Letting workspace take the whips,
With messages to guide your trip! 📦

🚥 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 accurately describes the main change: fixing bin/setup to handle pnpm workspace member directories that lack a lockfile.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

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]

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.

[claude]

Minor: print_success (green checkmark) typically signals a completed action. For a skipped step it could give a false sense that installation was verified. Consider using a plain echo with an informational prefix instead:

Suggested change

	print_success "JavaScript dependencies managed by workspace root"
	echo " JavaScript dependencies managed by workspace root (skipping local install)"

[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.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Silent skip if a lockfile is accidentally missing in a non-workspace directory.

The heuristic equates "no pnpm-lock.yaml" with "workspace member", which is valid today. However, if a directory that should own a lockfile (e.g., root or react_on_rails_pro) accidentally loses it, the script will silently print "JavaScript dependencies managed by workspace root" and proceed without installing — potentially causing confusing downstream failures instead of a clear error here.

If the set of workspace-managed directories is stable and well-known, consider tightening the guard to an explicit allowlist rather than pure lockfile detection:

💡 Proposed alternative: explicit skip list

-  # Skip pnpm install for workspace members (no lockfile) — they're covered by root install
-  if [ -f "package.json" ] && [ -f "pnpm-lock.yaml" ]; then
+  # Workspace members (no pnpm-lock.yaml) are covered by the root install.
+  # If pnpm-lock.yaml is missing but this is NOT a known workspace member, warn loudly.
+  if [ -f "package.json" ] && [ -f "pnpm-lock.yaml" ]; then
     echo "  Installing JavaScript dependencies..."
     if ! pnpm install --frozen-lockfile; then
       print_error "Failed to install JavaScript dependencies in $name"
       exit 1
     fi
     print_success "JavaScript dependencies installed"
   elif [ -f "package.json" ]; then
-    print_success "JavaScript dependencies managed by workspace root"
+    print_warning "No pnpm-lock.yaml found in $name — skipping pnpm install (assuming workspace member)"
   fi

At minimum, changing print_success to print_warning here makes the skip visible enough to catch the accidental case during local setup.

🤖 Prompt for AI Agents

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

In `@bin/setup` around lines 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.