fix(dev): add workspace validation to prevent module resolution errors #8548
Conversation
- Add pre-flight validation script for workspace packages - Validate dist/ folders exist before starting dev server - Provide clear recovery commands (build:packages, dev:fix, dev:clean) - Update CONTRIBUTING.md with troubleshooting guide Prevents cryptic Vite errors when build artifacts are missing/corrupted after interrupted builds or branch switches.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe changes add workspace validation infrastructure to prevent module resolution errors during development. A new validation script checks artifact-based packages for non-empty dist/ folders and valid entry points. Package scripts are extended with build, clean, and validation commands. Documentation covers troubleshooting and remediation strategies. A critical syntax error exists in turbo.json. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 23-24: The package.json has a JSON syntax error: the "validate"
script value for the "validate" key is missing a trailing comma which breaks
parsing and prevents pnpm commands; fix by adding a comma after the "validate"
script entry so the object entry for "validate" is properly separated from the
next "check:types" script, ensuring the "validate" and "check:types" script keys
remain unchanged.
In `@scripts/validate-workspace.js`:
- Around line 129-138: The filter callback is destructuring a property named
`path`, which shadows the imported `path` module and can cause confusion; update
the callback used to build `missingFiles` (where `expectedFiles.filter(({ path
}) => !fs.existsSync(path))` is currently used) to rename the destructured
variable (for example `filePath` or `expectedPath`) and then use that new name
in the `fs.existsSync(...)` call and in the subsequent `missingFiles.map(...)`
so all references avoid clashing with the imported `path` module.
🧹 Nitpick comments (2)
scripts/validate-workspace.js (2)
17-35: Package lists are clear and maintainable.The explicit separation of artifact-based vs source-resolved packages provides clarity. The lists align with the documentation in CONTRIBUTING.md.
Consider adding a comment noting that these lists should be updated when adding/removing workspace packages.
💡 Optional: Add maintenance hint
// Packages that MUST have dist/ folders (artifact-based resolution) +// NOTE: Update this list when adding/removing workspace packages const ARTIFACT_PACKAGES = [
200-208: The--fixflag doesn't actually fix anything.The
autoFixoption is accepted but only changes the output message rather than performing any actual repair. The comment on line 202 suggests this is intentional ("You can implement auto-rebuild here if desired"), but having a--fixflag that doesn't fix is misleading.The
dev:fixscript in package.json works around this by using|| (pnpm build:packages ...), but users runningpnpm validate --fixmight expect automatic repair.Consider either:
- Implementing actual auto-fix (spawning
pnpm build:packages)- Removing the
--fixflag until it's implemented- Documenting that
--fixis a placeholder for future functionality💡 Option 1: Implement auto-fix
if (autoFix) { - log('🔧 Auto-fix enabled: Run pnpm build:packages to rebuild', 'cyan'); - log(' (You can implement auto-rebuild here if desired)\n', 'cyan'); + log('🔧 Auto-fix enabled: Rebuilding workspace packages...', 'cyan'); + const { execSync } = require('child_process'); + try { + execSync('pnpm build:packages', { stdio: 'inherit', cwd: WORKSPACE_ROOT }); + log('✓ Workspace packages rebuilt successfully!\n', 'green'); + return true; + } catch (error) { + log('✗ Auto-fix failed. Please run manually: pnpm build:packages\n', 'red'); + return false; + } } else {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CONTRIBUTING.mdpackage.jsonscripts/validate-workspace.jsturbo.json
💤 Files with no reviewable changes (1)
- turbo.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,mjs,cjs}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Use
@importtags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Files:
scripts/validate-workspace.js
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
scripts/validate-workspace.jspackage.json
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
scripts/validate-workspace.js
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use
workspace:*for internal packages andcatalog:for external dependencies in imports
Files:
package.json
🧠 Learnings (10)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to {turbo.json,**/*.sh} : Use Turbo for build system orchestration with configuration in turbo.json
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/package.json : Use `workspace:*` for internal packages and `catalog:` for external dependencies in imports
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to pnpm-workspace.yaml : Use pnpm-workspace.yaml for monorepo workspace configuration
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : All features require unit tests using the existing test framework per package
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/package.json : Use `workspace:*` for internal packages and `catalog:` for external dependencies in imports
Applied to files:
scripts/validate-workspace.jspackage.json
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to pnpm-workspace.yaml : Use pnpm-workspace.yaml for monorepo workspace configuration
Applied to files:
scripts/validate-workspace.js
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Run ESLint checks across all packages with `pnpm check:lint`
Applied to files:
scripts/validate-workspace.js
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Auto-fix formatting and linting issues with `pnpm fix`
Applied to files:
scripts/validate-workspace.js
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/plane/ui/**/*.{ts,tsx} : Build components in `plane/ui` package with Storybook for isolated development
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to {turbo.json,**/*.sh} : Use Turbo for build system orchestration with configuration in turbo.json
Applied to files:
package.json
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Run TypeScript type checking with `pnpm check:types`
Applied to files:
package.json
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,css} : Use Prettier with Tailwind plugin for code formatting, run `pnpm fix:format`
Applied to files:
package.json
⏰ 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). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (6)
CONTRIBUTING.md (1)
245-336: LGTM! Well-structured troubleshooting documentation.The new troubleshooting section is comprehensive and provides clear guidance for developers encountering module resolution errors. The documentation correctly covers:
- Symptom identification with example error messages
- Root cause explanations
- Multiple recovery options with appropriate use cases
- Command reference table for quick lookup
The commands and package lists align with the implementation in
package.jsonandscripts/validate-workspace.js.package.json (1)
10-14: New workspace scripts look good, with one consideration.The script implementations are well-designed:
build:packagescorrectly uses Turbo's filter syntaxdevproperly chains validation before startingdev:cleanfollows logical cleanup → build → dev flowdev:fixprovides graceful fallback with informative messageOne minor note:
clean:packagesuses shell globpackages/*/distwhich works on Unix-like systems but may fail on Windows without WSL.Consider whether Windows compatibility is needed. If so, a cross-platform approach using a Node script or
rimrafwould be more portable.scripts/validate-workspace.js (4)
1-16: LGTM! Well-documented script header.Clear documentation of the script's purpose and exit codes. Using only built-in Node.js modules (
fs,path) keeps this dependency-free as intended.
95-126: Exports parsing covers common patterns adequately.The implementation handles the most common export patterns (string exports, conditional import/require) and falls back to main/module fields. This is sufficient for validating core build artifacts.
Note: Complex exports patterns (nested conditions like
.default,.types, or subpath exports like./utils) are not fully validated, but this shouldn't cause false negatives for the primary validation goal.
218-235: LGTM! CLI execution is well-structured.The CLI block correctly:
- Guards execution with
require.main === module- Parses command-line flags
- Respects
SKIP_VALIDATIONenvironment variable- Exits with appropriate codes (0 for success, 1 for failure)
237-237: LGTM! Exports enable programmatic usage.Exporting both functions allows the validation logic to be reused in other scripts or tests if needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
package.json
Outdated
| "validate": "node scripts/validate-workspace.js --verbose" | ||
| "check:types": "turbo run check:types", |
There was a problem hiding this comment.
Critical: Missing comma causes JSON syntax error.
Line 23 is missing a trailing comma after the validate script definition. This will cause a JSON parse error, breaking all pnpm commands.
🐛 Proposed fix
- "validate": "node scripts/validate-workspace.js --verbose"
+ "validate": "node scripts/validate-workspace.js --verbose",
"check:types": "turbo run check:types",📝 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.
| "validate": "node scripts/validate-workspace.js --verbose" | |
| "check:types": "turbo run check:types", | |
| "validate": "node scripts/validate-workspace.js --verbose", | |
| "check:types": "turbo run check:types", |
🧰 Tools
🪛 Biome (2.1.2)
[error] 24-24: expected , but instead found "check:types"
Remove "check:types"
(parse)
🤖 Prompt for AI Agents
In `@package.json` around lines 23 - 24, The package.json has a JSON syntax error:
the "validate" script value for the "validate" key is missing a trailing comma
which breaks parsing and prevents pnpm commands; fix by adding a comma after the
"validate" script entry so the object entry for "validate" is properly separated
from the next "check:types" script, ensuring the "validate" and "check:types"
script keys remain unchanged.
scripts/validate-workspace.js
Outdated
| const missingFiles = expectedFiles.filter(({ path }) => !fs.existsSync(path)); | ||
|
|
||
| if (missingFiles.length > 0 && missingFiles.length === expectedFiles.length) { | ||
| // All expected files are missing - this is a problem | ||
| return { | ||
| valid: false, | ||
| reason: `Missing all expected artifacts: ${missingFiles.map(f => f.file).join(', ')}`, | ||
| packageName: packageJson.name, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Variable shadowing: path shadows the imported module.
The destructured path variable in the filter callback shadows the path module imported at line 15. While this works due to closure scope, it can cause confusion and potential bugs if the code is modified later.
🐛 Proposed fix: Rename destructured variable
- const missingFiles = expectedFiles.filter(({ path }) => !fs.existsSync(path));
+ const missingFiles = expectedFiles.filter(({ path: filePath }) => !fs.existsSync(filePath));🤖 Prompt for AI Agents
In `@scripts/validate-workspace.js` around lines 129 - 138, The filter callback is
destructuring a property named `path`, which shadows the imported `path` module
and can cause confusion; update the callback used to build `missingFiles` (where
`expectedFiles.filter(({ path }) => !fs.existsSync(path))` is currently used) to
rename the destructured variable (for example `filePath` or `expectedPath`) and
then use that new name in the `fs.existsSync(...)` call and in the subsequent
`missingFiles.map(...)` so all references avoid clashing with the imported
`path` module.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 20
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
package.json
Outdated
| "clean:packages": "rm -rf packages/*/dist", | ||
| "dev": "node scripts/validate-workspace.js && turbo run dev --concurrency=18", | ||
| "dev:clean": "pnpm clean:packages && pnpm build:packages && pnpm dev", | ||
| "dev:fix": "node scripts/validate-workspace.js --fix || (pnpm build:packages && echo '✓ Workspace repaired, starting dev...' && pnpm dev)", |
There was a problem hiding this comment.
dev:fix script never starts dev when validation passes
Medium Severity
The dev:fix script uses || which means the dev server only starts when validation fails. When all packages are valid and validation passes (exit code 0), the || short-circuits and the right side containing pnpm dev never executes. The command silently completes without starting the dev server. The correct logic would be (validate || build) && dev to ensure dev always starts.
|
Thanks for opening PR. |
…e builds - Remove custom validation script (scripts/validate-workspace.js) - Add prepare npm hook to auto-build packages after pnpm install - Update CONTRIBUTING.md to document new workflow - Fix turbo.json missing closing brace - Standardize package.json exports to use .cjs/.mjs extensions - Remove pnpm dev:fix and pnpm validate commands This resolves issue makeplane#8532 where pnpm dev fails after Ctrl+C by using npm's standard prepare lifecycle hook and Turbo's dependency graph for automatic package rebuilding.
|
Thanks for pointing this out. I’ve removed Going forward, the correct workflow is simply: pnpm devIf the process is interrupted, Turbo will automatically rebuild missing packages. For a clean recovery, you can also run: pnpm install
pnpm devThis replaces the old fix command with a more stable, standard setup. |
2 participants
fixes: #8532
Description
This PR adds pre-flight validation to prevent deterministic development environment failures caused by missing or corrupted workspace package build artifacts.
Problem:
Developers frequently encounter cryptic module resolution errors when running
pnpm dev:Root Cause:
The Plane monorepo uses artifact-based package resolution where packages export from
dist/folders. When builds are interrupted (Ctrl+C), git branches are switched, or dependencies are updated,dist/folders can be:Since Turbo runs tasks in parallel with high concurrency, Vite starts before packages finish rebuilding, causing immediate failures with unclear error messages.
Solution:
This PR introduces a smart validation script that:
pnpm devstartsdist/foldersChanges:
scripts/validate-workspace.js(new)--verboseflag for detailed package statusSKIP_VALIDATION=1env variablepackage.json- Added 5 new scripts:validate- Check workspace health with verbose outputbuild:packages- Build only workspace packages (fast, ~30s recovery)clean:packages- Remove only package dist/ folders (surgical cleanup)dev- Now includes automatic pre-validation before startingdev:clean- Full reset: clean + rebuild + devdev:fix- Auto-repair: validate + rebuild if needed + devCONTRIBUTING.md- Added comprehensive troubleshooting section:turbo.json- No functional changes (maintained for consistency)Type of Change
Screenshots and Media (if applicable)
Before (problematic experience):
After (with validation):
New validation command output:
$ pnpm validate 🔍 Validating workspace package artifacts... ✓ @plane/constants ✓ @plane/decorators ✓ @plane/editor ✓ @plane/hooks ✓ @plane/i18n ✓ @plane/logger ✓ @plane/propel ✓ @plane/services ✓ @plane/types ✓ @plane/ui ✓ @plane/utils 📦 Source-resolved packages (no build required): → shared-state ════════════════════════════════════════════════════════════ Valid packages: 11/11 All packages validated successfully! ✨ ════════════════════════════════════════════════════════════Test Scenarios
Scenario 1: All packages have valid build artifacts
$ pnpm validate ✓ All packages validated successfully! ✨ Exit code: 0✅ Passes validation, allows dev to proceed
Scenario 2: Missing dist/ folder
$ rm -rf packages/utils/dist $ pnpm validate ✗ @plane/utils: dist/ folder missing ⚠️ Workspace validation failed! Exit code: 1✅ Detected missing folder, provided recovery commands
Scenario 3: Empty dist/ folder
✅ Detected empty folder, provided recovery commands
Scenario 4: Missing expected entry points
$ rm packages/types/dist/index.mjs $ pnpm validate ✗ @plane/types: Missing all expected artifacts: ./dist/index.mjs, ./dist/index.cjs ⚠️ Workspace validation failed! Exit code: 1✅ Detected missing critical files, provided recovery commands
Scenario 5: Recovery with build:packages
✅ Quick recovery workflow tested and working
Scenario 6: Auto-repair with dev:fix
✅ Auto-repair workflow tested and working
Scenario 7: Skip validation for debugging
$ SKIP_VALIDATION=1 pnpm dev ⏭️ Skipping validation (SKIP_VALIDATION=1) # ... starts dev without validation ...✅ Escape hatch for advanced users tested
Scenario 8: Verbose validation output
✅ Verbose output tested, provides complete visibility
Performance Impact
pnpm devstartupbuild:packagesvs 2-3 minutes with full rebuildBackward Compatibility
SKIP_VALIDATION=1 pnpm dev^builddependencies)References
Additional Notes
Packages validated (artifact-based, require builds):
Packages skipped (source-based, no build required):
src/index.tsdirectly)New developer workflows:
pnpm validatepnpm build:packagespnpm clean:packagespnpm dev:cleanpnpm dev:fixNote
Adds a pre-flight validator to ensure workspace package artifacts exist before running dev, reducing cryptic module resolution failures.
scripts/validate-workspace.jsvalidatesdist/artifacts for key packages, supports--verbose,--fix, andSKIP_VALIDATION=1package.jsonscripts:devruns validation; addsvalidate,build:packages,clean:packages,dev:clean,dev:fixCONTRIBUTING.mdwith troubleshooting, commands, and explanation of artifact-based buildsturbo.jsonkept functionally unchangedWritten by Cursor Bugbot for commit 9ed0b7b. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.