fix: stop wiping depsRequiringBuild from the lockfile on repeat installs by zkochan · Pull Request #10551 · teambit/bit

zkochan · 2026-07-31T12:34:13Z

Summary

After the pnpm-v12/napi migration (#10508), every bit install wiped the bit.depsRequiringBuild block from pnpm-lock.yaml:

 bit:
-  depsRequiringBuild:
-    - '@apollo/protobufjs@1.2.2'
-    - '@parcel/watcher@2.5.1'
-    ...
+  depsRequiringBuild: []

Root cause: the migration dropped the returnListOfDepsRequiringBuild: true install option (the napi engine did not implement it), so installResult.depsRequiringBuild only carried blocked builds. Bit allows dependency builds to run, so the list was always empty — and lynx wrote that empty list over the recorded block on every install.

The fix:

Preserve the previously recorded list whenever the engine returns no list (depsRequiringBuild undefined): frozen-path repeat installs, and engines that predate the option. Verified against the currently pinned @pnpm/napi 12.0.0-beta.2 — a repeat install in this repo now leaves the block byte-identical.
Pass returnListOfDepsRequiringBuild: true again. Engine support was added in feat(napi): add returnListOfDepsRequiringBuild install option pnpm/pnpm#13532, so the next @pnpm/napi release reports the full list (every package whose files carry install scripts, regardless of the allow-build policy) on fresh resolves — the same semantics the pre-migration TypeScript engine had.

Written by an agent (Claude Code, claude-fable-5).

The napi migration dropped the returnListOfDepsRequiringBuild install option (the Rust engine did not support it), so installResult.depsRequiringBuild only carried blocked builds. Bit allows dependency builds to run, so every install reported an empty list and overwrote the bit.depsRequiringBuild lockfile block with []. Preserve the previously recorded list whenever the engine returns no list (frozen-path installs, or an engine predating the option), and pass returnListOfDepsRequiringBuild: true again so engines that support it report the full list on fresh resolves, matching the pre-migration TypeScript engine behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-07-31T12:34:56Z

PR Summary by Qodo

Fix lockfile depsRequiringBuild preservation on repeat installs

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Pass pnpm option to return full depsRequiringBuild on fresh resolves.
• Preserve existing lockfile bit.depsRequiringBuild when the engine omits the list.
• Prevent repeat bit install from overwriting the lockfile block with [].

Diagram

graph TD
  A["Bit install (lynx.ts)"] --> B(["@pnpm/napi install"])
  B --> C{"depsRequiringBuild present?"}
  C -->|"yes"| D["Sort + merge bit attrs"] --> E[("pnpm-lock.yaml")]
  C -->|"no"| F["Reuse existing bit attrs"] --> E
  subgraph Legend
    direction LR
    _code["Code path"] ~~~ _decision{"Decision"} ~~~ _file[("Lockfile")]
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Patch-merge lockfile YAML instead of rewriting via pnpm model

➕ More robust preservation of custom bit: extension data beyond depsRequiringBuild
➕ Less coupling to pnpm engine’s typed lockfile round-tripping behavior
➖ Higher implementation complexity and YAML edge cases
➖ Risk of diverging from pnpm’s canonical lockfile formatting/normalization

2. Derive depsRequiringBuild by scanning resolved packages for install scripts

➕ Independent of pnpm engine support/version for returnListOfDepsRequiringBuild
➕ Deterministic behavior even on frozen-lockfile paths
➖ More I/O and complexity; requires access to manifests/store metadata
➖ Harder to keep consistent with pnpm’s internal definition of build-needed deps

Recommendation: Current approach is the best trade-off: it restores the intended pnpm option when available, and prevents destructive writes by treating an omitted list as “unknown” rather than “empty”. This keeps behavior compatible across engine versions and frozen-lockfile installs while aligning semantics with the pre-migration TypeScript engine.

Files changed (1) +22 / -10

Bug fix (1) +22 / -10

lynx.tsKeep lockfile depsRequiringBuild when pnpm doesn’t report it +22/-10

Keep lockfile depsRequiringBuild when pnpm doesn’t report it

• Adds 'returnListOfDepsRequiringBuild: true' to pnpm N-API install options (with a typing bridge for older published types). Updates lockfile patching logic to only overwrite 'bit.depsRequiringBuild' when pnpm actually returns a list; otherwise it preserves the pre-install 'bit:' attributes to avoid wiping the recorded block on repeat installs.

scopes/dependencies/pnpm/lynx.ts

qodo-free-for-open-source-projects · 2026-07-31T12:37:38Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Invalid bit lockfile block 🐞 Bug ≡ Correctness ⭐ New

Description

When the install returns no depsRequiringBuild list, mergeBitLockfileAttrs() returns
preInstallAttrs unchanged; if it lacks depsRequiringBuild, addBitAttributesToLockfile() will
write a bit: block missing a required field. This creates a lockfile block that violates the
declared BitLockfileAttributes shape and may confuse/ break code that expects depsRequiringBuild
to be present.

Code

scopes/dependencies/pnpm/lynx.ts[R813-818]

+export function mergeBitLockfileAttrs(
+  preInstallAttrs: Partial<BitLockfileAttributes> | undefined,
+  sortedDepsRequiringBuild: string[] | undefined
+): Partial<BitLockfileAttributes> | undefined {
+  if (sortedDepsRequiringBuild == null) return preInstallAttrs;
+  return { ...preInstallAttrs, depsRequiringBuild: sortedDepsRequiringBuild };

Evidence

The new merge function explicitly returns preInstallAttrs unchanged when the list is uncomputed,
while the lockfile writer unconditionally casts and writes merged attrs as BitLockfileAttributes
(where depsRequiringBuild is required). This combination can serialize a bit: block without
depsRequiringBuild if the pre-existing attrs were partial.

scopes/dependencies/pnpm/lynx.ts[813-819]
scopes/dependencies/pnpm/lynx.ts[843-851]
scopes/dependencies/pnpm/lynx.ts[861-864]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`mergeBitLockfileAttrs()` can return a `preInstallAttrs` object that does not contain `depsRequiringBuild` when `sortedDepsRequiringBuild` is `undefined`. Later, `addBitAttributesToLockfile()` merges and casts this object to `BitLockfileAttributes` and writes it into `pnpm-lock.yaml`, which can produce a `bit:` block missing the required `depsRequiringBuild` array.

### Issue Context
This path is reachable specifically when the engine does not compute `depsRequiringBuild` (e.g. frozen-lockfile path / older engine) and the pre-existing `bit` block is partial (e.g. contains only `restoredFromModel`).

### Fix Focus Areas
- scopes/dependencies/pnpm/lynx.ts[813-819]
- scopes/dependencies/pnpm/lynx.ts[843-851]
- scopes/dependencies/pnpm/lynx.ts[861-864]

### Suggested fix
Ensure `depsRequiringBuild` is present whenever writing a `bit:` block, e.g.:
- In `mergeBitLockfileAttrs()`, when returning `preInstallAttrs` because `sortedDepsRequiringBuild` is `undefined`, return `undefined` unless `preInstallAttrs?.depsRequiringBuild` exists; OR
- Normalize to `{ ...preInstallAttrs, depsRequiringBuild: preInstallAttrs?.depsRequiringBuild ?? [] }` before calling `addBitAttributesToLockfile()`.
Pick the behavior that matches the intended lockfile schema (currently `BitLockfileAttributes.depsRequiringBuild` is required).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Install option ignored 🐞 Bug ≡ Correctness

Description

lynx.install() hard-codes returnListOfDepsRequiringBuild: true, so callers cannot disable this
behavior even though it is exposed as a public PackageManagerInstallOptions flag and is forwarded
by PnpmPackageManager. This breaks the expected configurability and can add unnecessary work to
installs that intentionally keep the flag off.

Code

scopes/dependencies/pnpm/lynx.ts[R419-422]

+    // Report every package with install scripts (not just blocked ones) in
+    // `depsRequiringBuild`, so the `bit:` lockfile block lists them even
+    // though Bit allows the builds to run.
+    returnListOfDepsRequiringBuild: true,

Evidence
The pnpm install option is exposed all the way at the PackageManager API layer and forwarded into
lynx.install(), but lynx.install() now always passes true to pnpm regardless of the
caller-provided value.
scopes/dependencies/pnpm/lynx.ts[231-266]
scopes/dependencies/pnpm/lynx.ts[409-423]
scopes/dependencies/dependency-resolver/package-manager.ts[141-146]
scopes/dependencies/pnpm/pnpm.package-manager.ts[194-247]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scopes/dependencies/pnpm/lynx.ts` currently sets `returnListOfDepsRequiringBuild: true` unconditionally, which overrides the caller-provided `options.returnListOfDepsRequiringBuild`.
This is a behavioral regression because the public API (`PackageManagerInstallOptions`) exposes this flag as optional, and `PnpmPackageManager` forwards it, so callers expect it to be respected.
### Issue Context
- `lynx.install()` accepts `returnListOfDepsRequiringBuild?: boolean` in its `options` parameter.
- `PnpmPackageManager.install()` forwards `installOptions.returnListOfDepsRequiringBuild` into `lynx.install()`.
- The current PR hard-codes `returnListOfDepsRequiringBuild: true` in the `installOptions` passed to `nodeApi.install()`.
### Fix Focus Areas
- scopes/dependencies/pnpm/lynx.ts[416-423]
### Suggested fix
Change the hard-coded value to honor the incoming option while keeping the desired default behavior, e.g.:
- `returnListOfDepsRequiringBuild: options.returnListOfDepsRequiringBuild ?? true`
(If the intent is to *always* force this on for all installs, then remove/deprecate the public option and stop forwarding it from `PnpmPackageManager` to avoid a misleading API.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit b63435d ⚖️ Balanced

Results up to commit 73bb07d

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Install option ignored 🐞 Bug ≡ Correctness

Description

lynx.install() hard-codes returnListOfDepsRequiringBuild: true, so callers cannot disable this
behavior even though it is exposed as a public PackageManagerInstallOptions flag and is forwarded
by PnpmPackageManager. This breaks the expected configurability and can add unnecessary work to
installs that intentionally keep the flag off.

Code

scopes/dependencies/pnpm/lynx.ts[R419-422]

+    // Report every package with install scripts (not just blocked ones) in
+    // `depsRequiringBuild`, so the `bit:` lockfile block lists them even
+    // though Bit allows the builds to run.
+    returnListOfDepsRequiringBuild: true,

Evidence
The pnpm install option is exposed all the way at the PackageManager API layer and forwarded into
lynx.install(), but lynx.install() now always passes true to pnpm regardless of the
caller-provided value.
scopes/dependencies/pnpm/lynx.ts[231-266]
scopes/dependencies/pnpm/lynx.ts[409-423]
scopes/dependencies/dependency-resolver/package-manager.ts[141-146]
scopes/dependencies/pnpm/pnpm.package-manager.ts[194-247]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`scopes/dependencies/pnpm/lynx.ts` currently sets `returnListOfDepsRequiringBuild: true` unconditionally, which overrides the caller-provided `options.returnListOfDepsRequiringBuild`.

This is a behavioral regression because the public API (`PackageManagerInstallOptions`) exposes this flag as optional, and `PnpmPackageManager` forwards it, so callers expect it to be respected.

### Issue Context
- `lynx.install()` accepts `returnListOfDepsRequiringBuild?: boolean` in its `options` parameter.
- `PnpmPackageManager.install()` forwards `installOptions.returnListOfDepsRequiringBuild` into `lynx.install()`.
- The current PR hard-codes `returnListOfDepsRequiringBuild: true` in the `installOptions` passed to `nodeApi.install()`.

### Fix Focus Areas
- scopes/dependencies/pnpm/lynx.ts[416-423]

### Suggested fix
Change the hard-coded value to honor the incoming option while keeping the desired default behavior, e.g.:
- `returnListOfDepsRequiringBuild: options.returnListOfDepsRequiringBuild ?? true`

(If the intent is to *always* force this on for all installs, then remove/deprecate the public option and stop forwarding it from `PnpmPackageManager` to avoid a misleading API.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-07-31T12:37:39Z

+    // Report every package with install scripts (not just blocked ones) in
+    // `depsRequiringBuild`, so the `bit:` lockfile block lists them even
+    // though Bit allows the builds to run.
+    returnListOfDepsRequiringBuild: true,


1. Install option ignored 🐞 Bug ≡ Correctness

lynx.install() hard-codes returnListOfDepsRequiringBuild: true, so callers cannot disable this behavior even though it is exposed as a public PackageManagerInstallOptions flag and is forwarded by PnpmPackageManager. This breaks the expected configurability and can add unnecessary work to installs that intentionally keep the flag off.

Agent Prompt

### Issue description `scopes/dependencies/pnpm/lynx.ts` currently sets `returnListOfDepsRequiringBuild: true` unconditionally, which overrides the caller-provided `options.returnListOfDepsRequiringBuild`. This is a behavioral regression because the public API (`PackageManagerInstallOptions`) exposes this flag as optional, and `PnpmPackageManager` forwards it, so callers expect it to be respected. ### Issue Context - `lynx.install()` accepts `returnListOfDepsRequiringBuild?: boolean` in its `options` parameter. - `PnpmPackageManager.install()` forwards `installOptions.returnListOfDepsRequiringBuild` into `lynx.install()`. - The current PR hard-codes `returnListOfDepsRequiringBuild: true` in the `installOptions` passed to `nodeApi.install()`. ### Fix Focus Areas - scopes/dependencies/pnpm/lynx.ts[416-423] ### Suggested fix Change the hard-coded value to honor the incoming option while keeping the desired default behavior, e.g.: - `returnListOfDepsRequiringBuild: options.returnListOfDepsRequiringBuild ?? true` (If the intent is to *always* force this on for all installs, then remove/deprecate the public option and stop forwarding it from `PnpmPackageManager` to avoid a misleading API.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Extract sortDepsRequiringBuild and mergeBitLockfileAttrs from the install body so the wipe this PR fixes is pinned by a unit test: an uncomputed list (undefined) preserves the recorded block, while a computed empty list replaces it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-07-31T13:25:52Z

+export function mergeBitLockfileAttrs(
+  preInstallAttrs: Partial<BitLockfileAttributes> | undefined,
+  sortedDepsRequiringBuild: string[] | undefined
+): Partial<BitLockfileAttributes> | undefined {
+  if (sortedDepsRequiringBuild == null) return preInstallAttrs;
+  return { ...preInstallAttrs, depsRequiringBuild: sortedDepsRequiringBuild };


1. Invalid bit lockfile block 🐞 Bug ≡ Correctness

When the install returns no depsRequiringBuild list, mergeBitLockfileAttrs() returns preInstallAttrs unchanged; if it lacks depsRequiringBuild, addBitAttributesToLockfile() will write a bit: block missing a required field. This creates a lockfile block that violates the declared BitLockfileAttributes shape and may confuse/ break code that expects depsRequiringBuild to be present.

Agent Prompt

### Issue description `mergeBitLockfileAttrs()` can return a `preInstallAttrs` object that does not contain `depsRequiringBuild` when `sortedDepsRequiringBuild` is `undefined`. Later, `addBitAttributesToLockfile()` merges and casts this object to `BitLockfileAttributes` and writes it into `pnpm-lock.yaml`, which can produce a `bit:` block missing the required `depsRequiringBuild` array. ### Issue Context This path is reachable specifically when the engine does not compute `depsRequiringBuild` (e.g. frozen-lockfile path / older engine) and the pre-existing `bit` block is partial (e.g. contains only `restoredFromModel`). ### Fix Focus Areas - scopes/dependencies/pnpm/lynx.ts[813-819] - scopes/dependencies/pnpm/lynx.ts[843-851] - scopes/dependencies/pnpm/lynx.ts[861-864] ### Suggested fix Ensure `depsRequiringBuild` is present whenever writing a `bit:` block, e.g.: - In `mergeBitLockfileAttrs()`, when returning `preInstallAttrs` because `sortedDepsRequiringBuild` is `undefined`, return `undefined` unless `preInstallAttrs?.depsRequiringBuild` exists; OR - Normalize to `{ ...preInstallAttrs, depsRequiringBuild: preInstallAttrs?.depsRequiringBuild ?? [] }` before calling `addBitAttributesToLockfile()`. Pick the behavior that matches the intended lockfile schema (currently `BitLockfileAttributes.depsRequiringBuild` is required).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-07-31T13:25:55Z

Code review by qodo was updated up to the latest commit b63435d

zkochan · 2026-07-31T13:39:25Z

Heads up on the red bit_pr: it is not caused by this PR.

The job dies at bit ci pr → install packages in 2 capsules with Received "killed" signal (an OOM kill, no stack trace). The same failure, at the same step, with the same capsule count, happens on the ci-sync branch, which contains none of these changes:

build	branch	result
432599	pull/10508	success (09:39, pre-merge)
432666	pull/10551	killed at "install packages in 2 capsules"
432690	ci-sync	killed at "install packages in 2 capsules"
432699	pull/10551	killed at "install packages in 2 capsules"

So bit_pr is currently failing for every branch based on current master, and it started somewhere between 09:39 and 12:41 today. This PR's other jobs are green, including e2e_test, lint, generate_and_check_types, check_circular_dependencies and check_generated_reference.

For what it is worth on the memory question: bit_pr runs with NODE_OPTIONS=--max-old-space-size=30000 on a 32 GB 2xlarge. Since the capsule install now runs the Rust engine in-process, its native allocations sit alongside a V8 heap that is allowed to grow to nearly the whole box before GC pressure kicks in — the same allowance-ballooning shape that was fixed for the e2e helper in #10508 via childNodeOptions(), which capped spawned children at 2048.

I have deliberately not retuned that job here, since picking the number needs data on the step's real peak heap and it is unrelated to the lockfile fix in this PR. Happy to dig into the memory profile in a separate PR if that is useful.

Nothing in this PR reaches that path: on the pinned @pnpm/napi 12.0.0-beta.2 the new returnListOfDepsRequiringBuild option is an unknown field that the binding ignores, and the lockfile write now happens less often than before (it is skipped when the engine reports no list).

Written by an agent (Claude Code, claude-fable-5).

qodo-free-for-open-source-projects Bot reviewed Jul 31, 2026

View reviewed changes

GiladShoham approved these changes Jul 31, 2026

View reviewed changes

zkochan mentioned this pull request Jul 31, 2026

feat(napi): add returnListOfDepsRequiringBuild install option pnpm/pnpm#13532

Merged

3 tasks

qodo-free-for-open-source-projects Bot reviewed Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: stop wiping depsRequiringBuild from the lockfile on repeat installs - #10551

fix: stop wiping depsRequiringBuild from the lockfile on repeat installs#10551
zkochan wants to merge 2 commits into
teambit:masterfrom
zkochan:fix-deps-requiring-build-wipe

zkochan commented Jul 31, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot Jul 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Jul 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026

Uh oh!

zkochan commented Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zkochan commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

qodo-free-for-open-source-projects Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot commented Jul 31, 2026

Uh oh!

zkochan commented Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented Jul 31, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jul 31, 2026 •

edited

Loading