Skip to content

fix(sbom): add recursiveByDefault so --filter works in workspaces#12187

Closed
Saturate wants to merge 1 commit into
pnpm:mainfrom
Saturate:fix/sbom-filter-in-workspaces
Closed

fix(sbom): add recursiveByDefault so --filter works in workspaces#12187
Saturate wants to merge 1 commit into
pnpm:mainfrom
Saturate:fix/sbom-filter-in-workspaces

Conversation

@Saturate

@Saturate Saturate commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

pnpm sbom --filter <pkg> was silently ignored in workspaces. The sbom command was missing the recursiveByDefault export that other workspace-aware commands (like audit) have.

Without it, the --filter flag was accepted but the recursive code path that populates selectedProjectsGraph never ran. The SBOM always used the workspace root's name/version for the root component and walked all importers.

Reproduction: run pnpm sbom --sbom-format cyclonedx --filter <workspace-pkg> in any workspace. The root component in the output is the workspace root, not the filtered package. Component count drops (some filtering happens at the lockfile walker level), but root component metadata is wrong.

Fix: one-line addition of export const recursiveByDefault = true to the sbom command module, matching audit.


Written by an agent (Claude Code, claude-opus-4-6).

Summary by CodeRabbit

  • Bug Fixes
    • Fixed pnpm sbom --filter command in workspaces—the filter flag is now properly applied instead of being ignored.
    • Corrected SBOM root component metadata selection to accurately reflect filtered workspace projects instead of always defaulting to the workspace root.

The sbom command was missing the recursiveByDefault export that other
workspace-aware commands (like audit) have. Without it, the --filter
flag was accepted but silently ignored: the recursive code path that
populates selectedProjectsGraph from filter expressions never ran.

The result was that pnpm sbom --filter always used the workspace root
manifest for the SBOM root component and walked all importers instead
of only the filtered ones.
@Saturate Saturate requested a review from zkochan as a code owner June 4, 2026 08:15
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Fix sbom --filter being silently ignored in workspaces

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add recursiveByDefault export to sbom command
• Enables --filter flag to work correctly in workspaces
• Fixes sbom always using workspace root metadata
• Ensures filtered packages generate correct SBOM output
Diagram
flowchart LR
  A["sbom command"] -->|missing recursiveByDefault| B["--filter ignored"]
  B -->|root metadata used| C["incorrect SBOM output"]
  A -->|add recursiveByDefault| D["--filter works"]
  D -->|filtered package metadata| E["correct SBOM output"]

Loading

Grey Divider

File Changes

1. deps/compliance/commands/src/sbom/sbom.ts 🐞 Bug fix +2/-0

Add recursiveByDefault export to sbom command

• Added export const recursiveByDefault = true export
• Enables workspace-aware filtering for sbom command
• Aligns sbom with other workspace commands like audit

deps/compliance/commands/src/sbom/sbom.ts


2. .changeset/sbom-filter-workspace-fix.md 📝 Documentation +6/-0

Add changeset for sbom filter workspace fix

• Created changeset documenting the bug fix
• Marks patch version bump for affected packages
• Describes the issue and resolution

.changeset/sbom-filter-workspace-fix.md


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec11f171-509a-4849-83d1-767ddc9ebccd

📥 Commits

Reviewing files that changed from the base of the PR and between 3b76b8e and e10c86e.

📒 Files selected for processing (2)
  • .changeset/sbom-filter-workspace-fix.md
  • deps/compliance/commands/src/sbom/sbom.ts
📜 Recent review details
⏰ 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: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Follow Standard Style with trailing commas, preferring functions over classes, and declaring functions after they are used (relying on hoisting)
Use a single options object instead of multiple parameters when a function needs more than two or three arguments
Follow Import Order: Standard libraries first, then external dependencies (alphabetically), then relative imports
Write self-documenting code where function names, parameters, and types explain what a function does without requiring prose comments
Do not write comments that restate what the code already says; refactor via renaming, splitting helpers, or restructuring instead
Do not repeat documentation at call sites that already exists in JSDoc on the callee; update JSDoc once for all call sites to benefit
Use JSDoc only for a function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the body
Do not record past implementation shape, refactor history, or 'the previous code did X' framing in code; use git log and git blame instead
Write comments only when: the reason for code is non-obvious (hidden invariant, workaround for known bug, deliberate exception), or the right name doesn't fit (temporary technical constraint)

