Skip to content

fix(isolator): keep capsule optimization dependency-closed - #10613

Open
luvkapur wants to merge 3 commits into
masterfrom
agent/fix-capsule-optimization-boundary
Open

fix(isolator): keep capsule optimization dependency-closed#10613
luvkapur wants to merge 3 commits into
masterfrom
agent/fix-capsule-optimization-boundary

Conversation

@luvkapur

@luvkapur luvkapur commented Aug 13, 2026

Copy link
Copy Markdown
Member

Proposed Changes

  • Make capsule optimization dependency-aware: a component can use its published package only when its entire dependency subtree is also safe to consume as packages.
  • Keep package-only subtrees optimized while retaining mixed-generation dependency paths as capsules.
  • Add a registry-backed constructor-identity regression and strengthen bit ci pr --keep-lane coverage for unchanged components.

Closes #10583.

Problem

Capsule optimization, introduced in #9820, avoids creating capsules for exported, built, and unmodified dependencies by installing their published packages instead.

The previous decision was made independently for each component:

If this component is published and unmodified, use its package.

That establishes package availability, but not whether the resulting dependency graph is safe.

A dependency still needs a capsule when it is:

  • new or not exported;
  • modified locally, even when an older version was published;
  • a seeder or original seeder that is part of the current build;
  • exported without a successful build;
  • not configured as an installable package; or
  • a snap that built successfully but was never published.

This allowed a graph such as:

A (registry package) ──> B (published package copy)
                             +
                         B (current capsule copy)

If B had to remain a capsule, the published A could still resolve its recorded package copy of B, while another path loaded the current B capsule. JavaScript classes and module state are identified by runtime instance, not package name or semantic version. Two copies can therefore break instanceof, Harmony singletons, shared remotes, caches, and error recovery based on constructors.

bit ci pr --keep-lane exposed this consistently because a reused lane can preserve artifacts from an earlier run while later runs rebuild only changed components.

Fix

The package side of the optimized graph is now required to be dependency-closed:

If A is installed as a package, every transitive dependency of A must also be safe to install as a package.

The isolator now:

  1. Applies the existing eligibility rules:
    • seeders, modified components, and non-installable components start in the capsule set;
    • published, built, unmodified, installable components start as package candidates.
  2. Walks dependency edges backwards from the capsule set.
  3. Promotes any package candidate that depends on a capsule into the capsule set.
  4. Repeats until no package-to-capsule edge remains.

For example:

A ──> B ──> C
            ^
       must be a capsule

C causes B to remain a capsule, which causes A to remain a capsule. Conversely, a subtree that is fully resolvable from the registry remains optimized:

A (package) ──> B (package) ──> C (package)

The closure is computed in O(V + E) time using a reverse dependency map and a work queue.

Behavioral Impact

  • Mixed graphs may create more capsules than before. This is the correctness cost of keeping one coherent module generation.
  • Registry-only dependency subtrees keep the original optimization.
  • Capsule optimization remains enabled globally.
  • Lane reuse remains enabled; this does not force every component on a reused lane to rebuild.
  • Capsule selection gains linear graph traversal and O(V + E) temporary bookkeeping.

Regression Coverage

The registry-backed regression constructs:

