Skip to content

fix(ci): restore lane config changes during ci merge to main#10219

Open
davidfirst wants to merge 3 commits intomasterfrom
fix/restore-lane-config-on-ci-merge
Open

fix(ci): restore lane config changes during ci merge to main#10219
davidfirst wants to merge 3 commits intomasterfrom
fix/restore-lane-config-on-ci-merge

Conversation

@davidfirst
Copy link
Member

When bit ci merge runs on a workspace checked out to a lane, config changes (from bit env set, bit deps set, etc.) were lost during the switch to main.

The fix compares each lane component's Version extensions with its main Version extensions. Any differences are saved to .bitmap before tagging, so config changes survive the lane-to-main transition.

Test plan

  • Added e2e test: bit ci merge when lane has config changes (env-set)
  • Verifies env config set on lane is preserved after merge
  • Verifies component is tagged and exported successfully
  • Verifies status is clean after merge

When `bit ci merge` switches from a lane to main, config changes made via
`bit env set` or `bit deps set` were lost. Now compares lane Version
extensions with main Version extensions and saves differences to .bitmap
before tagging.
Copilot AI review requested due to automatic review settings March 6, 2026 17:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes bit ci merge behavior when the workspace is checked out to a lane so that lane-only component config changes (e.g. from bit env set, bit deps set) are not lost during the switch back to main before tagging.

Changes:

  • Capture lane components before switching to main, then restore lane-vs-main extension config differences into .bitmap after checkout head.
  • Add an e2e test covering bit ci merge preserving an env-set config change made on a lane.

Reviewed changes

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

File Description
scopes/git/ci/ci.main.runtime.ts Restores lane config changes into .bitmap during CI merge by diffing lane vs main Version extension configs.
e2e/harmony/ci-commands.e2e.ts Adds an e2e scenario asserting env config is preserved after bit ci merge.

You can also share your feedback on Copilot code review. Take the survey.

- Collapse redundant configDiff intermediate object into single-pass loop
- Flatten deep nesting for mainConfig loading with optional chaining
- Parallelize component processing with Promise.all
- Add missing test assertions for remote export and lane cleanup
Copilot AI review requested due to automatic review settings March 6, 2026 21:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +432 to +459
describe('bit ci merge when lane has config changes (env-set)', () => {
let mergeOutput: string;
before(() => {
helper.scopeHelper.setWorkspaceWithRemoteScope();
setupGitRemote();
const defaultBranch = setupComponentsAndInitialCommit();

// Create lane and change env on comp1
helper.command.createLane('config-lane');
helper.command.setEnv('comp1', 'teambit.harmony/aspect');
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();

// Create git branch, commit, and merge to default branch
helper.command.runCmd('git checkout -b feature/config-change');
helper.command.runCmd('git add .');
helper.command.runCmd('git commit -m "feat: env config change on lane"');

helper.command.runCmd(`git checkout ${defaultBranch}`);
helper.command.runCmd('git merge feature/config-change');

// Run bit ci merge
mergeOutput = helper.command.runCmd('bit ci merge');
});
it('should preserve the env config from the lane', () => {
const envData = helper.command.showAspectConfig('comp1', 'teambit.envs/envs');
expect(envData.config.env).to.equal('teambit.harmony/aspect');
});
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This e2e covers env-set preservation, but the new restoration logic should also preserve lane-side config removals (e.g. bit env unset), which follows a different code path (aspect entries disappear rather than change). Consider adding a companion e2e scenario that unsets the env on the lane and asserts the config is removed after bit ci merge, to prevent regressions once removal handling is added.

Copilot uses AI. Check for mistakes.
Comment on lines +744 to +750

for (const [aspectId, config] of Object.entries(laneConfig)) {
if (!isEqual(config, mainConfig[aspectId])) {
this.workspace.bitMap.addComponentConfig(laneComp.id, aspectId, config as Record<string, any>);
hasChanges = true;
}
}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restoreLaneConfigChanges() only iterates over laneConfig entries, so it can add/overwrite aspect configs but it can’t represent aspect-level removals done on the lane (e.g. bit env unset removes teambit.envs/envs and the env aspect entry). After checkout head, main’s aspect config will remain in .bitmap, so the lane’s removal won’t survive the lane→main transition. Consider also diffing keys present in mainConfig but missing from laneConfig and removing them from .bitmap (e.g. via bitMap.removeComponentConfig(...) or addComponentConfig(..., undefined as any)), so unsets are preserved too.

Suggested change
for (const [aspectId, config] of Object.entries(laneConfig)) {
if (!isEqual(config, mainConfig[aspectId])) {
this.workspace.bitMap.addComponentConfig(laneComp.id, aspectId, config as Record<string, any>);
hasChanges = true;
}
}
// Apply additions/overwrites from the lane config
for (const [aspectId, config] of Object.entries(laneConfig)) {
if (!isEqual(config, mainConfig[aspectId])) {
this.workspace.bitMap.addComponentConfig(laneComp.id, aspectId, config as Record<string, any>);
hasChanges = true;
}
}
// Also apply removals: aspects present on main but missing from the lane
const removedAspects = Object.keys(mainConfig).filter((aspectId) => !(aspectId in laneConfig));
for (const aspectId of removedAspects) {
if (typeof (this.workspace.bitMap as any).removeComponentConfig === 'function') {
(this.workspace.bitMap as any).removeComponentConfig(laneComp.id, aspectId);
} else {
// Fallback for BitMap implementations without removeComponentConfig
this.workspace.bitMap.addComponentConfig(laneComp.id, aspectId, undefined as any);
}
hasChanges = true;
}

Copilot uses AI. Check for mistakes.
Comment on lines +730 to +735
const laneVersion = (await repo.load(laneComp.head)) as Version;
if (!laneVersion) return;

const laneConfig = laneVersion.extensions.toConfigObject();
if (!laneConfig || Object.keys(laneConfig).length === 0) return;

Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repo.load() returns null when an object is missing (unless throws=true). Here, missing lane Version objects are silently skipped (if (!laneVersion) return;), which can lead to config restoration not happening with no signal in CI logs. Consider using repo.load(laneComp.head, true) (fail fast) or logging a warning when a referenced Version can’t be loaded.

Suggested change
const laneVersion = (await repo.load(laneComp.head)) as Version;
if (!laneVersion) return;
const laneConfig = laneVersion.extensions.toConfigObject();
if (!laneConfig || Object.keys(laneConfig).length === 0) return;
const laneVersion = (await repo.load(laneComp.head, true)) as Version;
const laneConfig = laneVersion.extensions.toConfigObject();
if (!laneConfig || Object.keys(laneConfig).length === 0) return;

Copilot uses AI. Check for mistakes.
Comment on lines +460 to +465
it('should tag and export successfully', () => {
expect(mergeOutput).to.include('Merged PR');
const list = helper.command.listParsed();
const comp1 = list.find((comp) => comp.id.includes('comp1'));
expect(comp1).to.exist;
expect(comp1?.currentVersion).to.equal('0.0.2');
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test title should tag and export successfully doesn’t actually verify export (export is asserted in the next it). To avoid misleading/fragile intent, either rename this it to only mention tagging, or move an export assertion into this block.

Copilot uses AI. Check for mistakes.
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.

2 participants