fix(deps-graph): preserve workspace lockfile entries on reimport

[zkochan]

Summary

• Re-importing a component into a populated workspace used to overwrite pnpm-lock.yaml with only the imported component's subgraph. pnpm then re-resolved every other workspace dep from the manifest specifiers and silently drifted to newer registry versions.
• This PR makes PnpmPackageManager.dependenciesGraphToLockfile merge the graph-derived subset into the existing wanted lockfile instead of overwriting it.
• Also fixes an in-place mutation in Scope.getDependenciesGraphByComponentIds that corrupted the cached Version._dependenciesGraph.
• Adds e2e coverage for the drift scenarios, 3-way merge, and dev/optional dep round-trip.

Context / full analysis: #10317.

Changes

scopes/dependencies/pnpm/pnpm.package-manager.ts

dependenciesGraphToLockfile now reads the existing wanted lockfile and overlays the graph's importers / packages / snapshots onto it. The importer overlay merges per-importer, per-dep-type, per-dep-key — which keeps unrelated projects' existing deps intact even though convertGraphToLockfile emits a sparse importer entry for every workspace project.

components/legacy/scope/scope.ts

getDependenciesGraphByComponentIds clones the first loaded graph via serialize/deserialize before merging subsequent graphs into it. Version.loadDependenciesGraph caches the graph on the object, so merging into it in place was mutating the cache and leaking merged state to later callers.

e2e/harmony/deps-graph.e2e.ts

Four new describe blocks:

1. importing a component into a workspace that already has an installed component — asserts the existing component's deps aren't re-resolved after importing a second component.
2. re-importing an updated version of an already-imported component — asserts unrelated components' deps aren't re-resolved on update.
3. three components sharing a peer dependency — 3-way merge, highest-wins on the root edge. Includes one assertion documenting a known remaining limitation where lower-version non-peer transitives stay in the lockfile.
4. dev and optional dependencies round-trip through the graph.

Test plan

• All 20 cases in e2e/harmony/deps-graph.e2e.ts pass locally.
• npm run lint and npm run oxlint clean.
• CI green.

Known follow-ups (tracked in #10317, not in this PR)

• Transitive snapshot reconciliation on merge (lower-version non-peer packages still linger after a 3-way merge).
• Coverage for: missing graph Source, patchedDependencies, overrides, directory-type resolutions, specifier drift between tag and import.
• Duplicate root edges accumulated by DependenciesGraph.merge (cosmetic today).

[Copilot]

Pull request overview

Adds e2e coverage for dependency-graph-based lockfile restoration edge cases and introduces initial fixes to prevent lockfile drift and cached graph mutation during re-import/merge flows.

Changes:

• Add new e2e scenarios covering re-import drift, 3-way graph merge behavior, and dev/optional dependency flag round-tripping.
• Update pnpm lockfile restoration to merge graph-derived lockfile content into an existing wanted lockfile instead of overwriting it.
• Prevent mutation of cached DependenciesGraph instances when merging graphs in Scope.getDependenciesGraphByComponentIds.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
scopes/dependencies/pnpm/pnpm.package-manager.ts	Merges graph-derived lockfile data into an existing lockfile to avoid dependency drift on subsequent imports.
e2e/harmony/deps-graph.e2e.ts	Adds e2e coverage for re-import drift reproduction and regression scenarios around graph merge/flags.
components/legacy/scope/scope.ts	Avoids mutating cached dependency graphs by cloning before merge.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

components/legacy/scope/scope.ts:759

• getDependenciesGraphByComponentIds() mutates allGraph from within a Promise.all(componentIds.map(async ...)) loop. Because the callbacks run concurrently, multiple iterations can observe allGraph == null and assign it, causing some graphs to be dropped (last writer wins) instead of merged. Collect the per-component graphs first (e.g. const graphs = await Promise.all(...)), then merge them deterministically in a single synchronous loop (or iterate for...of with await).

  public async getDependenciesGraphByComponentIds(componentIds: ComponentID[]): Promise<DependenciesGraph | undefined> {
    let allGraph: DependenciesGraph | undefined;
    if (!isFeatureEnabled(DEPS_GRAPH)) return undefined;
    await Promise.all(
      componentIds.map(async (componentId) => {
        const graph = await this.getDependenciesGraphByComponentId(componentId);
        if (graph == null || graph.isEmpty()) return;
        if (allGraph == null) {
          // loadDependenciesGraph caches the graph on the Version object; merging into
          // it in place would mutate the cached instance and corrupt subsequent callers.
          allGraph = DependenciesGraph.deserialize(graph.serialize());
        } else {
          allGraph.merge(graph);
        }
      })
    );

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

components/legacy/scope/scope.ts:762

• getDependenciesGraphByComponentIds uses pMapPool(..., { concurrency: concurrentComponentsLimit() }) but mutates the shared allGraph inside the concurrent mapper. With concurrency > 1, multiple tasks can observe allGraph == null and overwrite it, or interleave allGraph.merge(graph) calls, producing a nondeterministic/incomplete merged graph. Consider collecting the per-component graphs concurrently (e.g., return each graph from the mapper) and then merging them sequentially in a deterministic order, or guard the allGraph assignment/merge with a mutex (or run this mapper with concurrency 1 just for the merge step).

    await pMapPool(
      componentIds,
      async (componentId) => {
        const graph = await this.getDependenciesGraphByComponentId(componentId);
        if (graph == null || graph.isEmpty()) return;
        if (allGraph == null) {
          // loadDependenciesGraph caches the graph on the Version object; merging into
          // it in place would mutate the cached instance and corrupt subsequent callers.
          allGraph = DependenciesGraph.deserialize(graph.serialize());
        } else {
          allGraph.merge(graph);
        }
      },
      { concurrency: concurrentComponentsLimit() }

[Copilot]

mergeEntryRecords() only shallow-merges each packages[...] / snapshots[...] entry ({ ...existingEntry, ...graphEntry }). If a nested object exists on both sides (notably resolution in packages[...]), the graph value replaces the entire nested object and can drop fields that the graph doesn't round-trip (e.g. tarball), even though the goal here is to preserve pnpm-managed metadata. Consider doing a targeted nested merge for known nested objects (e.g. resolution: { ...existingEntry.resolution, ...graphEntry.resolution }) and/or using a deep-merge strategy that keeps existing nested keys when graph omits them.