feat(sbom): per-package SBOM generation with --out and --split#12097
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (17)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThis PR extends SBOM generation to support workspace-aware per-package output and workspace dependency collection. The command adds ChangesSBOM workspace support with per-package output
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Handler
participant buildSharedContext
participant resolveWorkspaceDeps
participant collectSbomComponents
participant Serializer
participant FileSystem
CLI->>Handler: parse options (--filter, --out, --split, --format)
Handler->>buildSharedContext: load lockfile + root manifest (+storeDir)
alt split mode (--split or %s in --out)
Handler->>Handler: iterate workspace projects
loop per matched project
Handler->>resolveWorkspaceDeps: discover link: dependencies
resolveWorkspaceDeps-->>Handler: WorkspaceLink[] + additionalImporterIds
Handler->>collectSbomComponents: with workspacePackages + resolvedWorkspaceDeps
collectSbomComponents-->>Handler: SBOM with workspace components
Handler->>Serializer: SbomResult + compact/specVersion
Serializer-->>Handler: serialized JSON
Handler->>FileSystem: write file or emit NDJSON line
end
else single SBOM
Handler->>collectSbomComponents: generate single SBOM
collectSbomComponents-->>Handler: SBOM
Handler->>Serializer: SbomResult + compact/specVersion
Serializer-->>Handler: serialized JSON
Handler->>FileSystem: write to --out or stdout
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
a5ccfbc to
0ba6cc8
Compare
Review Summary by QodoAdd per-package SBOM generation with --out and --split flags for workspaces
WalkthroughsDescription• Add --out flag to write SBOM to file with %s and %v placeholders • Add --split flag for NDJSON output with per-package SBOM generation • Include workspace inter-dependencies and transitive deps in SBOM • Use filtered package metadata as root component when --filter selects single package • Add recursiveByDefault to enable --filter in workspace contexts • Support --prod and --lockfile-only for workspace dependency filtering Diagramflowchart LR
A["SBOM Command"] -->|"--out %s"| B["Per-Package Files"]
A -->|"--split"| C["NDJSON to stdout"]
A -->|"--filter"| D["Single Package Metadata"]
D --> E["Root Component"]
F["Workspace Deps"] --> G["Transitive Resolution"]
G --> H["SBOM Components"]
File Changes1. deps/compliance/commands/src/sbom/sbom.ts
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/sbom-workspace-root-component.md:
- Line 7: The PR adds new user-visible SBOM options but pacquet's CLI lacks a
corresponding sbom subcommand and flags—update the pacquet CLI to mirror pnpm by
adding an Sbom variant to the CliCommand enum (in
pacquet/crates/cli/src/cli_args.rs) and wire in flags matching pnpm: --out
(string with format placeholder), --split (bool), and --filter (package
selector) and ensure the command handlers that construct SBOM behavior consume
these flags and apply root-component metadata fallback logic; alternatively, if
an upstream pacquet commit already implements this parity, reference the exact
pacquet commit/PR that adds the sbom subcommand and flag wiring in the PR
description so the repo stays synchronized.
In `@deps/compliance/commands/src/sbom/sbom.ts`:
- Around line 179-192: The single-output branch writes opts.out verbatim so
placeholders like %s or %v aren't expanded; update the non-split path in the
code around serialOpts/shouldSplit/handleSplit/generateSbomForProject so it
applies the same placeholder-rendering used by split mode to produce a concrete
file path before calling fs.mkdirSync/fs.writeFileSync (i.e., compute an
expandedOutPath from opts.out using the same placeholder logic used by
handleSplit, use that expanded path for dirname creation and writing, and ensure
you don't change the split-detection logic).
- Around line 288-299: buildSharedContext currently reads rootManifest from
opts.dir (the filtered package) so license/author/repository/description
fallbacks point to the package instead of the actual workspace root; update the
code paths (where rootManifest is set and used at the return sites around
readProjectManifestOnly and later fallback logic at the blocks referenced) to
load and use rootProjectManifest and rootProjectManifestDir (or workspace root
manifest) as the fallback source when opts.dir is a workspace package—ensure
buildSharedContext and any fallback logic (including rootDescription resolution)
consistently prefer values from rootProjectManifest/rootProjectManifestDir
before using the package manifest so filtered-package runs correctly inherit
workspace root metadata.
In `@deps/compliance/sbom/src/collectComponents.ts`:
- Around line 55-62: The workspace importer deps
(workspaceDeps.additionalImporterIds) are being walked with parentPurl set to
rootPurl so direct deps of a reachable workspace package attach to the root;
change the walker initialization so each importer walk uses that importer's
component PURL as its starting parent instead of rootPurl — e.g., when building
importerWalkers via lockfileWalkerGroupImporterSteps, pass or compute the
correct parent PURL for each importerId (use the workspace component PURL
derived from workspaceDeps/importer id) so the walker functions (which reference
parentPurl) emit dependencies under the workspace package rather than the root;
apply the same fix at the other occurrence around lines 125-135.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58b80bcf-8232-4775-9c74-ff4c86bf0918
⛔ Files ignored due to path filters (2)
deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.changeset/sbom-workspace-root-component.mddeps/compliance/commands/src/sbom/sbom.tsdeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/app/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/dev-tool/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-a/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-b/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/shared-lib/package.jsondeps/compliance/commands/test/sbom/index.tsdeps/compliance/sbom/src/collectComponents.tsdeps/compliance/sbom/src/index.tsdeps/compliance/sbom/src/serializeCycloneDx.tsdeps/compliance/sbom/src/serializeSpdx.ts
📜 Review details
🧰 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/sbom/src/index.tsdeps/compliance/sbom/src/serializeCycloneDx.tsdeps/compliance/sbom/src/serializeSpdx.tsdeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
🧠 Learnings (35)
📓 Common learnings
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: 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.
📚 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:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/package.jsondeps/compliance/sbom/src/index.tsdeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-b/package.json.changeset/sbom-workspace-root-component.mddeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/dev-tool/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/shared-lib/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-a/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/app/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/package.jsondeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 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:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 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:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml.changeset/sbom-workspace-root-component.mddeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yaml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/Cargo.toml : Declare new shared dependencies in the root [workspace.dependencies] and use { workspace = true } in pnpr crate's Cargo.toml
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml.changeset/sbom-workspace-root-component.mddeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-a/package.json
📚 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:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 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: Do not change lockfile format, store layout, `.npmrc` semantics, or CLI surface unless pnpm changed them first
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml.changeset/sbom-workspace-root-component.mddeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yaml
📚 Learning: 2026-05-20T23:08:06.093Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:08:06.093Z
Learning: Pacquet (pnpm's Rust port) has a cardinal rule: "match pnpm exactly — do not fix pnpm quirks unless the same fix has landed in pnpm first." Review comments should not suggest behavioral deviations from upstream pnpm, even when the upstream behavior appears buggy. If a real bug is identified, it must be fixed upstream first.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yaml
📚 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. 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.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 Learning: 2026-06-01T08:59:48.719Z
Learnt from: KSXGitHub
Repo: pnpm/pnpm PR: 12093
File: pacquet/crates/cli/src/cli_args/run/recursive.rs:290-315
Timestamp: 2026-06-01T08:59:48.719Z
Learning: In pacquet's recursive run implementation (`pacquet/crates/cli/src/cli_args/run/recursive.rs`), the `pnpm-exec-summary.json` format for failed package entries correctly includes `prefix` and `message` fields in addition to `status` and `duration`. This matches pnpm's `ActionFailure` variant in `cli/utils/src/recursiveSummary.ts` and the direct serialization in `exec/commands/src/exec.ts`. There is no `ExecutionStatusInSummary` type in pnpm. The only intentional divergence is omitting the JS `error` field, whose `JSON.stringify` output is non-deterministic due to non-enumerable `Error` properties.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml
📚 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:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml
📚 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: Extract and refactor common code into shared packages rather than duplicating it across the monorepo
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yamldeps/compliance/sbom/src/index.ts
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-b/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/dev-tool/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/shared-lib/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-a/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/app/package.jsondeps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/package.json
📚 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:
deps/compliance/sbom/src/index.tsdeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 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/sbom/src/index.tsdeps/compliance/sbom/src/serializeCycloneDx.tsdeps/compliance/sbom/src/serializeSpdx.tsdeps/compliance/sbom/src/collectComponents.tsdeps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 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-workspace-root-component.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: Create a changeset file in .changeset directory for changes affecting published packages, with explicit version bump types (patch, minor, major)
Applied to files:
.changeset/sbom-workspace-root-component.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-workspace-root-component.mddeps/compliance/sbom/src/collectComponents.ts
📚 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-workspace-root-component.mddeps/compliance/sbom/src/collectComponents.ts
📚 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-workspace-root-component.md
📚 Learning: 2026-05-19T19:23:00.981Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11752
File: pacquet/crates/config/src/lib.rs:1062-1073
Timestamp: 2026-05-19T19:23:00.981Z
Learning: In `pacquet/crates/config/src/lib.rs`, `modules_dir` does not need a `!virtual_store_dir_explicit` guard on its workspace re-anchor because `modules_dir` is in pnpm's `excludedPnpmKeys` (filtered out by `WorkspaceSettings::clear_workspace_only_fields`) and therefore can only be set by workspace yaml (applied immediately after) or env vars (applied later in the cascade) — not by global `config.yaml`. `virtual_store_dir`, by contrast, IS settable from global config and requires the `if !virtual_store_dir_explicit` guard to survive the workspace-root re-anchor.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yamldeps/compliance/commands/src/sbom/sbom.ts
📚 Learning: 2026-06-02T14:39:24.423Z
Learnt from: KSXGitHub
Repo: pnpm/pnpm PR: 11938
File: pacquet/crates/config/src/lib.rs:959-966
Timestamp: 2026-06-02T14:39:24.423Z
Learning: In the pnpm/pnpm pacquet Rust port (`pacquet/crates/config/src/lib.rs`), `Config.extra_bin_paths` is intentionally left empty (`Vec::new()`) until workspace fan-out support for `run`/`exec` is implemented. It mirrors pnpm's `Config.extraBinPaths` (the workspace-root `node_modules/.bin`), which is also empty outside a workspace. Populating it before the workspace-root is resolved would put a non-existent path on `PATH`, so it should only be derived once workspace support lands. Do not flag this as a bug or missing derivation in reviews.
Applied to files:
deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yaml
📚 Learning: 2026-06-02T13:18:30.659Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12134
File: pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs:311-325
Timestamp: 2026-06-02T13:18:30.659Z
Learning: In pacquet's lockfile resolution verifier (`pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`), URL-keyed tarball dependencies do NOT need a separate `non_semver_version` field in `VerifyCtx`. Unlike the TypeScript side (which derives `version` from `snapshot.version` and threads `nonSemverVersion` separately), pacquet's `collect_candidates` takes `version` from the lockfile key suffix. For a URL-keyed dep the key is `name@<url>`, so `ctx.version` is the URL string, which fails `node_semver::Version::parse(ctx.version)` and the existing guard `if node_semver::Version::parse(ctx.version).is_err() { return ResolutionVerification::Ok; }` already skips the registry lookup correctly. Adding a `non_semver_version` field to `VerifyCtx` for this purpose would be inert.
Applied to files:
deps/compliance/sbom/src/collectComponents.ts
📚 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: Applies to pacquet/**/tests/**/*.rs : Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 Learning: 2026-05-20T21:18:56.391Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11778
File: pacquet/crates/resolving-local-resolver/tests/resolve.rs:365-372
Timestamp: 2026-05-20T21:18:56.391Z
Learning: In `pacquet/crates/resolving-local-resolver/tests/resolve.rs`, the test `fail_when_resolving_from_not_existing_directory_an_injected_dependency` intentionally uses `injected: false`. The test is a verbatim port of the upstream pnpm TypeScript test (resolving/local-resolver/test/index.ts at ef87f3ccff). The `injected` flag only affects the file/link protocol choice for plain directory paths; when the `file:` scheme is explicit in the bare specifier, the flag has no effect on the resolution code path. The misleading test name is inherited from upstream.
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 Learning: 2026-05-05T23:03:30.044Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:30.044Z
Learning: In the pnpm/pnpm repository, the pattern `cross-env NODE_OPTIONS="$NODE_OPTIONS ..." jest` is an intentional, established convention used across many package.json files (e.g., `fs/hard-link-dir`, `worker`, `__utils__/scripts`). Do not flag this as a cross-platform issue in individual files; any fix should be applied repo-wide as a separate change.
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 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: Applies to **/*.test.{ts,tsx} : Use util.types.isNativeError() instead of instanceof Error for error type checking in Jest tests
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 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: Build the pnpm bundle (pnpm/dist/pnpm.mjs) by running `pnpm --filter pnpm run compile` before running e2e tests
Applied to files:
deps/compliance/commands/test/sbom/index.tsdeps/compliance/commands/src/sbom/sbom.ts
📚 Learning: 2026-05-23T16:55:36.507Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: pacquet/crates/cli/tests/lockfile_verification.rs:158-162
Timestamp: 2026-05-23T16:55:36.507Z
Learning: In `pacquet/crates/cli/tests/lockfile_verification.rs`, the `trust_lockfile_skips_verification` and `trust_lockfile_cli_flag_skips_verification` tests intentionally do NOT assert `output.status.success()`. The hand-rolled fixture lockfile uses a placeholder integrity hash (`sha512-AAA…`), so the install always fails the downstream tarball integrity check regardless of the supply-chain gate. The contract being tested is "gate-skipped, not install-succeeded"; asserting success would require generating a real lockfile via the `generate_lockfile` pattern (see `hoist.rs`) which is considered not worth the extra wiring for an opt-out smoke test.
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 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: Applies to pacquet/**/*.rs : Tests are documentation — do not duplicate test scenarios, edge cases, failure modes, or worked examples in prose when they are already captured by tests
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 Learning: 2026-05-24T08:18:06.019Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11895
File: pnpm/test/deploy.ts:91-95
Timestamp: 2026-05-24T08:18:06.019Z
Learning: In the pnpm/pnpm repository, integration tests that hit the real `registry.npmjs.org` (e.g., for `pacquet` or `pnpm/pacquet`) do NOT use a runtime env-var gate (such as `PNPM_RUN_PUBLIC_REGISTRY_TESTS`). They simply pass `--config.registry=https://registry.npmjs.org/` directly to `execPnpm` and set a higher timeout. This is the established pattern, as seen in `pnpm/test/install/pacquet.ts` and `pnpm/test/deploy.ts`. Do not suggest adding env-var guards for these tests.
Applied to files:
deps/compliance/commands/test/sbom/index.ts
📚 Learning: 2026-05-20T21:18:55.266Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11778
File: pacquet/crates/resolving-local-resolver/src/parse_bare_specifier.rs:253-278
Timestamp: 2026-05-20T21:18:55.266Z
Learning: In `pacquet/crates/resolving-local-resolver/src/parse_bare_specifier.rs`, the `resolve_path` function intentionally short-circuits absolute specifiers verbatim (returns them unchanged without normalizing `..` components), mirroring the upstream TypeScript `resolvePath` in `resolving/local-resolver/src/parseBareSpecifier.ts` at ef87f3ccff. The OS resolves `..` at `fs.read` time. Do not suggest normalizing the absolute branch — it would invent behavior pnpm doesn't have, violating the pacquet AGENTS.md cardinal rule of fidelity to upstream.
Applied to files:
deps/compliance/commands/src/sbom/sbom.ts
📚 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: Do not add features, flags, or behaviors that pnpm does not have
Applied to files:
deps/compliance/commands/src/sbom/sbom.ts
📚 Learning: 2026-05-26T18:31:14.579Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11967
File: .changeset/git-fetcher-reject-non-sha-commits.md:2-2
Timestamp: 2026-05-26T18:31:14.579Z
Learning: In the pnpm monorepo, the `fetching/` directory contains multiple separate npm packages each with their own scoped name using a dot-separator convention, e.g., `pnpm/fetching.git-fetcher` (declared in `fetching/git-fetcher/package.json`) and `pnpm/fetching.tarball-fetcher`. There is no top-level `pnpm/fetching` package. Changesets targeting the git-fetcher should use `"pnpm/fetching.git-fetcher"`.
Applied to files:
deps/compliance/commands/src/sbom/sbom.ts
📚 Learning: 2026-05-20T22:49:17.652Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11787
File: pacquet/crates/catalogs-resolver/src/lib.rs:156-167
Timestamp: 2026-05-20T22:49:17.652Z
Learning: In pacquet's `catalogs-resolver` crate (`pacquet/crates/catalogs-resolver/src/lib.rs`), the protocol detection pattern `catalog_lookup.split(':').next().unwrap_or("")` is an intentional byte-for-byte port of pnpm's upstream JavaScript `getProtocol`/split logic in `catalogs/resolver/src/resolveFromCatalog.ts#L95`. A bare value like `"workspace"` (without a colon) is deliberately classified as the `"workspace"` protocol, matching upstream behavior. pacquet's cardinal rule is to match upstream pnpm behavior including quirks; any behavioral change must land in pnpm first and then be ported here.
Applied to files:
deps/compliance/commands/src/sbom/sbom.ts
🔇 Additional comments (10)
deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/app/package.json (1)
1-10: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/dev-tool/package.json (1)
1-7: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/package.json (1)
1-5: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom-dev/pnpm-workspace.yaml (1)
1-3: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-a/package.json (1)
1-9: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom/app-b/package.json (1)
1-8: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom/package.json (1)
1-5: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom/pnpm-workspace.yaml (1)
1-4: LGTM!deps/compliance/commands/test/sbom/fixtures/workspace-sbom/shared-lib/package.json (1)
1-8: LGTM!deps/compliance/commands/test/sbom/index.ts (1)
2-2: LGTM!Also applies to: 11-11, 282-754
7268a44 to
106c8ca
Compare
Code Review by Qodo
1. Silent manifest read failure
|
|
Code review by qodo was updated up to the latest commit a9c9bcd |
|
Code review by qodo was updated up to the latest commit 4ea59bb |
Contributor
Integrated-Benchmark Report (Linux)Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.33309694706,
"stddev": 0.1647017853570764,
"median": 4.36848863236,
"user": 3.9878318399999997,
"system": 3.53405938,
"min": 4.09149161236,
"max": 4.55971461536,
"times": [
4.5109913293599995,
4.09149161236,
4.35541975636,
4.55971461536,
4.417466861359999,
4.10205471036,
4.15255075636,
4.41485219236,
4.34487012836,
4.38155750836
]
},
{
"command": "pacquet@main",
"mean": 4.2043125659600005,
"stddev": 0.10682301753174021,
"median": 4.18344040036,
"user": 3.9641756399999992,
"system": 3.5438295799999997,
"min": 4.0882977903599995,
"max": 4.42669314836,
"times": [
4.42669314836,
4.20344195536,
4.2137493483599995,
4.28652349036,
4.11834195536,
4.30260394936,
4.16343884536,
4.11838576436,
4.12164941236,
4.0882977903599995
]
},
{
"command": "pnpr@HEAD",
"mean": 2.12228607956,
"stddev": 0.11190447147131097,
"median": 2.1402907108599996,
"user": 2.64920634,
"system": 2.97274108,
"min": 1.9324428423600002,
"max": 2.2771839683599997,
"times": [
2.20131835836,
2.19323662636,
2.1290057493599996,
2.01411706036,
2.21863815036,
2.15157567236,
2.2771839683599997,
1.98263603236,
1.9324428423600002,
2.1227063353599998
]
},
{
"command": "pnpr@main",
"mean": 2.1664264305600005,
"stddev": 0.1276723832251958,
"median": 2.1216238233599998,
"user": 2.63904414,
"system": 2.9736089799999994,
"min": 2.0297729903599997,
"max": 2.37357457236,
"times": [
2.37357457236,
2.1462901613599996,
2.09695748536,
2.33527845236,
2.14945788636,
2.05202449836,
2.31921880536,
2.08250503236,
2.0297729903599997,
2.07918442136
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6247373162,
"stddev": 0.01223387560020348,
"median": 0.6188255063000001,
"user": 0.3780213,
"system": 1.3138781,
"min": 0.6122629223,
"max": 0.6422488973,
"times": [
0.6194246263000001,
0.6236082623,
0.6141591703,
0.6414835223,
0.6422488973,
0.6172243763,
0.6122629223,
0.6166947383,
0.6420402603,
0.6182263863
]
},
{
"command": "pacquet@main",
"mean": 0.6465583151000001,
"stddev": 0.05040650302491906,
"median": 0.6291261068,
"user": 0.3734084,
"system": 1.3252814,
"min": 0.6195670753,
"max": 0.7860262223000001,
"times": [
0.6586874393000001,
0.6210236063000001,
0.6298157323,
0.6345961683,
0.6284364813000001,
0.6413445133000001,
0.6195670753,
0.6217309103,
0.6243550023000001,
0.7860262223000001
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6814613193,
"stddev": 0.016674539521310622,
"median": 0.6857611868,
"user": 0.37091679999999994,
"system": 1.3665986000000003,
"min": 0.6531855423,
"max": 0.6994847813,
"times": [
0.6619760423000001,
0.6670513783,
0.6994847813,
0.6985530493000001,
0.6531855423,
0.6740877243000001,
0.6802799583,
0.6912424153000001,
0.6969768153,
0.6917754863000001
]
},
{
"command": "pnpr@main",
"mean": 0.6875101109,
"stddev": 0.026730686153557077,
"median": 0.6793291443,
"user": 0.39107719999999996,
"system": 1.3473103000000002,
"min": 0.6521993783000001,
"max": 0.7344452143000001,
"times": [
0.7344452143000001,
0.6821183133000001,
0.6765399753,
0.6758468363000001,
0.6944657803000001,
0.6564524473000001,
0.7102775873,
0.7188163483000001,
0.6739392283000001,
0.6521993783000001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.264090858240001,
"stddev": 0.06874075785068832,
"median": 4.264026363639999,
"user": 3.7885456999999993,
"system": 3.388741600000001,
"min": 4.17436224414,
"max": 4.39083408014,
"times": [
4.35239669514,
4.21126444414,
4.26252189314,
4.20243348514,
4.17436224414,
4.26553083414,
4.39083408014,
4.26613702014,
4.30205536314,
4.21337252314
]
},
{
"command": "pacquet@main",
"mean": 4.263507668339999,
"stddev": 0.043127559128454326,
"median": 4.26024050064,
"user": 3.7816162999999996,
"system": 3.3856376,
"min": 4.17954686514,
"max": 4.3364360391400005,
"times": [
4.29405963214,
4.30417446414,
4.25677379914,
4.2322311611400005,
4.17954686514,
4.25250332414,
4.26370720214,
4.27478309714,
4.3364360391400005,
4.24086109914
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1968462890400007,
"stddev": 0.14657713411986031,
"median": 2.1635732556400002,
"user": 2.4930117999999997,
"system": 2.9208235,
"min": 2.01866301514,
"max": 2.41590996214,
"times": [
2.01866301514,
2.34689642314,
2.3508689521400004,
2.08305509214,
2.41590996214,
2.09603652814,
2.23110998314,
2.0869804811400003,
2.29098908914,
2.04795336414
]
},
{
"command": "pnpr@main",
"mean": 2.1903939926400002,
"stddev": 0.11729265409896569,
"median": 2.1534324166400003,
"user": 2.480596,
"system": 2.9218843,
"min": 2.05467516314,
"max": 2.3848028071400003,
"times": [
2.3381417791400003,
2.05467516314,
2.09357267114,
2.15782806814,
2.2595128841400003,
2.11599859314,
2.06828777514,
2.3848028071400003,
2.28208342014,
2.14903676514
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.39339588346,
"stddev": 0.02159119339594864,
"median": 1.39585780126,
"user": 1.3927168599999997,
"system": 1.7429561599999999,
"min": 1.36640817926,
"max": 1.43288795826,
"times": [
1.39077692526,
1.4009386772599999,
1.3793997602599999,
1.41028607226,
1.40979639826,
1.43288795826,
1.40140995826,
1.37482470526,
1.36723020026,
1.36640817926
]
},
{
"command": "pacquet@main",
"mean": 1.38867008996,
"stddev": 0.0199881854485022,
"median": 1.3870546397599999,
"user": 1.36732316,
"system": 1.74562226,
"min": 1.36187244926,
"max": 1.42303228326,
"times": [
1.40727134726,
1.38102207226,
1.39050919226,
1.3836000872599998,
1.42303228326,
1.39917470726,
1.3633711582599999,
1.36187244926,
1.40443604226,
1.37241156026
]
},
{
"command": "pnpr@HEAD",
"mean": 0.66982843266,
"stddev": 0.047713013450584645,
"median": 0.6565513517600001,
"user": 0.34289345999999993,
"system": 1.2957151599999999,
"min": 0.6368800932600001,
"max": 0.8027627392600001,
"times": [
0.6555067372600001,
0.6368800932600001,
0.8027627392600001,
0.65008619726,
0.6527549902600001,
0.65759596626,
0.6668408512600001,
0.66893874626,
0.6447980042600001,
0.6621200012600001
]
},
{
"command": "pnpr@main",
"mean": 0.6539819678600001,
"stddev": 0.013560620124998932,
"median": 0.65230340626,
"user": 0.33653456000000004,
"system": 1.2921858599999998,
"min": 0.6352175152600001,
"max": 0.6762284932600001,
"times": [
0.6574818252600001,
0.65274953826,
0.64134768626,
0.6440027662600001,
0.6352175152600001,
0.6762284932600001,
0.6476097672600001,
0.6518572742600001,
0.6574885992600001,
0.6758362132600001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.0293610975200003,
"stddev": 0.03294176498455949,
"median": 3.0203432791199996,
"user": 1.8022027599999997,
"system": 1.96482156,
"min": 2.9967180776199998,
"max": 3.10975399662,
"times": [
3.0342493876199996,
2.9967180776199998,
2.99803795962,
3.01691661462,
3.05083759562,
3.00964131762,
3.01807185762,
3.10975399662,
3.0367694676199997,
3.0226147006199997
]
},
{
"command": "pacquet@main",
"mean": 3.0608972310199998,
"stddev": 0.07156377200897597,
"median": 3.03159156512,
"user": 1.8508644599999997,
"system": 1.9620173600000002,
"min": 2.97623429162,
"max": 3.1978067916199997,
"times": [
3.0444629116199997,
3.0365490086199998,
3.02663412162,
3.01408200762,
2.97623429162,
3.02107116362,
3.01751600262,
3.1978067916199997,
3.11722677162,
3.1573892396199996
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6905373415200001,
"stddev": 0.005102945150751648,
"median": 0.6917412931200001,
"user": 0.35558305999999995,
"system": 1.31619896,
"min": 0.6803382866200001,
"max": 0.6964001856200001,
"times": [
0.6958374116200001,
0.6917817396200001,
0.6938398946200001,
0.6917008466200001,
0.68894437362,
0.69418974462,
0.68601217262,
0.6803382866200001,
0.6964001856200001,
0.6863287596200001
]
},
{
"command": "pnpr@main",
"mean": 0.6771469396199999,
"stddev": 0.044960376880998736,
"median": 0.66503326312,
"user": 0.34398745999999997,
"system": 1.3037417599999999,
"min": 0.64901574362,
"max": 0.8027682236200001,
"times": [
0.6709259226200001,
0.67275655962,
0.6567678436200001,
0.6577595906200001,
0.6569558106200001,
0.65914060362,
0.6728450976200001,
0.8027682236200001,
0.6725340006200001,
0.64901574362
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12097 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,264.09 ms(+1.62%)Baseline: 4,196.12 ms | 5,035.34 ms (84.68%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 3,029.36 ms(+0.88%)Baseline: 3,002.98 ms | 3,603.58 ms (84.07%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,393.40 ms(+5.33%)Baseline: 1,322.84 ms | 1,587.41 ms (87.78%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,333.10 ms(+5.03%)Baseline: 4,125.69 ms | 4,950.83 ms (87.52%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 624.74 ms(+0.67%)Baseline: 620.59 ms | 744.71 ms (83.89%) |
4ea59bb to
d01192d
Compare
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
|
Code review by qodo was updated up to the latest commit 7f539fa |
Contributor
|
| Branch | pr/12097 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,196.85 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 690.54 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 669.83 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,122.29 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 681.46 ms |
When --filter selects a single workspace, the SBOM root component now uses that workspace's name, version, description, license, and author instead of the workspace root's metadata. Author, repository, and license fall back to the root manifest when the workspace package doesn't define them. Workspace inter-dependencies (workspace: protocol) and their transitive dependencies are now included in the SBOM. Dev-only workspace deps are correctly excluded when --prod is used, and lockfile-only mode skips workspace resolution entirely to avoid unexpected disk reads.
Without recursiveByDefault, the sbom command in a workspace didn't enter the recursive code path that populates selectedProjectsGraph from --filter. The handler received no filter info and always used the workspace root manifest for the root component.
Add --out <path> to write SBOM to a file instead of stdout. Supports %s (package name) and %v (version) placeholders. When %s is present, generates one SBOM per workspace package automatically. Add --split to output NDJSON (one compact JSON per line) to stdout, for piping into tooling that processes multiple SBOMs. Add compact option to CycloneDX and SPDX serializers so NDJSON lines are single-line JSON without re-parsing. Sanitize %s and %v values to prevent path traversal in output paths.
…nt relationships Two bugs found by CodeRabbit: 1. --out with %v but no %s wrote a literal %v filename. Now both %s and %v are expanded in the single-output path using the SBOM root component's name and version. 2. The lockfile walker used rootPurl as parent for every importer, including additional workspace dep importers. This caused shared-lib's external deps (like is-odd) to appear as direct deps of the root package. Now each importer's walk uses the correct parent PURL based on whether it's an original or workspace dep importer.
…metadata Read the root manifest from rootProjectManifest/rootProjectManifestDir instead of opts.dir so license/author/repository/description fall back to the workspace root when a filtered package omits them. Add the missing rootDescription fallback. Fix the per-package CycloneDX test assertions to match the standard group/name split for scoped names.
…kspace deps - Neutralize '.', '..' and blank segments in --out path templates so a crafted package name/version cannot escape the output directory. - Skip lockfile link: targets that resolve outside the workspace root, and guard the workspace manifest reads with a containment check, preventing arbitrary package.json reads from a malicious lockfile. - Honor --lockfile-only inside collectSbomComponents so workspace links and their transitive deps are no longer traversed. - Include workspace packages that omit a version (default to 0.0.0, matching the root component). - Bound workspace manifest reads with p-limit to avoid EMFILE on large monorepos.
…orkspace BFS - Throw SBOM_OUT_PATH_COLLISION when two workspace packages sanitize to the same --out path instead of silently overwriting one SBOM with another. - Replace the resolveWorkspaceDeps BFS queue.shift() (O(n) per dequeue) with a moving head index for O(n) traversal on large workspaces.
7f539fa to
6c4dca9
Compare
…workspace importers - Strip ASCII control characters (incl. newlines) in sanitizePathSegment so a crafted package name/version cannot inject lines into the --split --out summary or produce confusing filenames. - Skip walking a reachable workspace importer when its package info is missing (e.g. unreadable manifest) instead of misattributing its deps to the root. - Bound importer-walker fan-out with p-limit to avoid resource pressure on large workspaces.
|
Code review by qodo was updated up to the latest commit 6c4dca9 |
|
Code review by qodo was updated up to the latest commit 1126b25 |
…Set for path collisions - Read workspace package manifests once into SharedContext (keyed by importer id) from the project graph, so split mode no longer re-reads them from disk for every emitted SBOM. - Track written --out paths in a Set instead of Array#includes to make collision detection O(1) per package rather than O(n2) overall.
|
Code review by qodo was updated up to the latest commit de542db |
…, cache root license - Only treat %s in --out as per-package (split) mode when a workspace project graph is present; in a single-project repo %s/%v interpolate from the root component on the single-output path. Adds a regression test. - Use an own-property check for lockfile importer existence so a crafted link: target cannot follow inherited keys (e.g. toString) via the prototype chain. - Fast-path resolveRootLicense when the manifest already declares an SPDX license and cache the workspace-root license in SharedContext, avoiding redundant on-disk LICENSE probing per emitted SBOM.
|
Code review by qodo was updated up to the latest commit 7394078 |
Member
|
@coderabbitai review |
✅ Action performedReview finished.
|
zkochan
approved these changes
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Per-package SBOM generation for workspaces. Each package gets its own SBOM with the correct root component and full transitive dependency tree, including workspace inter-dependencies.
New flags
--out <path>writes SBOM to a file instead of stdout. Supports%s(package name) and%v(version) placeholders. When%sis present, generates one SBOM per workspace package:--splitoutputs one compact JSON per line (NDJSON) to stdout, for piping into tools that process multiple SBOMs:Per-package correctness
When
--filterselects a single package, the SBOM root component uses that package's name, version, description, license, and author. Fields the package doesn't define fall back to the workspace root manifest.Workspace inter-dependencies (
workspace:protocol) and their transitive external dependencies are now included in the SBOM. Dev-only workspace deps are excluded with--prod.--lockfile-onlyskips workspace dep resolution to avoid disk reads beyond the lockfile.Changes
--outand--splitflags with%s/%vplaceholder supportcompactoption on CycloneDX and SPDX serializers for NDJSON outputresolveWorkspaceDepsBFS finds workspace link deps and tracks source importer and dep typeSharedContextreads lockfile, root manifest, and store path once (shared across all packages in split mode)recursiveByDefaultso--filterworks in workspaces (bug in released pnpm, separate fix in fix(sbom): add recursiveByDefault so --filter works in workspaces #12187)Depends on #12187.
@ultrox this should cover the per-workspace SBOM use case you described. Would be great to hear if it fits your Turborepo setup.
Summary by CodeRabbit
New Features
--out(writes to disk using%s/version placeholders) and--split(generates per-workspace-package SBOMs; otherwise a single SBOM is produced).Improvements
link:dependencies and their transitive workspace impacts are included in SBOM components/relationships.Documentation
Tests
--prod,--lockfile-only, and output modes.