feat(ci): opt-in --keep-lane to reuse lane across PR commits + adopt-on-conflict#10388
Merged
Conversation
…rent runs bit ci pr now reuses the existing remote lane on subsequent commits to the same PR, preserving lane history and any cloud-side edits. When two CI runners race the first push, the loser fetches the winner's lane, rebases its snapped Version parents onto the remote heads, and re-exports. - snapPrCommit: switch-to-existing-lane + merge-main-into-lane (brings forward config-only changes like deps/env set that landed on main during the PR). - writeObjectsToTheFS: validate LaneId uniqueness before writing files so a rejected concurrent push leaves no stale objects on the remote. - export.pushToRemotesCarefully: clean up pending dirs on persist failure so a retry isn't blocked by the export queue.
…d-rebase-on-conflict
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the bit ci pr flow to preserve and reuse a PR’s remote lane across subsequent commits, and adds a conflict-recovery path for concurrent CI exports so racing runners can converge on a single remote lane without forcing a costly re-snap/build.
Changes:
- Reuse existing remote PR lanes (instead of delete/recreate) and merge
maininto the PR lane to propagate non-git-tracked config changes. - Add “adopt-on-conflict” recovery for concurrent lane export hash mismatches by rebasing local snapped
Versionparents onto the remote lane heads and retrying export. - Add early LaneId uniqueness validation before writing objects to disk, and improve export pending-dir cleanup on persist failures.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scopes/scope/objects/objects/scope-index.ts | Adds a pre-write LaneId uniqueness validator and strengthens lane indexing against same-id/different-hash races. |
| scopes/scope/objects/objects/repository.ts | Reloads scope-index and runs LaneId uniqueness validation before persisting lane-related objects. |
| scopes/scope/export/export.main.runtime.ts | Ensures pending dirs are cleaned up when persistRemotes() fails to avoid stuck export queues. |
| scopes/git/ci/ci.main.runtime.ts | Reuses remote lanes across PR commits, merges main into PR lanes, and implements adopt-on-conflict export recovery for concurrent runs. |
| e2e/harmony/ci-commands.e2e.ts | Adds/updates E2E coverage for lane reuse, main→lane config propagation, and concurrent CI runner behavior. |
- scope-index: validateLaneIdUniqueness and addOne now also reject the rename case where the target LaneId already belongs to a different lane (different hash). - export: wrap pending-dir cleanup in try/catch so a cleanup failure logs a warning instead of masking the original persist error. - ci: add TODO documenting the older-runner-wins regression case for the adopt-on-conflict rebase.
GiladShoham
approved these changes
May 26, 2026
…rategy 'ours') When mergeMainIntoLane hits a bit-level component conflict (main and the lane both modified the same component), keep the lane's (PR's) version. File-level git conflicts are the user's to resolve before ci pr runs; once resolved, the workspace already reflects the PR author's intent, and bit shouldn't override that with main's content. Also adds an e2e test covering the scenario.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
scopes/scope/objects/objects/scope-index.ts:173
- This error message concatenation results in "(...). this typically indicates..." with a lowercase “this” after a period. Please capitalize it ("(...). This typically indicates...") for correct grammar and readability.
throw new Error(
`unable to add lane "${bitObject.toLaneId().toString()}" to the scope index. ` +
`a lane with the same id already exists with a different hash ` +
`(existing hash: ${sameLaneId.hash}, incoming hash: ${hash}). ` +
`this typically indicates a concurrent push race — retry the operation.`
);
- preserve merge-snap parents (and sync VersionHistory) when rebasing snaps onto the adopted remote lane, instead of collapsing parents to [remoteHead] - fetch the remote lane before dropping the local one, so a network fetch failure leaves the workspace on its valid local lane rather than orphaning the current-lane pointer - rethrow real switchToLane failures (cleanup path stays best-effort) instead of silently continuing into mergeMainIntoLane/snap on the wrong lane - make the concurrent-runner e2e assertion deterministic: a loser may recover via rebase OR via lane-reuse depending on scheduling, both are valid
…lane flow) The reuse-the-same-lane redesign is a big behavioral change for a high-blast- radius command, so gate it behind a flag instead of flipping the default: - default 'bit ci pr' restores the proven temp-lane flow verbatim (snap on a throwaway temp lane, delete+recreate the final lane at export, retry on hash-mismatch) - '--keep-lane' opts into the new flow: reuse/create the lane, merge main into it, snap, and export with adopt-on-conflict recovery — preserving lane history and cloud UI edits across PR commits - split snapPrCommit into snapAndExportWithTempLane (default) and snapAndExportReusingLane (--keep-lane); restored exportWithRetryOnLaneHashMismatch - the scope-index / repository / export robustness fixes stay unconditional — they harden concurrent-push races in both flows - the reuse/merge-main/conflict/concurrent e2e tests now pass --keep-lane
The bit_pr job runs the bit built from the branch checkout, so this exercises the new reuse-lane flow on bit's own PR pipeline.
- capitalize 'This' in the two scope-index lane-conflict error messages - use logger.consoleWarning (not warn) for the export pending-dir cleanup failure so it surfaces to the user - correct mergeMainIntoLane doc comment: it always runs/logs the merge (0 components when nothing to bring forward), it does not skip silently
…lane) In the --keep-lane flow, only merge main into the lane when the PR branch is up to date with the default branch. If it's behind (hasn't pulled main's latest), the git checkout still reflects the older fork point, so pulling main's newer bit state into the lane would desync it from the source — defer to the next run after the author merges the default branch in. The check is fetch-free: it compares HEAD against the already-local origin/<default> ref via 'merge-base --is-ancestor', avoiding the SSH host-key prompt hang that got isStaleCiRun removed (#10300). On any uncertainty it proceeds with the merge so config propagation is never silently disabled. Adds an e2e test for the behind-PR case.
mergeMainIntoLane used 'bit lane merge', which refuses to run while the workspace has code-modified components — but in 'bit ci pr' the workspace is always dirty (the PR's not-yet-snapped changes), so it aborted with 'component is modified, please snap/tag it first'. That's the bit_pr dogfood failure. In this workflow git is the source of truth for files (the author merges the default branch into their PR). The only thing git can't carry is config already tagged into objects on main (another PR's 'bit env set' / 'bit deps set'). So replace the full merge with syncConfigFromMain: a per-component 3-way merge of aspect *config only* (ComponentConfigMerger; base=common ancestor, ours=lane, theirs=main; 'ours' on conflict), stashed on an unmergedComponents.mergedConfig entry that the subsequent snap bakes in. No file checkout -> no clean-workspace requirement; source files stay git's job. - clear the component cache after writing entries so snap reloads aspects - drop the now-unused merge-lanes dep; add config-merger / snap-distance / dependency-resolver - keep the up-to-date gate (skip the sync when the PR branch is behind)
- validateLaneIdUniqueness: look up the same-hash entry among lanes only, so a rare hash collision with a ComponentItem can't throw on toLaneId() and mask the real LaneId-collision error. - syncConfigFromMain: remove any existing unmergedComponents entry before addEntry (which throws on duplicates), so a stale entry from a crashed prior run doesn't make every later run skip and serve stale config.
…t persist on failure The earlier try/catch around persistRemotes removed pending dirs after a failure, which broke two pre-existing safety invariants: 1. Pending dirs act as a cross-client lock via export-validate's waitIfNeeded queue (sorted by clientId/Date.now). Removing them lets a second client race in against a partial cross-scope commit (scopeA persisted, scopeB failed) and corrupt cross-scope dependencies. 2. Pending dirs are the substrate for 'bit export --resume <clientId>'. Removing them strands the original user with no objects to resume from. The motivating concern (stranded pending dir blocking the next bit ci pr retry) was misdiagnosed: the concurrent-push race for bit ci pr surfaces in validateRemotes via mergeLane's LANE_HASH_MISMATCH, and validateRemotes already cleans up on its own failure path.
…heFS The reload was unnecessary: every remote-action entry point goes through scope-api/lib/action.ts → loadScope, which initializes the Repository from disk; on a long-running server, the Scope-aspect watcher (watchSystemFiles) keeps the cached scope's in-memory index in sync with concurrent writers. So by the time validateLaneIdUniqueness runs, the in-memory scopeIndex is already fresh enough to detect a same-id/different-hash conflict. Keep the validateLaneIdUniqueness pre-write check (cheap, prevents orphan Version files on a rejected push) and the addOne guard (defense in depth).
The validateLaneIdUniqueness pre-write check (in writeObjectsToTheFS) and the new same-id/different-hash branch in addOne are both shadowed by the pre-existing sources.mergeLane guard at line 622: every concurrent same-id push hits mergeLane during the export-validate step (gated by the pending-dir queue's waitIfNeeded), which throws long before persist's writeObjectsToTheFS runs. The test-failure stack traces I saw all pointed at sources.mergeLane, never at the new guards. Revert both to master and update the inline comment in rebaseOntoRemoteLane to reference the actual mechanism.
The fs:// remote in tests doesn't behave like production: each remote action calls loadScope on entry but holds the resulting scope object through waitIfNeeded. The losing runner's mergeLane then reads against a scope cached BEFORE the winner persisted, misses the conflict, and lets the loser overwrite the winner's lane on persist. In production, the long-running bit-hub server keeps the cached scope fresh via watchSystemFiles, so mergeLane catches the conflict every time. Use HttpHelper here so the test exercises the same setup. Also fix the 'exactly one lane' filter — file:// flattens to l.name but http:// keeps l.id.name; accept either.
…nt push on an existing lane Covers two gaps the user flagged: 1. Existing concurrent-runners test (lane doesn't exist) — add a nested 'after the race, the lane on the remote is healthy' describe that imports the lane to a fresh workspace and asserts: snaps from both runners are on the lane, no staged/orphaned components, the lane object loads with both components, each component's VersionHistory loads with well-formed entries. Catches a broken parent pointer or VersionHistory after the in-place mutate in rebaseOntoRemoteLane. 2. New scenario: 'bit ci pr with concurrent runners adding snaps to an already-existing remote lane'. Initial bit ci pr creates the lane; two runners then race on a second commit. Both reuse the existing remote lane (no lane-hash mismatch, no rebase path), and the conflict surfaces in sources.mergeLane's per-component diverge check instead. Uses independent components (no comp1->comp2->comp3 chain) so a touch on comp2 doesn't auto-tag comp1 — without that, both runners snap comp1 with different hashes and the loser hits ComponentNeedsUpdate. Local full ci-commands suite: 63 passing (6m).
The recovery only caught LANE_HASH_MISMATCH, so it only ever fired when the
lane DIDN'T exist on the remote (both runners minted random sha1(v4()) hashes
via Lane.create). When the lane DOES exist and both runners snap the SAME
component with DIFFERENT content, the conflict instead surfaces in
sources.mergeLane's per-component diverge check, which throws
MergeConflictOnRemote('merge error occurred when exporting the component(s)…').
That propagated as a hard failure with 'please re-import the above components'.
Widen exportWithAdoptOnConflict's catch to recognize the per-component
divergence marker and route it through the same rebaseOntoRemoteLane path: the
loser's snap gets its first parent re-pointed to the winner's head, then
re-exported. Lane ends up with the loser's snap as the head and the winner's
snap preserved in history (last-writer-wins on content, both snaps preserved).
Add a new e2e describe that fails on master / fix branch without this change
and passes with it — both runners modify the same comp1 file with different
content; the loser hits the divergence error, recovers via rebase, re-exports
cleanly. Local full ci-commands suite: 67 passing (6m).
Comment on lines
+418
to
+423
| const mergedConfig = configMerger.merge().getSuccessfullyMergedConfig(); | ||
| if (!mergedConfig || !Object.keys(mergedConfig).length) continue; | ||
|
|
||
| // Strip dependency deletion markers (version: '-'); the aspects-merger applies mergedConfig | ||
| // as-is, so a leftover '-' would land in the policy. | ||
| this.filterDeletedDependenciesFromConfig(mergedConfig); |
2 tasks
davidfirst
added a commit
that referenced
this pull request
May 29, 2026
## Summary Surfaced by #10397's `bit_pr` failure. When `--keep-lane` (from #10388) found the lane on the remote, it called `switchToLane` and assumed that landed us on the lane. If the stored lane was stale relative to the current PR — typically referencing a `ModelComponent` the PR has since removed/renamed, surfaced as `unable to merge lane …, the component … was not found` — the switch failed (`switchToLane` logs-and-swallows the underlying error), the post-switch guard then threw, and every subsequent `bit ci pr --keep-lane` on that branch wedged the same way until the lane was manually deleted on the remote. ## Fix When we observe that we did not land on the requested lane after switching, delete the stale remote lane and create a fresh one (same name) — the same shape the `else` branch already takes when the lane doesn't exist on the remote. Lane history for the contested run is lost, but the next run preserves history again and CI is no longer blocked.
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.
Adds an opt-in
--keep-laneflag tobit ci pr. With the flag, subsequent commits to the same PR reuse the existing remote lane instead of deleting and recreating it — preserving lane history, lane-based cloud UI edits, and avoiding archived-lane pileup. Without the flag the behavior is unchanged:bit ci prkeeps the proven temp-lane flow (snap on a throwaway lane, delete + recreate the final lane at export). Gating it behind a flag de-risks rollout of a big behavioral change on a high-blast-radius command.--keep-laneflowbit lane merge main. Source files are git's job — the PR branch's checkout already carries them, and any file conflict between main and the PR is resolved by the author in git before re-running CI. What git can't carry is config (bit deps set/bit env set) that other PRs tagged into bit's objects on main: that config lives inside the Version object under.bit/objects(not git-tracked), and once it's tagged the entry is cleared from.bitmap(which is git-tracked) — so a plaingit checkoutof a long-running PR branch sees stale config relative to main. A full lane-merge here would be both heavy (capsule installs, builds) and wrong (it'd try to reconcile file-level snaps that git already handled, surfacing avoidable conflicts). The sync walks each lane component, diffs main's head against the lane head withComponentConfigMerger(3-way aspect-config merge,mergeStrategy: 'ours'to keep the PR's config on a true conflict), and stages the merged config as anunmergedComponentsentry. The upcomingsnapreads those entries via the existing aspects-merger path and produces a merge snap whose second parent links to main's head — the same shapebit lane mergewould have produced, just for the bits that actually need merging.git fetchin CI — that's what hung on the SSH host-key prompt in fix(ci): remove isStaleCiRun to prevent ci pr hang on SSH host-key prompt #10300) and reads the already-localorigin/<default>ref.sources.mergeLanedoesn't trip on the same-id/different-hash guard), fetches the winner's lane, and rebases its snapped Versions onto the remote heads — both runners' snaps end up chained on one lane, no costly re-snap with--build. The rebase re-points only the lane-lineage parent (preserving merge-snap ancestry) and keeps the localVersionHistoryin sync so the re-export's diverge computation stays correct.CI
bit_prCircleCI job now runsbit ci pr --build --keep-lane, dogfooding the new flow on bit's own repo.Test plan
bit ci pr workflow— default (temp-lane) flow still passes.bit ci pr --keep-lane reuses the existing remote lane across subsequent PR commits.bit ci pr --keep-lane merges main into the lane—bit deps setandbit env seton main flow onto the lane.bit ci pr --keep-lane does NOT merge main when the PR branch is behind— config change is not pulled in; logs the skip.bit ci pr --keep-lane keeps the PR's content when main and lane both modified the same component(mergeStrategy: 'ours').bit ci pr --keep-lane with concurrent runners— two cloned workspaces race against an HTTP-served bare scope (so the long-running server'swatchSystemFileskeeps the cached scope-index fresh between concurrent persists, mirroring production); the conflict is handled via rebase or lane-reuse, both snaps chained on a single lane.ci-commands.e2e.tssuite passes locally (53/53).