Revert "fix: exclude root package from workspace script filters to prevent recursion"#114
Revert "fix: exclude root package from workspace script filters to prevent recursion"#114
Conversation
…event recursion" This reverts commit b852d53. We are not getting builds of @fideus-labs/fidnii in CI.
Deploying convert-fidnii with
|
| Latest commit: |
b8d7fc2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://79894bbc.convert-fidnii.pages.dev |
| Branch Preview URL: | https://recursive-build.convert-fidnii.pages.dev |
📝 WalkthroughWalkthroughThe pull request modifies three npm scripts in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Reverts the previous change that excluded the root package from Bun workspace script filters, aiming to restore CI builds for @fideus-labs/fidnii.
Changes:
- Updated root
build,dev, andtestscripts to usebun run --filter '*' ...instead of excluding the root package.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "build": "bun run --filter '*' build", | ||
| "check": "biome check .", | ||
| "dev": "bun run --filter '!./' dev", | ||
| "dev": "bun run --filter '*' dev", | ||
| "format": "biome format --write .", | ||
| "lint": "biome lint .", | ||
| "prepare": "lefthook install", | ||
| "test": "bun run --filter '!./' test" | ||
| "test": "bun run --filter '*' test" |
There was a problem hiding this comment.
Using bun run --filter '*' <script> will also match the root workspace package (this package.json). Because the root build/dev/test scripts are implemented by re-invoking bun run --filter ... <script>, including the root package causes self-invocation recursion (or repeated execution) and can hang/overflow CI.
To keep building all workspaces without recursing, exclude the root package explicitly (e.g., by package name) or avoid a wildcard and instead target the known workspace globs (e.g., fidnii and examples/*) in separate invocations.
Greptile SummaryThis PR reverts commit Key changes and concerns:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["bun run build (workspace root)"]
B{"--filter '*'"}
C["@fideus-labs/fidnii-monorepo (root)\nbuild = bun run --filter '*' build"]
D["@fideus-labs/fidnii\nbuild = tsc"]
E["@fideus-labs/fidnii-example-convert\nbuild = tsc && vite build"]
F["@fideus-labs/fidnii-getting-started\n(no build script – skip)"]
G["♻️ INFINITE RECURSION"]
A --> B
B -->|includes root| C
B --> D
B --> E
B --> F
C -->|calls itself again| G
D -->|tsc compiles fidnii/src| H["✅ dist/ produced"]
E -->|tsc + vite build| I["✅ examples/convert built"]
style G fill:#ff4444,color:#fff
style H fill:#22aa44,color:#fff
style I fill:#22aa44,color:#fff
Last reviewed commit: "Revert "fix: exclude..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ], | ||
| "scripts": { | ||
| "build": "bun run --filter '!./' build", | ||
| "build": "bun run --filter '*' build", |
There was a problem hiding this comment.
The original commit b852d53 explicitly added !./ to prevent infinite recursion: when bun run build is invoked at the workspace root, --filter '*' includes the root package (@fideus-labs/fidnii-monorepo) whose build script is bun run --filter '*' build, causing an infinite recursive loop. The revert brings that risk back.
The reason the !./ fix broke CI builds of @fideus-labs/fidnii is likely that --filter '!./' in isolation acts as a negation-only pattern — it excludes root but doesn't provide a positive match set, so Bun ends up running the script in zero packages.
A more robust fix would use explicit workspace-path filters that clearly include all desired workspace members without touching the root:
"build": "bun run --filter './fidnii' --filter './examples/*' build",
"dev": "bun run --filter './fidnii' --filter './examples/*' dev",
"test": "bun run --filter './fidnii' --filter './examples/*' test"This is unambiguous, avoids the recursion entirely, and will reliably build @fideus-labs/fidnii in CI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@package.json`:
- Around line 11-17: The workspace scripts "build", "dev", and "test" currently
use bun run --filter '*' which includes the root package and causes recursive
execution; update each script ("build", "dev", "test") to use bun run --filter
'!./' (exclude the root) instead so the root package is not re-selected during
filtered runs.
| "build": "bun run --filter '*' build", | ||
| "check": "biome check .", | ||
| "dev": "bun run --filter '!./' dev", | ||
| "dev": "bun run --filter '*' dev", | ||
| "format": "biome format --write .", | ||
| "lint": "biome lint .", | ||
| "prepare": "lefthook install", | ||
| "test": "bun run --filter '!./' test" | ||
| "test": "bun run --filter '*' test" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Bun 1.x workspaces, when executed from monorepo root, does bun run --filter '' <script>include the root package itself? If yes, does that cause recursive execution when the root script is alsobun run --filter '' <script>?
💡 Result:
Yes. In Bun 1.x, the workspace “package set” that --filter operates on includes the root package.json as an addressable package (e.g. you can explicitly target it with --filter './', and Bun’s docs even show excluding it with --filter '!./'). [1]
So from the monorepo root, bun run --filter '*' <script> will match the root package too (assuming * matches its package name / path), meaning the root’s <script> will be executed alongside the workspace packages. [1][2]
And yes: if the root’s <script> is itself bun run --filter '*' <script>, that will cause recursive re-entry (the root run triggers a filtered run that includes root again, etc.) unless you explicitly exclude the root (e.g. --filter '!./') or change the root script to something non-self-referential. [1][2]
Sources: [1] Bun docs: bun --filter (root can be included/excluded via ./ / !./). [2] Bun runtime docs: bun run --filter <pattern> <script> runs a script in all matching workspace packages.
Root package recursion risk with --filter '*' in workspace scripts.
On lines 11, 13, and 17, --filter '*' matches the root package itself. When the root's build, dev, and test scripts execute bun run --filter '*' <script>, the root is re-selected, causing recursive re-entry.
Use --filter '!./' to exclude the root from filtered execution.
Proposed fix
"scripts": {
- "build": "bun run --filter '*' build",
+ "build": "bun run --filter '!./' build",
+ "build:fidnii": "bun run --filter './fidnii' build",
"check": "biome check .",
- "dev": "bun run --filter '*' dev",
+ "dev": "bun run --filter '!./' dev",
"format": "biome format --write .",
"lint": "biome lint .",
"prepare": "lefthook install",
- "test": "bun run --filter '*' test"
+ "test": "bun run --filter '!./' test"
},📝 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.
| "build": "bun run --filter '*' build", | |
| "check": "biome check .", | |
| "dev": "bun run --filter '!./' dev", | |
| "dev": "bun run --filter '*' dev", | |
| "format": "biome format --write .", | |
| "lint": "biome lint .", | |
| "prepare": "lefthook install", | |
| "test": "bun run --filter '!./' test" | |
| "test": "bun run --filter '*' test" | |
| "build": "bun run --filter '!./' build", | |
| "build:fidnii": "bun run --filter './fidnii' build", | |
| "check": "biome check .", | |
| "dev": "bun run --filter '!./' dev", | |
| "format": "biome format --write .", | |
| "lint": "biome lint .", | |
| "prepare": "lefthook install", | |
| "test": "bun run --filter '!./' test" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 11 - 17, The workspace scripts "build", "dev", and
"test" currently use bun run --filter '*' which includes the root package and
causes recursive execution; update each script ("build", "dev", "test") to use
bun run --filter '!./' (exclude the root) instead so the root package is not
re-selected during filtered runs.
|
Needed |
This reverts commit b852d53.
We are not getting builds of @fideus-labs/fidnii in CI.
Summary by CodeRabbit