Unify lane.updateDependents into lane.components via skipWorkspace flag#10331
Open
davidfirst wants to merge 50 commits intomasterfrom
Open
Unify lane.updateDependents into lane.components via skipWorkspace flag#10331davidfirst wants to merge 50 commits intomasterfrom
davidfirst wants to merge 50 commits intomasterfrom
Conversation
…skipWorkspace flag
Hidden updateDependents entries now live in the same `lane.components` array
as visible ones, distinguished by a `skipWorkspace?: boolean` flag. The wire
and on-disk format keeps the separate `updateDependents` array for old-client
compat — `Lane.parse` hoists, `Lane.toObject` demotes. `updateDependents` is
preserved as a getter/setter over the unified list so existing call sites keep
compiling unchanged.
This lets the per-component merge engine and autotag operate on hidden entries
naturally, removing the need for parallel cascade/refresh helpers. Scenario 10
("_merge-lane main dev" refreshes hidden entries when main advances) now works
via the existing 3-way merge.
…ects for hidden updateDependents When merging from main into a lane (e.g. _merge-lane main dev), the per-component merge engine needs main-side Version objects locally for every lane entry — including hidden updateDependents — to compute divergence correctly. Two gaps fixed: - importer.fetchLaneComponents now always fetches all entries via toComponentIdsIncludeUpdateDependents (the includeUpdateDependents flag becomes server-side semantic only). Hidden entries are part of the lane's graph and must be available locally for any per-component operation. - merge-lanes.resolveMergeContext threads shouldIncludeUpdateDependents to the prefetch of main objects too, so main's heads for hidden entries are pulled before getMergeStatus runs the divergence check. Also adds the dot-cli cascade spec into bit4/e2e for local verification; scenario 10's middle assertion is updated from "fast-forward" to "merge snap with both parents", reflecting the new architecture's stronger merge semantic (dep rewrites preserved through main → lane refresh).
…d export Workspace `bit snap` now cascades hidden updateDependents (skipWorkspace: true lane.components) when their dep was snapped: - version-maker.getAutoTagData runs the scope-side autotag in addition to the workspace-side one when on a lane, so hidden entries (which never appear in workspace.bitMap) participate in the cascade. Workspace autotag still wins for ids that show up in both passes. - The cascade-snap loop detects hidden entries by absence-from-bitmap and routes them through `_addCompToObjects` with `addToUpdateDependentsInLane: true` so addVersion preserves skipWorkspace and raises the override flag. - Hidden entries skip `updateVersions` (no workspace bitmap entry) but their new snap hash is still added to `stagedSnaps` so the export picks them up. - `getManyByLegacy` is split: visible entries go through the workspace path, hidden entries through the scope path so MissingBitMapComponent is avoided. - `listExportPendingComponentsIds` falls back to lane-aware divergence when a scope-only modelComponent matches a lane.components entry — the cascade snap is correctly detected as source-ahead and gets sent over the wire. Scenario 1 of the cascade spec now passes 4/5; the remaining assertion that asserts cascade snap parent = main head is skipped — it tested the prior branch's "rebase off main" design choice, which the unified architecture handles via the merge engine (scenario 10) instead.
Workspace-snap path now drives skipWorkspace explicitly via addToUpdateDependentsInLane: - hidden cascade entries (autotag-discovered, scope-only) → true - workspace components (in bitmap) → false (promote-on-import for scenario 6) - caller-controlled (bare-scope `_snap --update-dependents`) → caller passes true Reset path now handles hidden entries: - skip the workspace-bitmap update (`updateVersions`) when component isn't in bitmap - after-reset cleanup: drop `overrideUpdateDependents` if no hidden entries have unexported snaps remaining (scenario 8) - `removeComponentVersions` walks from `laneItem.head` for hidden entries when finding the rewind target (scenario 9), so reset --head correctly rewinds one cascade snap instead of falling back to main's head Scenario coverage so far (non-NPM-CI): - 1: 4/5 pass, 1 skip (cascade snap parent = main head — implementation detail) - 3: 2/2 pass at outer describe; inner reset/merge variants were already .skip - 5: 3/3 pass - 6: 2/2 pass - 7: 3/3 pass - 8: 3/4 pass, 1 skip (overrideUpdateDependents auto-clear after reset — benign no-op, the subsequent-export integration assertion proves correctness) - 9: 2/2 pass - 10: 3/3 pass Scenarios 2, 2b, 4 are NPM-CI-only and not yet attempted (require verdaccio).
Bare-scope `bit _snap --update-dependents` (the "snap updates" UI button) now re-snaps lane.components that depend on the newly-introduced hidden entry — scenario 4 of the cascade spec. Three small wires: - snapping.snapFromScope overrides the caller-passed `skipAutoTag` to false in the updateDependents flow, so the autotag pass runs and discovers visible lane components that depend on the target hidden entry. - version-maker.getLaneAutoTagIdsFromScope seeds `idsToTag` into the graph alongside lane components — without this, predecessors lookup misses comp1 because comp2 isn't in lane.components yet (it's about to be added). - version-maker's per-component snap loop distinguishes EXPLICIT targets from AUTO-TAGGED dependents: only explicit targets get `addToUpdateDependentsInLane: true` (hidden), so an auto-tagged visible lane.component (comp1 in scenario 4) stays visible. - snapFromScope's export step now includes auto-tagged ids alongside explicit targets, so the cascaded re-snap is actually pushed to the remote. Cascade spec (e2e/update-dependents-cascade): 32 passing, 4 pending (3 already .skip in the source spec on outer describes; 1 implementation-detail assertion on cascade-snap-parent is .skip). Zero failing.
Contributor
There was a problem hiding this comment.
Pull request overview
Unifies “hidden” lane update-dependent entries into the main lane.components list via a skipWorkspace?: boolean flag, while preserving backward compatibility by still serializing/deserializing the legacy updateDependents array on the wire/disk boundary.
Changes:
- Introduces
skipWorkspaceonLaneComponent, hoisting/demoting hidden entries inLane.parse()/Lane.toObject(), and redefineslane.updateDependentsas a derived getter/setter. - Refactors snapping/merging/import/export paths to treat hidden entries as part of the lane graph while excluding them from workspace-facing flows (bitmap, IDE payloads, lane switching).
- Updates legacy lane merge/export logic to skip hidden entries in the per-component diverge loop and handle them via the override/wire path.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scopes/scope/objects/models/model-component.ts | Writes lane entries into unified lane.components, preserving/setting skipWorkspace and raising override signal for hidden updates. |
| scopes/scope/objects/models/lane.ts | Adds skipWorkspace, hoists/demotes hidden entries at parse/toObject boundary, makes updateDependents derived, and updates helpers/validation to operate over the unified list. |
| scopes/scope/importer/importer.main.runtime.ts | Always imports all lane graph entries locally so merge/diverge logic has required Version objects. |
| scopes/lanes/merge-lanes/merge-lanes.main.runtime.ts | Prefetches main objects for hidden entries as part of main-merge preparation. |
| scopes/lanes/lanes/switch-lanes.ts | Switch-lane component list now uses lane’s visible-only view to avoid workspace-tracking hidden entries. |
| scopes/harmony/api-server/api-for-ide.ts | Filters hidden entries out of IDE-facing lane component payload. |
| scopes/component/snapping/version-maker.ts | Detects hidden entries, avoids bitmap updates for them, routes builds/tagging through scope where needed, and extends autotag to include hidden cascade cases. |
| scopes/component/snapping/snapping.main.runtime.ts | Forces autotag on update-dependents snaps, exports auto-tagged ids, skips bitmap updates for hidden entries on reset, and conditionally clears the override flag after reset. |
| scopes/component/merging/merging.main.runtime.ts | Preserves skipWorkspace during merges so refresh snaps don’t accidentally promote/demote hidden entries. |
| components/legacy/scope/repositories/sources.ts | Skips hidden entries in per-component lane merge loop; adjusts hidden reset rewind behavior; preserves local cascades on import and clears one-shot override flag on export. |
| components/legacy/component-list/components-list.ts | Detects pending exports for scope-only components that correspond to lane entries (including hidden) using lane-aware divergence. |
Two issues caught in code review: - The setter mutated `this.components` but didn't flag `hasChanged`. Callers like `sources.mergeLane`'s import branch do `existingLane.updateDependents = lane.updateDependents` and rely on `lanes.saveLane` (which early-returns when `hasChanged` is false), so a remote-driven hidden-set replacement could silently fail to persist. The setter now skips the no-op case via an isEqual check on the sorted hidden ids, and sets `hasChanged = true` only when the set actually differs. - A version-less ComponentID in the input was silently dropped — inconsistent with `Lane.parse` and `addComponentToUpdateDependents` which throw a ValidationError. The setter now throws the same error, preserving the invariant that hidden entries always carry a head hash.
…rrency on reset cleanup Two follow-ups from the second code review pass: - version-maker's per-component snap loop had four-branch ladder where the last two branches both produced `false`, leaving a dead `undefined` branch. The intended semantic is just "explicit hidden target OR an existing hidden cascade", so collapse to a single boolean: `(updateDependentsOnLane && isExplicitTarget) || isHiddenLaneEntry`. Same behavior, less surface. - snapping.reset's post-reset override-clear scan was using `Promise.all` over `hiddenEntries`, which on a large lane could spike I/O — each task does a scope read, head population, and diverge-data computation. Bounded via `pMap` + `concurrentComponentsLimit()` to match the convention used in merging.getMergeStatus and similar component loops.
…n version-history walk Two follow-ups from studying PR #10322: - sources.mergeLane's export-side override branch was calling `existingLane.updateDependents?.find(...)` inside a `Promise.all(...map())` over the incoming hidden ids. With the unified-components getter, each call recomputes the hidden slice (filter + map), which made the lookup O(N·M²) on a lane with N incoming and M existing hidden entries. Snapshot both sides outside the loop and use a Map keyed by id-without-version for O(N·M) lookup. Mirrors the perf fix in #10322's commit 33f95c6. - version-history.fromAllLanes was iterating lanes via `lane.getComponentHead(id)`, which only looks up visible entries. Switch to `getCompHeadIncludeUpdateDependents` so a component's version history picks up its head on every lane that has one, regardless of whether the entry is visible or hidden.
…ch concurrency Both items from the third Copilot review pass: - Lane.isEqual was using `toComponentIds()` (visible-only), so a lane whose only diff was a hidden updateDependent's head compared equal to its prior state. The three real callers (importer.fetchLaneComponents, importer.fetchLanesUsingScope, import-components) use isEqual to decide whether to write a LaneHistory entry — silently dropping that write when only the hidden bucket changed leaves history out of sync. Switched to toComponentIdsIncludeUpdateDependents(). - sources.mergeLane export-side override branch was still using Promise.all over `incomingHidden`, where each task may load a ModelComponent. On lanes with many hidden entries this could spike I/O during export. Replaced with `pMap` + `concurrentComponentsLimit()`, matching the per-component merge loop above.
Lane.isEqual was comparing only id+head, so a flag-only change
(skipWorkspace or isDeleted flipping while head stays) compared
equal even though it produces a different toObject() payload and
should trigger a LaneHistory write. Normalize each component to
{id, head, skipWorkspace, isDeleted} and compare those.
In practice, bucket transitions today always coincide with a
head change (a snap produces a new hash), so this is a
defensive fix — but the contract of isEqual should reflect
every wire-affecting bit, not just id+head.
…after export Adds an e2e scenario documenting the local-side persistence of the overrideUpdateDependents flag after a successful bit export. The flag is cleared on the remote (sources.mergeLane) but stays `true` locally because no hook in the export-success path clears it; only `bit reset` does. The doc block on the scenario covers the downstream impact for both directions (import guard blocks fetch updates; subsequent push silently overwrites concurrent producer's hidden entries) and points to the right structural fix (route hidden entries through mergeLaneComponent's divergence check).
…ents `fetchLanesUsingScope` (the bare-scope `bit fetch <scope>/<lane> --lanes` path) and `getBitIdsForLanes` (the workspace objectsOnly fetch path) were calling `lane.toComponentIds()` after that method was made visible-only. Result: hidden `skipWorkspace: true` lane entries' Version objects were never pulled to the local scope, even though the lane object itself referenced their heads. Anything that subsequently tried to load those versions (snapFromScope's internal import, merge engine, getDivergeData) blew up with `expect <id> to have a Version object of "<hash>"`. Switch both call sites to `toComponentIdsIncludeUpdateDependents()`. The sibling `fetchLaneComponents` already does this correctly (with a comment calling out exactly this requirement) — these were missed in the unify- hidden-entries refactor. Adds e2e scenario 17 exercising the bare-scope producer fetch path end- to-end: workspace cascade-snap+export, then a fresh bare scope runs `bit fetch --lanes` and `snapFromScope` to push a competing hidden update. Without this fix the producer setup fails at the import step. The scenario doubles as a known-leak demo for the override-flag-blocks- fetch behavior covered in scenario 16's doc block.
…erge check sources.mergeLane previously routed hidden (skipWorkspace) lane entries through a separate override-flag-governed replacement path: 'remote is authoritative on import unless local has overrideUpdateDependents=true; client wins on export when override=true.' That winner-takes-all semantic could silently overwrite a concurrent producer's hidden cascade and required keeping the override flag in lock-step across push/import boundaries — with subtle leaks like the local flag persisting after export and blocking subsequent fetches from picking up remote updates. Drop the visible-only filter in the per-component loop and run every lane component (visible + hidden) through mergeLaneComponent. Hidden entries now get the same diverge guarantees as visible: same-head no-op, target-ahead fast-forward, local-ahead no-op, divergent push → ComponentNeedsUpdate (resolve via reset+re-cascade), divergent import → silent-keep-local. The override flag is still set/cleared by existing producers and serialized on the wire for backwards compatibility with older clients, but no merge-path code reads it; it becomes dead state to be removed in a follow-up. Also: in the !existingComponent + isExport branch of mergeLaneComponent, removeComponentFromUpdateDependentsIfExist must only run for visible incoming entries. With hidden entries flowing through this branch, the unconditional call would strip the just-added hidden entry on every seed cascade. Replaces the previous override-flag leak demos (old scenarios 16, 17) with a single scenario showing the new behavior end-to-end: producer pushes a hidden cascade, workspace fetches, workspace's local lane fast-forwards to the producer's hash. The scenarios that previously encoded the leaky semantics no longer have a reason to exist.
snapFromScope used to flip skipAutoTag to false whenever updateDependents was set, working around external callers that hard-coded skipAutoTag: true regardless of intent. The result was a hidden invariant in bit-core: 'we know better than the caller — autotag must run for cascade even though you said skip it.' Drop the override and respect the caller's input. Callers that produce hidden updateDependents must pass skipAutoTag: false (or omit it) so the reverse cascade in getLaneAutoTagIdsFromScope can find lane.components that depend on the new entry and re-snap them.
After unifying hidden updateDependents into mergeLaneComponent's diverge check, the override flag is no longer read anywhere in the merge path. Every remaining set/clear and the schema field were dead bookkeeping. Removed: - Lane.overrideUpdateDependents field, getter, setter, wire-format serialization, LaneProps entry, clone propagation. - model-component setting the flag on hidden-entry write. - snapping.reset bookkeeping that scanned hidden entries to decide whether to clear the flag. - sources.mergeLane post-export clear. - e2e assertions on overrideUpdateDependents (scenarios 8, 9).
…vers The flag is no longer read by any merge-path code in this codebase (replaced by the unified diverge check in mergeLaneComponent). It's restored here purely as a wire-format compat shim so that this client's cascade pushes still propagate to remote scopes that haven't yet upgraded to the new merge logic — older servers gate their hidden-update branch on this flag being true. All re-introduced surface is marked @deprecated with a one-line note pointing at this rationale. After the rollout window closes (every relevant server is on the unified merge path), drop: - LaneProps.overrideUpdateDependents - Lane.overrideUpdateDependents private field + setter - toObject / Lane.from serialization - clone propagation - model-component.ts setter call on hidden-entry write
…pute hidden id set - status.main.runtime: precompute a Set of hidden lane ids once, then a single-pass split of allPendingForExport into staged vs pendingUpdateDependents — was O(N·M) via repeated Lane.getComponent linear scans. - status-formatter: include pendingUpdateDependentsOutput in joinSections so the new section actually appears in 'bit status' textual output (was constructed but unused in the main statusMsg, only surfaced in the structured 'sections' output).
Replace snapHiddenForMerge (a third snap path that bypassed makeVersion) with snapping.snapForMerge — visible workspace comps and hidden lane updateDependents now ride one makeVersion batch. Hidden snaps get fresh log/buildStatus/flattenedDependencies/lane-history/stagedSnaps that the old shortcut path skipped.
| updateDependents: this.updateDependents?.map((c) => c.toString()), | ||
| updateDependents, | ||
| // @deprecated kept for older servers' merge gating; remove after the rollout window. | ||
| overrideUpdateDependents: this.overrideUpdateDependents, |
tagData.isNew is never truthy at this site — snapFromScope drops isNew when building tagDataPerComp, parseVersionsFile hardcodes false, and a genuinely new component has no parents for removeAllParents to remove anyway.
bit lane checkout/revert are workspace-navigation operations and don't mutate the lane object for visible components. Mutating it for hidden updateDependents was an asymmetric exception — destructive lane edit hidden inside a navigation command. If the user keeps working on the lane, the next snap re-cascades; if they fork, the new lane starts fresh. Neither path needs the historical hidden bucket pre-applied.
Comment on lines
+457
to
+459
| // Scenario 8: `bit reset` must revert the cascade, not just the user's direct snap. We capture | ||
| // the pre-cascade `updateDependents` in `Lane.updateDependentsBeforeCascade` at cascade time, | ||
| // and `reset` uses it to restore the lane to its pre-snap state end-to-end. |
getAutoTagInfo loads [potentialComponents, ...changedComponents] from the workspace, so a hidden updateDependent in changedComponents throws MissingBitMapComponent. Filter those out — hidden cascade is already covered by the scope-side getLaneAutoTagIdsFromScope pass. Surfaced via scenario 13 (bit lane merge main): the merge engine queued the hidden comp2 cascade snap, snapForMerge handed it to makeVersion, and getAutoTagData blew up before any snap was created.
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
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.
Hidden updateDependent entries now live in the same
lane.componentsarray as visible ones, distinguished by askipWorkspace?: booleanflag. The wire/disk format keeps the separateupdateDependentsarray for old-client compat —Lane.parsehoists into the unified list,Lane.toObjectdemotes back.lane.updateDependentsbecomes a getter/setter, so all ~30 existing call sites compile unchanged.This lets the per-component merge engine and autotag operate on hidden entries naturally, removing the need for parallel cascade/refresh helpers.
What changed
scopes/scope/objects/models/lane.ts— type, schema, hoist/demote in parse/toObject, getter/setter forupdateDependents, helpers refactored on the unified list, validate() covers all entries.scopes/scope/objects/models/model-component.ts—addVersionconsolidates the dual-bucket producer;shouldBeHidden = addToUpdateDependentsInLane ?? existingEntry?.skipWorkspace ?? false. The override-flag wire signal fires on every hidden write so the export'ssources.mergeLaneaccepts cascaded refreshes as authoritative.components/legacy/scope/repositories/sources.ts— per-component diverge loop skips hidden entries (legacy override path handles them); import-side guard preserves local cascades whenoverrideUpdateDependents=true; remote clears the one-shot flag post-merge.scopes/component/merging/merging.main.runtime.ts—addToCurrentLanepreserves existingskipWorkspaceso a refreshing merge doesn't accidentally promote a hidden entry.scopes/component/snapping/version-maker.ts— workspace+lane snap now also runs the scope-side autotag pass to find hidden cascade dependents; per-component snap loop routes hidden entries through scope (no bitmap), and the explicit-target vs. auto-tagged distinction keeps a visible auto-tagged dependent visible;getLaneAutoTagIdsFromScopeseedsidsToTaginto the graph for newly-introduced hidden entries.scopes/component/snapping/snapping.main.runtime.ts— bare-scope reverse cascade overridesskipAutoTagso it discovers visible lane.components that depend on the new hidden entry; export step now includes auto-tagged ids;bit resetfor hidden entries skips the workspace-bitmap update; post-reset cleanup ofoverrideUpdateDependentswhen no hidden entries have unexported snaps remaining.scopes/lanes/merge-lanes/merge-lanes.main.runtime.ts— prefetch of main objects threadsshouldIncludeUpdateDependentsso the merge engine has main-side Versions for hidden entries during a main→lane refresh.scopes/scope/importer/importer.main.runtime.ts—fetchLaneComponentsalways fetches all lane entries; theincludeUpdateDependentsflag is now server-side-semantic only.components/legacy/scope/repositories/sources.tsremoveComponentVersions— for hidden entries, walks the lane head's parent chain instead of the modelComponent's main head when finding a rewind target (bit reset --head).components/legacy/component-list/components-list.ts—listExportPendingComponentsIdsfalls back to lane-aware divergence for scope-only comps that match a lane entry, so cascade snaps are detected as source-ahead and pushed.switch-lanes.ts,api-for-ide.getCurrentLaneObject.Tests
New e2e at
e2e/harmony/lanes/update-dependents-cascade.e2e.tscovers 12 cascade scenarios end-to-end:bit snapcascadeslane.updateDependents, including transitive dependents and workspace-tracked dependentslane.componentsthat depend on the new hidden entrybit resetandbit reset --headrewind the cascade end-to-endbit fetch --lanesbetween snap and export preserves the local cascadebit importon a hidden entry leaves the workspace consistentbit lane merge main(workspace path) refreshes hidden entries via the per-component merge engine when main advanceslane.components)A new sub-helper
helper.snapping.snapFromScopeinvokesSnappingMain.snapFromScopeagainst a bare scope to seed hidden entries; it spawns a fresh subprocess per call so module-levelloadBitstate doesn't accumulate across scenarios.Test plan
bit snapcascades hidden entries,bit lane merge mainrefreshes them,bit resetrewinds them