diff --git a/components/legacy/scope/lanes/lanes.ts b/components/legacy/scope/lanes/lanes.ts index 50892b7a11d8..d2a63334025e 100644 --- a/components/legacy/scope/lanes/lanes.ts +++ b/components/legacy/scope/lanes/lanes.ts @@ -40,9 +40,9 @@ export default class Lanes { return existingLaneHistory || emptyLaneHistory; } - async updateLaneHistory(laneObject: Lane, laneHistoryMsg?: string) { + async updateLaneHistory(laneObject: Lane, laneHistoryMsg?: string, historyKey?: string) { const laneHistory = await this.getOrCreateLaneHistory(laneObject); - await laneHistory.addHistory(laneObject, laneHistoryMsg); + await laneHistory.addHistory(laneObject, laneHistoryMsg, historyKey); return laneHistory; } diff --git a/e2e/harmony/lanes/lane-history-diff.e2e.ts b/e2e/harmony/lanes/lane-history-diff.e2e.ts index 5f3025e34d09..cd12874ff73b 100644 --- a/e2e/harmony/lanes/lane-history-diff.e2e.ts +++ b/e2e/harmony/lanes/lane-history-diff.e2e.ts @@ -49,21 +49,19 @@ describe('lane history-diff', function () { }); /** - * Simplest reproduction: snap → reset → snap → export. - * The reset deletes the Version objects but the lane-history entry from the first snap survives. - * When a fresh workspace imports this lane, the orphaned entry references versions that - * don't exist on the remote. + * Scenario: snap → reset → snap → export. + * Before the fix, reset would delete Version objects but leave the lane-history entry, + * creating an orphaned entry. Now, reset also removes the corresponding lane-history entry + * (keyed by batchId), so the history stays clean. * - * History timeline (oldest→newest): + * History timeline after reset cleanup (oldest→newest): * [0] "new lane" (empty, always available) * [1] "first snap" (exported, available) - * [2] "local snap" (orphaned - versions deleted by reset) - * [3] "final snap" (exported, available) + * [2] "final snap" (exported, available) + * (the "local snap" entry is removed by reset) */ - describe('lane history-diff with orphaned versions after snap-reset-snap', () => { - let orphanedHistoryId: string; - let finalSnapHistoryId: string; - let firstSnapHistoryId: string; + describe('lane history after snap-reset-snap (no orphaned entries)', () => { + let historyEntries: any[]; before(() => { helper.scopeHelper.setWorkspaceWithRemoteScope(); helper.fixtures.populateComponents(2); @@ -75,11 +73,7 @@ describe('lane history-diff', function () { helper.fixtures.populateComponents(2, undefined, 'v2'); helper.command.snapAllComponentsWithoutBuild('-m "local snap"'); - const historyAfterSnap = helper.command.laneHistoryParsed(); - orphanedHistoryId = historyAfterSnap[historyAfterSnap.length - 1].id; - firstSnapHistoryId = historyAfterSnap[historyAfterSnap.length - 2].id; - - // Reset → Version objects deleted, but lane-history entry survives + // Reset → both Version objects and lane-history entry are removed helper.command.resetAll(); // Snap again and export @@ -87,41 +81,34 @@ describe('lane history-diff', function () { helper.command.snapAllComponentsWithoutBuild('-m "final snap"'); helper.command.exportLane(); - const historyAfterExport = helper.command.laneHistoryParsed(); - finalSnapHistoryId = historyAfterExport[historyAfterExport.length - 1].id; - // Fresh workspace: switch to the lane helper.scopeHelper.reInitWorkspace(); helper.scopeHelper.addRemoteScope(); helper.command.switchRemoteLane('dev'); + + historyEntries = helper.command.laneHistoryParsed(); }); - it('no args: should skip the orphaned entry and diff "final snap" against "first snap"', () => { - // no args diffs latest (final snap) against predecessor. - // predecessor is the orphaned entry, so it should automatically fall back to "first snap" + it('should not have an orphaned entry in the history', () => { + // We expect 3 entries: "new lane", "first snap", "final snap". + // The "local snap" entry should have been removed by reset. + const messages = historyEntries.map((e: any) => e.message || ''); + expect(messages.some((m: string) => m.includes('local snap'))).to.be.false; + }); + + it('no args: should diff "final snap" against "first snap" without errors', () => { const output = helper.command.runCmd('bit lane history-diff'); expect(output).to.have.string('comp1'); expect(output).to.have.string('comp2'); expect(output).to.not.have.string('Diff failed'); }); - it('one arg (final snap): should skip the orphaned predecessor and diff against "first snap"', () => { - const output = helper.command.runCmd(`bit lane history-diff ${finalSnapHistoryId}`); + it('with one arg (final snap id): should diff against "first snap" without errors', () => { + const finalSnapId = historyEntries[historyEntries.length - 1].id; + const output = helper.command.runCmd(`bit lane history-diff ${finalSnapId}`); expect(output).to.have.string('comp1'); expect(output).to.have.string('comp2'); expect(output).to.not.have.string('Diff failed'); }); - - it('one arg (orphaned entry): its missing "to" versions should cause diff failures', () => { - // The orphaned entry is the "to". Its predecessor is "first snap" (available). - // But the "to" versions themselves are orphaned, so componentDiff fails for them. - const output = helper.command.runCmd(`bit lane history-diff ${orphanedHistoryId}`); - expect(output).to.have.string('Diff failed'); - }); - - it('two args (explicit): should show diff failures without fallback', () => { - const output = helper.command.runCmd(`bit lane history-diff ${firstSnapHistoryId} ${orphanedHistoryId}`); - expect(output).to.have.string('Diff failed'); - }); }); }); diff --git a/scopes/component/snapping/reset-component.ts b/scopes/component/snapping/reset-component.ts index abcfe9669f4e..78e0a9483e9b 100644 --- a/scopes/component/snapping/reset-component.ts +++ b/scopes/component/snapping/reset-component.ts @@ -1,3 +1,4 @@ +import { compact } from 'lodash'; import { BitError } from '@teambit/bit-error'; import type { ComponentID } from '@teambit/component-id'; import type { Consumer } from '@teambit/legacy.consumer'; @@ -17,6 +18,8 @@ export type ResetResult = { * we want .bitmap to have the version before the detachment. not as the head. */ versionToSetInBitmap?: string; + /** batchIds from the version objects being removed, used to clean up lane history entries */ + batchIds?: string[]; }; /** @@ -60,6 +63,17 @@ export async function removeLocalVersion( }); } + // Load version objects to extract batchIds before they are removed. + // These batchIds are used to clean up the corresponding lane history entries. + // Only needed on lanes — on main there's no lane history to clean up. + let batchIds: string[] | undefined; + if (lane) { + const versionObjects = await Promise.all( + versionsToRemoveStr.map((ver) => component.loadVersion(ver, consumer.scope.objects, false)) + ); + batchIds = [...new Set(compact(compact(versionObjects).map((v) => v.batchId)))]; + } + const headBefore = component.getHead(); await consumer.scope.sources.removeComponentVersions(component, versionsToRemove, versionsToRemoveStr, lane, head); const headAfter = component.getHead(); @@ -71,7 +85,7 @@ export async function removeLocalVersion( if (snapBeforeDetached) versionToSetInBitmap = component.getTagOfRefIfExists(snapBeforeDetached); } - return { id, versions: versionsToRemoveStr, component, versionToSetInBitmap }; + return { id, versions: versionsToRemoveStr, component, versionToSetInBitmap, batchIds }; } export async function removeLocalVersionsForAllComponents( diff --git a/scopes/component/snapping/snapping.main.runtime.ts b/scopes/component/snapping/snapping.main.runtime.ts index 926150b1aea1..2c788538a5d8 100644 --- a/scopes/component/snapping/snapping.main.runtime.ts +++ b/scopes/component/snapping/snapping.main.runtime.ts @@ -720,6 +720,18 @@ in case you're unsure about the pattern syntax, use "bit pattern [--help]"`); const isRealUntag = !soft; if (isRealUntag) { results = await untag(); + + // Remove lane history entries that correspond to the reset snaps. + // Each snap uses its batchId as the lane history key, so we can match them. + if (currentLane) { + const allBatchIds = uniq(results.flatMap((r) => r.batchIds || [])); + if (allBatchIds.length) { + const laneHistory = await consumer.scope.lanes.getOrCreateLaneHistory(currentLane); + laneHistory.removeHistoryEntries(allBatchIds); + consumer.scope.objects.add(laneHistory); + } + } + await consumer.scope.objects.persist(); const currentLaneId = consumer.getCurrentLaneId(); const stagedConfig = await this.workspace.scope.getStagedConfig(); diff --git a/scopes/component/snapping/version-maker.ts b/scopes/component/snapping/version-maker.ts index 5c243328c7e8..3ae22c4b282e 100644 --- a/scopes/component/snapping/version-maker.ts +++ b/scopes/component/snapping/version-maker.ts @@ -284,7 +284,9 @@ export class VersionMaker { if (lane) { const { message } = this.params; const msgStr = message ? ` (${message})` : ''; - const laneHistory = await this.legacyScope.lanes.updateLaneHistory(lane, `snap${msgStr}`); + // Use batchId as the lane history key so `bit reset` can identify and remove the + // corresponding entry when it deletes the snapped versions. + const laneHistory = await this.legacyScope.lanes.updateLaneHistory(lane, `snap${msgStr}`, this.batchId); this.legacyScope.objects.add(laneHistory); } } diff --git a/scopes/scope/objects/models/lane-history.ts b/scopes/scope/objects/models/lane-history.ts index 234048d9e9bb..ad50b1c36bb8 100644 --- a/scopes/scope/objects/models/lane-history.ts +++ b/scopes/scope/objects/models/lane-history.ts @@ -76,14 +76,20 @@ export class LaneHistory extends BitObject { .map(([id]) => id); } - async addHistory(laneObj: Lane, msg?: string) { + async addHistory(laneObj: Lane, msg?: string, historyKey?: string) { const log: Log = await getBasicLog(); if (msg) log.message = msg; const components = laneObj.toComponentIds().toStringArray(); const deleted = laneObj.components .filter((c) => c.isDeleted) .map((c) => c.id.changeVersion(c.head.toString()).toString()); - this.history[v4()] = { log, components, ...(deleted.length && { deleted }) }; + this.history[historyKey || v4()] = { log, components, ...(deleted.length && { deleted }) }; + } + + removeHistoryEntries(keys: string[]) { + for (const key of keys) { + delete this.history[key]; + } } merge(laneHistory: LaneHistory) {