comp1 -(dev)-> comp2 -(prod)-> comp3 -(prod)-> comp4
  |                                              ^
  `--------------------(dev)---------------------'

comp4 is modified and must remain a capsule. Before the fix, registry-installed comp3 loaded its package copy of comp4, while comp1 loaded the current capsule. Constructor identity failed with:

Expected constructor: SharedClass
Received constructor: SharedClass

After the fix, the capsule requirement propagates through comp3 and comp2, and the test passes. The existing optimization test also proves that a registry-only subtree is still pruned.

The keep-lane regression now verifies that a second PR run reuses the remote lane while preserving the exact head of an unchanged component.

Validation

  • Identity regression: fails before the fix and passes after it.
  • Capsule optimization matrix: 3 passing, including the registry-only optimization case.
  • Keep-lane reuse suite: 4 passing, including exact unchanged-head preservation.
  • Focused Checkout/Lanes build: 18 passing, 2 pending.
  • pnpm exec tsc --noEmit.
  • oxlint: 0 warnings and 0 errors.
  • Prettier and git diff --check.

Circle validation used two source-identical runs:

  • Fresh-lane control: bit_pr #440098 passed.
  • Reused-lane run: an empty commit triggered bit_pr #440134, which explicitly logged that the existing remote lane was being reused and completed successfully.

@luvkapur

Copy link
Copy Markdown
Member Author

Circle validation completed:

  • Fresh-lane control: bit_pr #440098 passed.
  • Reuse test: pushed empty commit 84e3bc730 (no source changes) and ran bit_pr #440134.
  • The reuse run explicitly logged: Lane teambit.bit/agent-fix-capsule-optimization-boundary exists on remote, reusing it.
  • The command completed with PR command executed successfully; Circle reports the bit_pr check passed.

This isolates lane reuse as the changed condition while keeping the patch source-identical between the two runs.

@luvkapur
luvkapur marked this pull request as ready for review August 13, 2026 17:38
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix capsule optimization by enforcing a dependency-closed package boundary

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Enforce dependency-closed capsule/package boundaries to prevent duplicate runtime module
 instances.
• Add registry-backed e2e regression covering constructor identity across mixed package/capsule
 graphs.
• Strengthen bit ci pr --keep-lane test to ensure unchanged component heads remain stable.
Diagram

graph TD
  A["Isolator createGraph"] --> B["ComponentIdGraph"] --> C["filterUnmodifiedExportedDependencies"]
  C --> D["Dependency-closed capsule set"] --> F["Create/link capsules"]
  C --> E["Registry package candidates"] --> G["Install packages"]
  H["E2E regressions"] --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Disable optimization for any mixed capsule/package graph
  • ➕ Simpler reasoning: always isolate full closure when any capsule is required
  • ➕ Avoids all runtime duplication risks by construction
  • ➖ Potentially large performance regression (more capsules/builds) even when most of the graph is registry-safe
  • ➖ Negates the original optimization benefits for common cases
2. Force single-instance resolution via overrides/aliasing (npm overrides, pnpm hooks)
  • ➕ Could keep more packages while preventing duplicate dependency instances
  • ➖ Tooling-specific and brittle across package managers and lockfile states
  • ➖ Hard to guarantee for transitive/peer/dev edges and for Bit capsule linking semantics

Recommendation: Keep the PR’s approach: computing a dependency-closed package boundary via a reverse-dependency work queue is the minimal correctness fix that preserves the existing optimization for registry-only subtrees while preventing mixed-generation module identity issues. The added E2E regressions directly cover the previously unsafe scenario and lane reuse behavior.

Files changed (3) +129 / -11

Bug fix (1) +51 / -8
isolator.main.runtime.tsMake capsule optimization dependency-closed using reverse dependency propagation +51/-8

Make capsule optimization dependency-closed using reverse dependency propagation

• Updates 'filterUnmodifiedExportedDependencies' to accept the dependency subgraph and split components into required-capsule vs package-candidate sets. Enforces a dependency-closed package boundary by walking reverse edges from the capsule set and promoting any package candidate that depends on a capsule, then filters components accordingly to prevent mixed package/capsule graph instances at runtime.

scopes/component/isolator/isolator.main.runtime.ts

Tests (2) +78 / -3
build-cmd.e2e.tsAdd regression for package-to-capsule boundary causing constructor identity split +59/-0

Add regression for package-to-capsule boundary causing constructor identity split

• Adds an npm-registry-backed E2E that constructs a dependency graph where a registry-installed package would otherwise depend on a locally-capsuled component. Verifies the isolator keeps the transitive dependent (comp3) in the capsule graph to avoid loading two copies of the same class constructor.

e2e/harmony/build-cmd.e2e.ts

ci-commands.e2e.tsStrengthen keep-lane reuse test to assert unchanged component head stability +19/-3

Strengthen keep-lane reuse test to assert unchanged component head stability

• Extends the lane-reuse E2E by capturing a remote lane head for an unchanged component after the first 'bit ci pr --keep-lane' run and asserting it remains identical after a subsequent PR commit that changes a different component. This guards against inadvertent re-snapping/rebuilding of unaffected components when reusing lanes.

e2e/harmony/ci-commands.e2e.ts

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Versionless closure conflation ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new dependency-closure propagation keys nodes by toStringWithoutVersion(), so different
versions of the same component collapse to one ID and can incorrectly promote/keep otherwise-safe
package candidates as capsules. In graphs that contain multiple versions of the same component, this
can defeat the optimization and change which versions end up being isolated/linked.
Code

scopes/component/isolator/isolator.main.runtime.ts[R1787-1790]

+      const sourceId = graph.node(edge.sourceId)?.attr?.toStringWithoutVersion();
+      const targetId = graph.node(edge.targetId)?.attr?.toStringWithoutVersion();
+      if (!sourceId || !targetId) return;
+      const dependents = dependentsByDependencyId.get(targetId) ?? new Set<string>();
Evidence
The closure traversal builds its reverse dependency map and propagates capsule membership using
toStringWithoutVersion(), which necessarily merges different versions into one key. The same file
documents that versions should not be ignored when building the component list, implying
multi-version situations are possible and should remain version-accurate during filtering.

scopes/component/isolator/isolator.main.runtime.ts[457-488]
scopes/component/isolator/isolator.main.runtime.ts[1709-1813]

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

## Issue description
`filterUnmodifiedExportedDependencies()` computes dependency-closure by storing capsule/package candidates as **versionless** strings (via `toStringWithoutVersion()`). If the graph contains multiple versions of the same component, the closure computation can incorrectly treat them as the same node and propagate capsule status across versions.
### Issue Context
The same file explicitly notes that versions matter in the graph-building path (to avoid mixing multiple versions). The new closure algorithm reintroduces version-collapsing during propagation.
### Fix Focus Areas
- scopes/component/isolator/isolator.main.runtime.ts[1709-1813]
### Implementation guidance
- Use version-qualified IDs for `capsuleIds`, `packageCandidateIds`, `dependentsByDependencyId` keys, and `capsuleQueue` (e.g., `component.id.toString()` and `graph.node(...).id` / `attr.toString()`).
- If you still need "ignoreVersion" semantics for seeders/originalSeeders checks, keep those separate (only for membership checks), but keep the closure traversal version-accurate.
- Add/adjust a regression that includes two versions of the same component in the graph to ensure the optimization still behaves as intended.

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


Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous review results

Review updated until commit bf274b5 ⚖️ Balanced

Results up to commit 84e3bc7


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


Remediation recommended
1. Versionless closure conflation 🐞 Bug ≡ Correctness
Description
The new dependency-closure propagation keys nodes by toStringWithoutVersion(), so different
versions of the same component collapse to one ID and can incorrectly promote/keep otherwise-safe
package candidates as capsules. In graphs that contain multiple versions of the same component, this
can defeat the optimization and change which versions end up being isolated/linked.
Code

scopes/component/isolator/isolator.main.runtime.ts[R1787-1790]

+      const sourceId = graph.node(edge.sourceId)?.attr?.toStringWithoutVersion();
+      const targetId = graph.node(edge.targetId)?.attr?.toStringWithoutVersion();
+      if (!sourceId || !targetId) return;
+      const dependents = dependentsByDependencyId.get(targetId) ?? new Set<string>();
Evidence
The closure traversal builds its reverse dependency map and propagates capsule membership using
toStringWithoutVersion(), which necessarily merges different versions into one key. The same file
documents that versions should not be ignored when building the component list, implying
multi-version situations are possible and should remain version-accurate during filtering.

scopes/component/isolator/isolator.main.runtime.ts[457-488]
scopes/component/isolator/isolator.main.runtime.ts[1709-1813]

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

### Issue description
`filterUnmodifiedExportedDependencies()` computes dependency-closure by storing capsule/package candidates as **versionless** strings (via `toStringWithoutVersion()`). If the graph contains multiple versions of the same component, the closure computation can incorrectly treat them as the same node and propagate capsule status across versions.

### Issue Context
The same file explicitly notes that versions matter in the graph-building path (to avoid mixing multiple versions). The new closure algorithm reintroduces version-collapsing during propagation.

### Fix Focus Areas
- scopes/component/isolator/isolator.main.runtime.ts[1709-1813]

### Implementation guidance
- Use version-qualified IDs for `capsuleIds`, `packageCandidateIds`, `dependentsByDependencyId` keys, and `capsuleQueue` (e.g., `component.id.toString()` and `graph.node(...).id` / `attr.toString()`).
- If you still need "ignoreVersion" semantics for seeders/originalSeeders checks, keep those separate (only for membership checks), but keep the closure traversal version-accurate.
- Add/adjust a regression that includes two versions of the same component in the graph to ensure the optimization still behaves as intended.

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


Qodo Logo

Comment thread scopes/component/isolator/isolator.main.runtime.ts Outdated
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

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

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.

bit ci pr --keep-lane: lane reuse mixes artifact generations in capsules, deterministically failing core-component specs from the second push

1 participant