Files:

  • deps/compliance/commands/src/sbom/sbom.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In pacquet (pnpm/pnpm repo), `ResolvedPackage.optional` AND-folding intentionally mirrors pnpm's resolveDependencies.ts:1627-1648 revisit behavior: only the directly-visited package's `optional` flag is updated on revisit, not transitive descendants. pnpm CLI corrects stale optional flags via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`. Pacquet does not yet have this pruner equivalent, so raw `node.optional` flows directly into snapshot/virtual-store via `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up issue to port `copyDependencySubGraph` is planned.
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11789
File: pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs:145-146
Timestamp: 2026-05-21T00:33:05.035Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs`, the guard `bare.starts_with("workspace:.")` is intentionally broad — matching pnpm upstream's identical `startsWith('workspace:.')` check at `resolving/npm-resolver/src/index.ts`. All dot-prefixed workspace forms including `workspace:.foo` are intentionally passed through to the local-resolver, which handles them as `link:`-style directory specs via its prefix-stripping regex. Do not suggest narrowing this to `workspace:./` or `workspace:../` only.
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: Always explicitly include 'pnpm' in changeset files with appropriate version bump (patch, minor, or major)
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11755
File: pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs:193-200
Timestamp: 2026-05-20T01:52:55.764Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs`, the package-level `modified` shortcut for the maturity/minimumReleaseAge filter uses inclusive `<=` (not strict `<`) when comparing `modified_date <= cutoff`. This mirrors the corrected behavior in pnpm (fixed in ab4c96ead5). The reasoning: `modified` is "last modification time," which is an upper bound on every version's `time[v]`. The per-version maturity filter uses `<=` (a version published exactly at the cutoff is mature). Since `modified == cutoff` means every version satisfies the per-version filter, the abbreviated-metadata fast path should accept this case rather than forcing a full-metadata re-fetch or raising `MissingTime`. The same fix was applied to pnpm TS: `pickPackage.ts` (×2) and `pickPackageFromMeta.ts`.
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11789
File: pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs:145-146
Timestamp: 2026-05-21T00:33:05.035Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs`, the guard `bare.starts_with("workspace:.")` is intentionally broad — matching pnpm upstream's identical `startsWith('workspace:.')` check. All dot-prefixed workspace forms including `workspace:.foo` are intentionally handed off to the local-resolver, which handles them as `link:`-style directory specs via its prefix-stripping regex. Do not suggest narrowing this to `workspace:./` or `workspace:../` only.
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: Always explicitly include 'pnpm' in changeset files with appropriate version bump (patch, minor, or major)

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-20T01:52:55.764Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11755
File: pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs:193-200
Timestamp: 2026-05-20T01:52:55.764Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs`, the package-level `modified` shortcut for the maturity/minimumReleaseAge filter uses inclusive `<=` (not strict `<`) when comparing `modified_date <= cutoff`. This mirrors the corrected behavior in pnpm (fixed in ab4c96ead5). The reasoning: `modified` is "last modification time," which is an upper bound on every version's `time[v]`. The per-version maturity filter uses `<=` (a version published exactly at the cutoff is mature). Since `modified == cutoff` means every version satisfies the per-version filter, the abbreviated-metadata fast path should accept this case rather than forcing a full-metadata re-fetch or raising `MissingTime`. The same fix was applied to pnpm TS: `pickPackage.ts` (×2) and `pickPackageFromMeta.ts`.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-07T20:38:01.796Z
Learnt from: camcima
Repo: pnpm/pnpm PR: 11159
File: deps/compliance/license-checker/src/utils.ts:42-59
Timestamp: 2026-05-07T20:38:01.796Z
Learning: In `deps/compliance/license-checker/src/utils.ts`, `collectDirectDeps` intentionally uses name-only keys (not `nameversion`) for the shallow-depth filter. This is a deliberate design decision: being over-inclusive (auditing extra versions of a direct dep) is safer for compliance than being under-inclusive. Tightening to `nameversion` would require unsafe `(lockfile.packages as any)?.[ref]` casts through untyped lockfile internals. Do not flag this as a bug.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-24T21:11:04.272Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: Version pnpm CLI patch for bug fixes, internal refactors, and changes that don't require documentation; minor for new features/settings that should be documented; major for breaking changes

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-21T00:33:05.035Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11789
File: pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs:145-146
Timestamp: 2026-05-21T00:33:05.035Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs`, the guard `bare.starts_with("workspace:.")` is intentionally broad — matching pnpm upstream's identical `startsWith('workspace:.')` check at `resolving/npm-resolver/src/index.ts`. All dot-prefixed workspace forms including `workspace:.foo` are intentionally passed through to the local-resolver, which handles them as `link:`-style directory specs via its prefix-stripping regex. Do not suggest narrowing this to `workspace:./` or `workspace:../` only.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-24T21:11:04.272Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In pacquet (pnpm/pnpm repo), `ResolvedPackage.optional` AND-folding intentionally mirrors pnpm's resolveDependencies.ts:1627-1648 revisit behavior: only the directly-visited package's `optional` flag is updated on revisit, not transitive descendants. pnpm CLI corrects stale optional flags via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`. Pacquet does not yet have this pruner equivalent, so raw `node.optional` flows directly into snapshot/virtual-store via `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up issue to port `copyDependencySubGraph` is planned.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-24T16:07:54.784Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11904
File: pacquet/crates/package-manager/src/install.rs:556-560
Timestamp: 2026-05-24T16:07:54.784Z
Learning: In pacquet's `is_modules_yaml_consistent` (pacquet/crates/package-manager/src/install.rs), `enableGlobalVirtualStore` is intentionally NOT checked as a separate field. Upstream pnpm's `validateModules.ts` does not persist or check `enableGlobalVirtualStore` in `.modules.yaml` either. Drift on this setting is caught indirectly: toggling `enableGlobalVirtualStore` changes `config.effective_virtual_store_dir()` (GVS-on → `<store>/v11/links`, GVS-off → `<project>/node_modules/.pnpm`), so the existing `modules.virtual_store_dir == config.effective_virtual_store_dir()` comparison in `is_modules_yaml_consistent` already detects the mismatch and prevents the short-circuit. Do not flag the absence of an explicit `enableGlobalVirtualStore` field as a bug.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Match how the same feature is implemented in the TypeScript pnpm CLI — any change in pacquet must match pnpm's behavior, logic, edge cases, config resolution, error messages, file/lockfile formats, and existing tests

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-26T21:01:06.666Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.

Applied to files:

  • .changeset/sbom-filter-workspace-fix.md
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

  • deps/compliance/commands/src/sbom/sbom.ts
🔇 Additional comments (2)
deps/compliance/commands/src/sbom/sbom.ts (1)

72-72: LGTM!

.changeset/sbom-filter-workspace-fix.md (1)

1-7: LGTM!


📝 Walkthrough

Walkthrough

This PR fixes workspace filtering for the pnpm sbom command. The SBOM command now exports recursiveByDefault as true, enabling the --filter flag to properly populate the workspace project graph. The changeset documents the patch releases and clarifies that SBOM root component metadata selection now respects filtering instead of always deriving from the workspace root.

Changes

SBOM Filter Workspace Fix

Layer / File(s) Summary
SBOM recursiveByDefault export
deps/compliance/commands/src/sbom/sbom.ts
Exported constant recursiveByDefault set to true enables the command to default to recursive workspace behavior, allowing the --filter flag to populate the workspace project graph.
SBOM filter workspace fix changeset
.changeset/sbom-filter-workspace-fix.md
Changeset documents patch releases for @pnpm/deps.compliance.commands and pnpm, noting the fix for workspace filtering and the correction to SBOM root component metadata selection to respect the filter.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A flag that whispers recursive: true,
Now filters work in workspaces through and through,
SBOM respects your --filter call,
No more ignoring workspace bounds at all! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main fix: adding recursiveByDefault to enable --filter support in workspaces for the sbom command.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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.

@Saturate

Copy link
Copy Markdown
Contributor Author

Change landed on main via #12443.

@Saturate Saturate closed this Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant