diff --git a/e2e/harmony/ci-sync.e2e.ts b/e2e/harmony/ci-sync.e2e.ts index 797ced629d78..c3f07054a039 100644 --- a/e2e/harmony/ci-sync.e2e.ts +++ b/e2e/harmony/ci-sync.e2e.ts @@ -1120,6 +1120,258 @@ describe('bit ci sync', function () { }); }); + // A real scope carries components with tag blockers (e.g. circular dependencies). This pair is + // untouched in the strongest sense: no diff at all, so it is never part of the run's tag-pending + // set, and its blocker must not halt the run. Contrast the drift cell below, where a pair IS + // pending (a dep-only diff) and its blocker is tolerated instead of avoided. + describe('a snap-blocking issue on a component the lane never touches', () => { + const LANE = 'clean-lane'; + let defaultBranch: string; + let devPath: string; + + before(() => { + ({ defaultBranch } = setupSyncWorkspace({ lanes: ['*'] })); + // a circular pair on main: a tag blocker on components no lane sync will ever snap + helper.fs.outputFile('comp3/index.js', `require('@${helper.scopes.remote}/comp4');`); + helper.fs.outputFile('comp4/index.js', `require('@${helper.scopes.remote}/comp3');`); + helper.command.addComponent('comp3'); + helper.command.addComponent('comp4'); + helper.command.install(); + helper.command.tagAllWithoutBuild('--ignore-issues="CircularDependencies"'); + helper.command.export(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "add a circular pair to main"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + devPath = createLaneWithSnap(LANE, { 'comp1/index.js': comp1Src('lane-snap-1') }, 'lane snap 1'); + seedSync(LANE); + branchSideCommit(LANE, defaultBranch, 'comp1/index.js', comp1Src('dev-commit-1'), 'dev commit on comp1'); + }); + + it('snaps the dev commit onto the lane although the untouched pair has a tag blocker', () => { + const { output, exitCode } = syncRun(LANE); + expect(exitCode, `bit ci sync output:\n${output}`).to.equal(0); + expect(output).to.not.include('Workspace status verification failed'); + expect(output).to.include(`${LANE} -> export-branch`); + expect(laneTipFile(devPath, 'comp1/index.js')).to.include('dev-commit-1'); + }); + }); + + // The nothing-pending short-circuit (executeExportBranch) still writes the sync-ledger commit, or + // a docs-only commit (touches no bit-tracked file) would leave `stateCommit` (sync-state.ts, derived + // from `.bitmap`'s content, never commit messages) stuck behind it forever — `hasDevCommits` would + // stay true on every future run. The reconciler settles instead by recognizing its OWN tip: once + // that ledger commit is the branch's tip, a later run stops before redoing any export-branch work. + describe('a commit that touches no bit-tracked file settles instead of looping', () => { + const LANE = 'docs-only-lane'; + let defaultBranch: string; + let devPath: string; + + before(() => { + ({ defaultBranch } = setupSyncWorkspace({ lanes: ['*'] })); + devPath = createLaneWithSnap(LANE, { 'comp1/index.js': comp1Src('lane-snap-1') }, 'lane snap 1'); + seedSync(LANE); + branchSideCommit(LANE, defaultBranch, 'NOTES.md', '# notes\n', 'docs: add notes'); + }); + + it('reports nothing to snap once, then settles — the second run does not redo export-branch work', () => { + const first = syncRun(LANE); + expect(first.exitCode, `bit ci sync output:\n${first.output}`).to.equal(0); + // pins the noop summary's exact wording + expect(first.output).to.include( + `${LANE} -> export-branch (branch ${LANE} has no bit-tracked change; nothing snapped onto lane` + ); + + const second = syncRun(LANE); + expect(second.exitCode, `bit ci sync output:\n${second.output}`).to.equal(0); + // pins the settled summary's exact wording + expect(second.output).to.include( + `${LANE} -> noop (converged; branch tip is already this reconciler's own sync commit)` + ); + // executeExportBranch's own work (the checkout, the snap attempt) never ran a second time + expect(second.output).to.not.include('Exporting branch'); + }); + + // The withhold settles; it does not trap. A real dev commit on top of the recognized ledger tip + // must still clear it and export normally — the tip is no longer the reconciler's own commit. + it('a real dev commit on top of the settled tip clears the withhold and exports again', () => { + branchSideCommit( + LANE, + defaultBranch, + 'comp1/index.js', + comp1Src('dev-commit-after-settle'), + 'dev commit after settling' + ); + const { output, exitCode } = syncRun(LANE); + expect(exitCode, `bit ci sync output:\n${output}`).to.equal(0); + expect(output).to.include('Exporting branch'); + expect(output).to.include(`${LANE} -> export-branch`); + expect(output).to.not.include('branch tip is already this reconciler'); + expect(laneTipFile(devPath, 'comp1/index.js')).to.include('dev-commit-after-settle'); + }); + }); + + // This reproduces an engine bump with one bit binary: a committed root policy moves a recorded + // package range. Lanes carry the fan-out (spec decision 6): the lane run snaps the drifted + // component together with the git-authored change, in one snap under the developer's own + // message, and reports it as a surfaced side effect — git does not show this change, the run + // log and the PR report do. + describe('dependency-context drift rides the lane snap as a surfaced side effect', () => { + const LANE = 'drift-lane'; + let defaultBranch: string; + let devPath: string; + + before(() => { + ({ defaultBranch } = setupSyncWorkspace({ lanes: ['*'] })); + // A second file makes comp2 a multi-file component: the drift run's file-content compare + // must hash every file, not just one, to see that neither one changed. + helper.fs.outputFile('comp2/utils.js', `module.exports = () => 'comp2-util';\n`); + helper.fs.outputFile( + 'comp2/index.js', + `require('is-odd');\nrequire('./utils');\nmodule.exports = () => 'comp2: with-pkg';\n` + ); + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '1.0.0' } }); + helper.command.install(); + helper.command.tagAllWithoutBuild(); + helper.command.export(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "comp2 records is-odd 1.0.0"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + // Create the lane before the policy bump, or the dev's own (unscoped) `bit snap` sweeps the + // change in too, leaving nothing for `bit ci sync` to detect as drift. + devPath = createLaneWithSnap(LANE, { 'comp1/index.js': comp1Src('lane-snap-1') }, 'lane snap 1'); + seedSync(LANE); + branchSideCommit(LANE, defaultBranch, 'comp1/index.js', comp1Src('dev-commit-1'), 'dev commit on comp1'); + // `bit ci sync` resolves the workspace policy/engine aggregate once, at boot on the default + // branch; `Workspace._reloadConsumer` (used on later branch checkouts) reloads the consumer + // and bitmap but not this. Bumping the policy here after the lane forks is the engine-bump + // analogue: the same drift on an untouched component that a real engine bump produces. + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '3.0.1' } }); + helper.command.install(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "bump is-odd policy (engine-bump analogue)"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + }); + + it('snaps the dev commit, carries the drifted component onto the lane, and reports it', () => { + const before = remoteLaneFingerprint(LANE); + expect(before).to.not.include('comp2'); + const { output, exitCode } = syncRun(LANE); + expect(exitCode, `bit ci sync output:\n${output}`).to.equal(0); + expect(output).to.include('dependency-context drift'); + expect(output).to.include('comp2'); + expect(laneTipFile(devPath, 'comp1/index.js')).to.include('dev-commit-1'); + expect(remoteLaneFingerprint(LANE)).to.include('comp2'); + }); + }); + + // The case that used to halt production: a drifted component's pre-existing tag blocker must not + // block a lane run now that lanes carry the whole fan-out in one snap. Reuses the circular-pair + + // policy-bump recipe from the main-convergence describe below, on a lane instead of main. + describe('a drifted component with a pre-existing blocker rides the lane snap without halting', () => { + const LANE = 'drift-with-blocker-lane'; + let defaultBranch: string; + let devPath: string; + + before(() => { + ({ defaultBranch } = setupSyncWorkspace({ lanes: ['*'] })); + // A circular pair, recorded under a tag-blocker override — the blocker the drifted set's + // tolerance (`snapIgnoreIssues`) must carry through the lane snap. + helper.fs.outputFile('comp3/index.js', `require('is-odd');\nrequire('@${helper.scopes.remote}/comp4');`); + helper.fs.outputFile('comp4/index.js', `require('@${helper.scopes.remote}/comp3');`); + helper.command.addComponent('comp3'); + helper.command.addComponent('comp4'); + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '1.0.0' } }); + helper.command.install(); + helper.command.tagAllWithoutBuild('--ignore-issues="CircularDependencies"'); + helper.command.export(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "record circular pair under is-odd 1.0.0"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + // Create the lane before the policy bump, for the same reason as the drift cell above. + devPath = createLaneWithSnap(LANE, { 'comp1/index.js': comp1Src('lane-snap-1') }, 'lane snap 1'); + seedSync(LANE); + branchSideCommit(LANE, defaultBranch, 'comp1/index.js', comp1Src('dev-commit-1'), 'dev commit on comp1'); + // engine-bump analogue: drifts the circular pair, blocker and all + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '3.0.1' } }); + helper.command.install(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "bump is-odd policy (engine-bump analogue)"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + }); + + it('exits 0, snaps the dev commit, and carries the drifted pair onto the lane', () => { + const { output, exitCode } = syncRun(LANE); + expect(exitCode, `bit ci sync output:\n${output}`).to.equal(0); + expect(output).to.not.include('Workspace status verification failed'); + expect(output).to.include('dependency-context drift'); + expect(laneTipFile(devPath, 'comp1/index.js')).to.include('dev-commit-1'); + const laneFingerprint = remoteLaneFingerprint(LANE); + expect(laneFingerprint).to.include('comp3'); + expect(laneFingerprint).to.include('comp4'); + }); + }); + + // Convergence on main consumes the drift: one patch tag, exported, with the .bitmap bump riding + // the bit-sync/main flow. The circular pair also drifts. The tag must tolerate the blocker + // already present on the recorded heads (tagged with --ignore-issues originally). + describe('main reconcile converges dependency-context drift', () => { + const SYNC_BRANCH = 'bit-sync/main'; + let defaultBranch: string; + + before(() => { + ({ defaultBranch } = setupSyncWorkspace({ lanes: ['*'] })); + helper.fs.outputFile('comp2/index.js', `require('is-odd');\nmodule.exports = () => 'comp2: with-pkg';\n`); + helper.fs.outputFile('comp3/index.js', `require('is-odd');\nrequire('@${helper.scopes.remote}/comp4');`); + helper.fs.outputFile('comp4/index.js', `require('@${helper.scopes.remote}/comp3');`); + helper.command.addComponent('comp3'); + helper.command.addComponent('comp4'); + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '1.0.0' } }); + helper.command.install(); + helper.command.tagAllWithoutBuild('--ignore-issues="CircularDependencies"'); + helper.command.export(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "record deps under is-odd 1.0.0"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-odd': '3.0.1' } }); + // A bare workspace.jsonc edit is invisible to a running process. Only a real `install()` + // re-run moves what gets resolved from disk (node_modules/lockfile). + helper.command.install(); + helper.command.runCmd('git add -A'); + helper.command.runCmd('git commit -m "bump is-odd policy"'); + helper.command.runCmd(`git push origin ${defaultBranch}`); + }); + + it('dry-run reports the convergence and tags nothing', () => { + const { output, exitCode } = syncRun('--main --dry-run'); + expect(exitCode, output).to.equal(0); + expect(output).to.include('dependency-context drift'); + expect(output).to.include('dry-run'); + // Asserts on the returned summary, not just the mid-run log line. + expect(output).to.include('main -> dry-run: would converge'); + const list = helper.command.listRemoteScopeParsed(); + const comp2 = list.find((c: any) => c.id.includes('comp2')); + // comp2 is already recorded at 0.0.2 from the setup's own tag (is-odd 1.0.0); the dry-run + // must not advance it further. + expect(comp2.localVersion || comp2.currentVersion).to.equal('0.0.2'); + }); + + it('converges: one patch tag with the alignment message, exported, .bitmap bump on the sync branch', () => { + const { output, exitCode } = syncRun('--main'); + expect(exitCode, output).to.equal(0); + expect(output).to.include('align dependency context'); + expect(output).to.include('main -> pushed sync commit to'); + // 0.0.3, not 0.0.2: 0.0.2 is already true at the fork point and would pass regardless. + expect(fileOnBranch(SYNC_BRANCH, '.bitmap')).to.include('0.0.3'); + }); + + it('the next run finds a converged pair and no-ops', () => { + const { output, exitCode } = syncRun('--main'); + expect(exitCode, output).to.equal(0); + expect(output).to.match(/converged/i); + expect(output).to.not.include('align dependency context'); + }); + }); + describe('a stale bit-sync/main that conflicts with the default branch', () => { const SYNC_BRANCH = 'bit-sync/main'; let defaultBranch: string; diff --git a/scopes/git/ci/ci.docs.mdx b/scopes/git/ci/ci.docs.mdx index 42177b3c1a03..1763f3bc3064 100644 --- a/scopes/git/ci/ci.docs.mdx +++ b/scopes/git/ci/ci.docs.mdx @@ -395,6 +395,23 @@ pull-request diff, and a person rejects it: close the pull request. With `mainSy command commits the same drift on the default branch, and uses no sync branch and no pull request. The push is a plain push, so the run stops if the default branch moved during the run. +### Dependency-context drift + +A mirror workspace can show modified components when git has no changes. The +cause is a moved resolution context: the pinned bit engine ships new env +dependency templates, or a committed root policy changes a recorded range. +This is a real dependency change that the repository introduces. + +`bit ci sync` consumes it. A main run tags the drifted components with a +patch bump and a message of the shape +`chore: align dependency context (recorded with bit X, workspace runs bit Y)`, +then exports. A lane run snaps the drift together with the git-authored +change, in one snap under the developer's own message, and reports the +drifted components on the pull request — main convergence is for context +drift that reaches main without a pull request. Pin the engine in +`workspace.jsonc` (`"teambit.harmony/bit": { "engine": "" }`) so the +context moves only when a commit moves it. + ### Git host providers and credentials The command uses plain git for every git operation. The command uses a `GitHostProvider` for every diff --git a/scopes/git/ci/ci.main.runtime.ts b/scopes/git/ci/ci.main.runtime.ts index 183077f012c7..48a033d94a4f 100644 --- a/scopes/git/ci/ci.main.runtime.ts +++ b/scopes/git/ci/ci.main.runtime.ts @@ -1,6 +1,14 @@ import type { RuntimeDefinition, SlotRegistry } from '@teambit/harmony'; import { Slot } from '@teambit/harmony'; -import { CLIAspect, type CLIMain, MainRuntime } from '@teambit/cli'; +import { + CLIAspect, + type CLIMain, + MainRuntime, + formatWarningSummary, + formatSuccessSummary, + formatSection, + formatItem, +} from '@teambit/cli'; import { LoggerAspect, type LoggerMain, type Logger } from '@teambit/logger'; import { WorkspaceAspect, type Workspace } from '@teambit/workspace'; import { BuilderAspect, type BuilderMain } from '@teambit/builder'; @@ -13,9 +21,6 @@ import { ExportAspect, type ExportMain } from '@teambit/export'; import { ImporterAspect, type ImporterMain } from '@teambit/importer'; import { CheckoutAspect, checkoutOutput, type CheckoutMain } from '@teambit/checkout'; import type { MergeStrategy } from '@teambit/component.modules.merge-helper'; -import { getDivergeData } from '@teambit/component.snap-distance'; -import { ComponentConfigMerger } from '@teambit/config-merger'; -import { DependencyResolverAspect } from '@teambit/dependency-resolver'; import execa from 'execa'; import chalk from 'chalk'; import type { ReleaseType } from 'semver'; @@ -28,18 +33,21 @@ import { CiSyncCmd } from './commands/sync.cmd'; import { git } from './git'; import { ComponentIdList } from '@teambit/component-id'; import type { ComponentID } from '@teambit/component-id'; -import { compact, isEqual } from 'lodash'; +import { isEqual } from 'lodash'; import type { Version, LaneComponent, Lane } from '@teambit/objects'; import { Ref } from '@teambit/objects'; import type { LaneId } from '@teambit/lane-id'; import type { ConsumerComponent } from '@teambit/legacy.consumer-component'; import { SourceBranchDetector } from './source-branch-detector'; import { generateRandomStr } from '@teambit/toolbox.string.random'; -import { pMapPool } from '@teambit/toolbox.promise.map-pool'; -import { concurrentComponentsLimit } from '@teambit/harmony.modules.concurrency'; import { extractSkipTasksFromMessage } from './skip-tasks-from-message'; import { isPullRequestRef } from './pull-request-ref'; import { adoptAndRetrySwitch, isLaneMissingComponentError } from './sync/adopt-lane-new-components'; +import { getBitVersion } from '@teambit/bit.get-bit-version'; +import { detectContextDrift as detectContextDriftImpl } from './sync/context-drift-detector'; +import type { ContextDriftReport } from './sync/context-drift-detector'; +import { convergenceMessage, blockerNamesUnion } from './sync/context-drift'; +import { syncConfigFromMain as syncConfigFromMainImpl } from './sync/main-config-sync'; export type CiSwitchLaneOptions = SwitchLaneOptions & { /** after an adoption retry, write the adopted components' files (`checkout --reset`) */ @@ -58,6 +66,8 @@ import { isValidGitBranchName } from './sync/sync-config'; */ export type GitHostProviderSlot = SlotRegistry; +export type { ContextDriftReport }; + // Two distinct conflicts can surface from the remote on a concurrent `bit ci pr` race. // LANE_HASH_MISMATCH fires when both runners called `Lane.create` (the lane didn't exist on // the remote yet), so they each minted a random `sha1(v4())` hash — `sources.mergeLane` then @@ -434,22 +444,46 @@ export class CiMain { return 'chore: update .bitmap and lockfiles as needed [skip ci]'; } - private async verifyWorkspaceStatusInternal(strict: boolean = false) { + /** + * `verifyIds` scopes the status failure to a subset of what this run may snap — the git-authored + * change on a lane run, not the drifted components riding along with it (those are tolerated at + * snap time instead, via `driftIds`/`snapIgnoreIssues`). A global failure would otherwise block + * every snap in the repo. The snap itself still enforces blockers on every component it snaps. + */ + private async verifyWorkspaceStatusInternal( + strict: boolean = false, + { verifyIds }: { verifyIds?: ComponentID[] } = {} + ) { this.logger.console('📊 Workspace Status'); this.logger.console(chalk.blue('Verifying status of workspace')); + const formatOptions = strict + ? { strict: true, warnings: true } // When strict=true, fail on both issues and warnings + : { failOnError: true, warnings: false }; // By default, fail only on errors (tag blockers) const status = await this.status.status({ lanes: true }); - const { data: statusOutput, code } = await this.status.formatStatusOutput( - status, - strict - ? { strict: true, warnings: true } // When strict=true, fail on both errors and warnings - : { failOnError: true, warnings: false } // By default, fail only on errors (tag blockers) - ); + const { data: statusOutput, code } = await this.status.formatStatusOutput(status, formatOptions); // Log the formatted status output this.logger.console(statusOutput); - if (code !== 0) { + let effectiveCode = code; + if (code !== 0 && verifyIds) { + const inSet = ComponentIdList.fromArray(verifyIds); + const scoped = { + ...status, + componentsWithIssues: status.componentsWithIssues.filter((c) => inSet.hasWithoutVersion(c.id)), + }; + ({ code: effectiveCode } = await this.status.formatStatusOutput(scoped, formatOptions)); + if (effectiveCode === 0) { + this.logger.console( + formatWarningSummary( + "The issues above are on components outside this run's git-authored change — they do not fail this gate" + ) + ); + } + } + + if (effectiveCode !== 0) { throw new Error('Workspace status verification failed'); } @@ -532,226 +566,12 @@ export class CiMain { }).sync(opts); } - /** - * Sync *config-only* changes from main onto the lane — without a full `bit lane merge`. - * - * In this workflow git is the source of truth for files: the PR author merges the default branch - * into their PR branch, so source changes arrive via git. The one thing git can't carry is - * config that's already been *tagged into objects* on main — e.g. another PR ran `bit env set` / - * `bit deps set`; those records lived in `.bitmap`, rode git into main, and `bit ci merge` baked - * them into the component's Version (clearing them from `.bitmap`). A long-running PR's lane - * would otherwise miss them. - * - * A full lane merge is the wrong tool here: it does a 3-way *file* merge and refuses to run while - * the workspace has modified components — but in `bit ci pr` the workspace is always dirty (the - * PR's changes, not yet snapped). So instead we do a per-component 3-way merge of the aspect - * *config only* (base = common ancestor, ours = lane, theirs = main), keeping the PR's config on - * conflict, and stash the result on an `unmergedComponents` entry's `mergedConfig`. The - * subsequent `snap` reads it (via the aspects-merger on component load) and bakes main's config - * into the new snap, while the snap's files still come from the workspace (git). No file - * checkout, so no clean-workspace requirement. - */ + /** Config-only sync from main onto the lane; see `sync/main-config-sync.ts`. */ private async syncConfigFromMain(laneId: LaneId) { - const legacyScope = this.workspace.scope.legacyScope; - const repo = legacyScope.objects; - const mainLaneId = this.lanes.getDefaultLaneId(); - const currentLane = await this.lanes.getCurrentLane(); - if (!currentLane) return; - const workspaceIds = this.workspace.listIds(); - - this.logger.console(chalk.blue(`Syncing config changes from ${mainLaneId.toString()} into ${laneId.toString()}`)); - - // Resolve each lane component's head on main once, keeping only those that are on main and whose - // lane head differs from it (the rest have nothing to sync). This single pass feeds both the - // pre-fetch below and the merge loop, so we never load the same ModelComponent twice. - const componentsToSync = compact( - await Promise.all( - currentLane.components.map(async (laneComp) => { - try { - const modelComponent = await legacyScope.getModelComponentIfExist(laneComp.id); - const mainHead = modelComponent?.head; // the component's head on main - if (!modelComponent || !mainHead || mainHead.isEqual(laneComp.head)) return undefined; - return { laneComp, modelComponent, mainHead }; - } catch (e: any) { - // Best-effort per component (same contract as the merge loop below): one component's - // load failure shouldn't reject Promise.all and abort the whole config sync. - this.logger.console( - chalk.yellow( - ` ${laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})` - ) - ); - return undefined; - } - }) - ) + await syncConfigFromMainImpl( + { workspace: this.workspace, lanes: this.lanes, importer: this.importer, logger: this.logger }, + laneId ); - - // The lane import (switchToLane) brought each component's lane history plus the lightweight - // version-history (the parent graph) — that's enough for the diverge check below to see that - // main is ahead — but NOT the full Version object for main's head wherever main advanced past - // the lane's fork point. Those objects live only on main and were never fetched. Without them - // `loadVersion(mainHead)` throws VersionNotFoundOnFS, the per-component catch swallows it as - // "skipping config sync from main", and the sync silently degrades to a no-op for every - // diverged component. Pre-fetch main's head objects in one batched remote call (mirroring the - // lane-merge flows — see merge-status-provider / merge-lanes). Pass the *specific* main-head - // version so `cache: true` still fetches it: the component already exists locally at its lane - // version, so a version-less id would look satisfied and skip the remote. - const mainHeadIds = componentsToSync.map(({ laneComp, mainHead }) => - laneComp.id.changeVersion(mainHead.toString()) - ); - await this.prefetchFromMainForConfigSync(mainHeadIds, 'head objects'); - - // Resolve each component's diverge state up front — before the merge loop — so we can also - // pre-fetch the common-ancestor objects below. getDivergeData only walks the parent graph, - // which is already local (switchToLane brings the version-history, and the head pre-fetch above - // reinforced it), so no full Version object is needed yet. Keep only components where main is - // actually ahead or diverged; the rest have nothing to bring in from main. Bound the fan-out - // (getDivergeData traverses each component's version graph) so a lane with many components - // doesn't spawn one unbounded burst of concurrent graph walks. - const componentsToMerge = compact( - await pMapPool( - componentsToSync, - async (item) => { - try { - const divergeData = await getDivergeData({ - repo, - modelComponent: item.modelComponent, - sourceHead: item.laneComp.head, - targetHead: item.mainHead, - throws: false, - }); - if (!divergeData.isTargetAhead() && !divergeData.isDiverged()) return undefined; - return { ...item, divergeData }; - } catch (e: any) { - // Best-effort per component (same contract as the merge loop below). - this.logger.console( - chalk.yellow( - ` ${item.laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})` - ) - ); - return undefined; - } - }, - { concurrency: concurrentComponentsLimit() } - ) - ); - - // The head pre-fetch above brought main's head Version plus the version-history (parent graph), - // but NOT the full Version object of the common ancestor (the lane's fork point) for components - // where the lane and main have BOTH snapped since the fork. The 3-way config merge below loads - // that base Version (`baseVersion.extensions`); without it, `loadVersion(baseSnap)` throws - // VersionNotFoundOnFS, the per-component catch swallows it as "skipping config sync from main", - // and the sync silently no-ops for every diverged component. The fork point lives on main and, - // like the head, was never fetched (includeVersionHistory carries the graph, not each ancestor's - // Version). Batch-fetch the bases in one call — same best-effort contract as the head pre-fetch. - const baseIds = compact( - componentsToMerge.map(({ laneComp, divergeData }) => { - const baseSnap = divergeData.commonSnapBeforeDiverge; - return baseSnap ? laneComp.id.changeVersion(baseSnap.toString()) : undefined; - }) - ); - await this.prefetchFromMainForConfigSync(baseIds, 'common-ancestor objects'); - - const syncedIds: ComponentID[] = []; - for (const { laneComp, modelComponent, mainHead, divergeData } of componentsToMerge) { - try { - const laneHead = laneComp.head; - const currentVersion = await modelComponent.loadVersion(laneHead.toString(), repo); - const otherVersion = await modelComponent.loadVersion(mainHead.toString(), repo); - // base = common ancestor. When the lane is strictly behind main (no divergence) the common - // ancestor IS the lane head, so the lane's own aspects serve as the base. - const baseSnap = divergeData.commonSnapBeforeDiverge; - const baseVersion = baseSnap ? await modelComponent.loadVersion(baseSnap.toString(), repo) : currentVersion; - - const configMerger = new ComponentConfigMerger( - laneComp.id.toStringWithoutVersion(), - workspaceIds, - undefined, // merging from main (the default lane) — there's no Lane object for it - currentVersion.extensions, - baseVersion.extensions, - otherVersion.extensions, - laneId.toString(), - mainLaneId.toString(), - this.logger, - 'ours' as MergeStrategy // keep the PR's config on a genuine conflict - ); - 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); - - // Upsert: addEntry throws if an entry for this component already exists. A prior - // --keep-lane run that crashed mid-snap (or otherwise left unmerged.json entries behind) - // would otherwise make every later run throw here, skip the component, and keep serving - // stale config. Remove any existing entry first so repeated runs converge on main's latest. - legacyScope.objects.unmergedComponents.removeComponent(laneComp.id); - legacyScope.objects.unmergedComponents.addEntry({ - id: { scope: laneComp.id.scope, name: laneComp.id.fullName }, - head: mainHead, - laneId: mainLaneId, - mergedConfig, - }); - syncedIds.push(laneComp.id); - this.logger.console( - chalk.blue( - ` ${laneComp.id.toStringWithoutVersion()}: applying main's config (${Object.keys(mergedConfig).join(', ')})` - ) - ); - } catch (e: any) { - // Best-effort per component: one component's config-merge quirk shouldn't abort the whole - // `bit ci pr`. Log and move on — the build just won't reflect that component's main-side - // config this run. - this.logger.console( - chalk.yellow(` ${laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})`) - ); - } - } - - if (!syncedIds.length) { - this.logger.console(chalk.blue('No config changes from main to sync')); - return; - } - await legacyScope.objects.unmergedComponents.write(); - // The components were already loaded (and their aspects cached) earlier in this run, before the - // unmergedComponents entries existed. Clear the cache so the upcoming `snap` reloads them and - // the aspects-merger folds in the synced `mergedConfig`. - this.workspace.clearAllComponentsCache(); - this.logger.console(chalk.green(`Synced config from main for ${syncedIds.length} component(s)`)); - } - - /** - * Batch-fetch main-side Version objects the config merge needs (heads, then common ancestors), - * mirroring the lane-merge flows (merge-status-provider / merge-lanes). Best-effort: a fetch - * hiccup shouldn't abort `bit ci pr` — the merge loop still runs and any component whose object is - * still missing just logs the existing per-component skip. `label` names which objects for the log. - */ - private async prefetchFromMainForConfigSync(ids: ComponentID[], label: string) { - if (!ids.length) return; - try { - await this.importer.importObjectsFromMainIfExist(ids, { cache: true }); - } catch (e: any) { - this.logger.console( - chalk.yellow(`Could not pre-fetch main's ${label} for config sync (continuing): ${e?.message || e}`) - ); - } - } - - /** - * Copied from `merging.main.runtime` (`filterDeletedDependenciesFromConfig`): the config merge - * can emit deletion markers (`version: '-'`) for deps removed on main. The aspects-merger applies - * `mergedConfig` verbatim, so strip those here to avoid writing a policy entry with version '-'. - */ - private filterDeletedDependenciesFromConfig(mergeConfig?: Record): void { - const policy: Record> | undefined = - mergeConfig?.[DependencyResolverAspect.id]?.policy; - if (!policy) return; - Object.keys(policy).forEach((depType) => { - const filtered = policy[depType].filter((dep) => dep.version !== '-'); - if (filtered.length === 0) delete policy[depType]; - else policy[depType] = filtered; - }); } /** @@ -792,6 +612,87 @@ export class CiMain { } } + /** The bit binary this process runs — compared against a drifted component's `recordedBitVersion`. */ + getRunningBitVersion(): string { + return getBitVersion(); + } + + /** + * Split the tag-pending set into git-authored changes and dependency-context drift. + * See `sync/context-drift-detector.ts` for what counts as drift. + */ + async detectContextDrift(): Promise { + return detectContextDriftImpl(this.workspace, this.logger); + } + + /** + * Consume dependency-context drift on main with one patch tag over the drifted set. Auto-tag + * then carries each drifted component's dependents; skipping it would leave their recorded deps + * stale and the next run would re-detect the same drift. Ignores blockers on the drifted + * components, including ones the context change itself introduces — their files and config + * already match the recorded head. Leaves .bitmap/lockfile updates in the working tree for the + * caller's mainSync commit flow. + */ + async convergeContextDrift({ dryRun }: { dryRun?: boolean } = {}): Promise<{ + converged: number; + detected: boolean; + summary: string; + }> { + const { drift } = await this.detectContextDrift(); + if (!drift.length) return { converged: 0, detected: false, summary: 'no dependency-context drift' }; + const running = this.getRunningBitVersion(); + this.logger.console( + formatSection( + 'dependency-context drift', + '', + drift.map((d) => + formatItem( + `${d.id.toStringWithoutVersion()} (${d.changedKeys.join(', ')})` + + `${d.recordedBitVersion && d.recordedBitVersion !== running ? ` recorded with bit ${d.recordedBitVersion}` : ''}` + ) + ) + ) + ); + const idStrs = drift.map((d) => d.id.toStringWithoutVersion()); + if (dryRun) { + return { converged: 0, detected: true, summary: `dry-run: would converge ${drift.length} component(s)` }; + } + const message = convergenceMessage( + drift.map((d) => d.recordedBitVersion), + running + ); + const status = await this.status.status({ lanes: true }); + const ignoreIssues = blockerNamesUnion(status.componentsWithIssues, new Set(idStrs)); + const results = await this.snapping.tag({ + ids: idStrs, + message, + releaseType: 'patch', + autoTagReleaseType: 'patch', + ignoreIssues, + build: undefined, + persist: false, + failFast: true, + }); + // Drift detected but the tag produced nothing (detector and tag disagree); `detected` stays + // true here, unlike the empty-drift case above. + if (!results) { + return { + converged: 0, + detected: true, + summary: `drift detected but nothing was taggable (${drift.length} component(s))`, + }; + } + this.logger.console(chalk.blue(message)); + const exportIds = [ + ...results.taggedComponents.map((c) => c.id.toString()), + ...results.autoTaggedResults.map((r) => r.component.id.toString()), + ]; + await this.exporter.export({ ids: exportIds }); + const count = results.taggedComponents.length; + this.logger.console(formatSuccessSummary(`Converged ${count} component(s)`)); + return { converged: count, detected: true, summary: `converged ${count} component(s)` }; + } + async verifyWorkspaceStatus() { await this.verifyWorkspaceStatusInternal(); @@ -819,6 +720,8 @@ export class CiMain { skipCleanup, skipTasks, noDestructiveRecovery, + verifyIds, + driftIds, }: { laneIdStr: string; message: string; @@ -836,6 +739,19 @@ export class CiMain { * stale-lane case throws instead, which the sync executor surfaces as a halt for a human. */ noDestructiveRecovery?: boolean; + /** + * Scope the status verification to these ids (no version) instead of every pending component; + * unset for `bit ci pr` (production: global). The snap itself is never scoped — it always snaps + * whatever bit's own tag-pending resolution finds. + */ + verifyIds?: string[]; + /** + * Dependency-context drift ids riding along in this snap (lane runs only). Consumed only here, + * to compute the snap's `ignoreIssues`: a drifted component's files and config already equal its + * recorded head, so a tag-blocker already present on it is not new — see `blockerNamesUnion`. + * Never forwarded past this method. + */ + driftIds?: string[]; }) { // The post-export cleanup switches the workspace back to main, which re-checks-out main's HEAD // and re-imports every workspace component — pointless when the workspace is about to be @@ -868,7 +784,21 @@ export class CiMain { const laneId = await this.lanes.parseLaneId(laneIdStr); - await this.verifyWorkspaceStatusInternal(strict); + const resolvedVerifyIds = verifyIds ? await this.workspace.resolveMultipleComponentIds(verifyIds) : undefined; + + const { status } = await this.verifyWorkspaceStatusInternal(strict, { verifyIds: resolvedVerifyIds }); + + // The union of tag-blocker issue names already present on the drifted set — tolerated because a + // drifted component's files and config equal its recorded head, so its blockers are not new. + // Computed here (against the status this call already loaded) rather than in the caller, so a + // lane run doesn't pay for a second full `status()` pass. + // Coarse by construction (spec 5.2): `ignoreIssues` applies to the whole snap, not per component, + // so a blocker `syncConfigFromMain` freshly introduces on some OTHER component is also shadowed + // whenever its type happens to match one already tolerated for the drifted set. No re-verify + // step recovers from this; per-component granularity needs a snapping API change (out of scope). + const snapIgnoreIssues = driftIds?.length + ? blockerNamesUnion(status.componentsWithIssues, new Set(driftIds)) + : undefined; await this.importer .import({ @@ -896,6 +826,7 @@ export class CiMain { skipCleanup: resolvedSkipCleanup, skipTasks: resolvedSkipTasks, noDestructiveRecovery, + snapIgnoreIssues, }); } return this.snapAndExportWithTempLane({ @@ -906,6 +837,7 @@ export class CiMain { dryRun, skipCleanup: resolvedSkipCleanup, skipTasks: resolvedSkipTasks, + snapIgnoreIssues, }); } @@ -968,6 +900,7 @@ export class CiMain { skipCleanup, skipTasks, noDestructiveRecovery, + snapIgnoreIssues, }: { laneId: LaneId; originalLane: Lane | undefined; @@ -977,6 +910,8 @@ export class CiMain { skipCleanup: boolean; skipTasks?: string; noDestructiveRecovery?: boolean; + /** Tag-blocker issue names to tolerate on this snap; see `snapPrCommit`'s `driftIds`. */ + snapIgnoreIssues?: string; }) { // Query the remote (by name, to avoid fetching all lanes) so we know whether to reuse or create const existingLanes = await this.lanes.getLanes({ remote: laneId.scope, name: laneId.name }).catch((e) => { @@ -1017,6 +952,10 @@ export class CiMain { ) ); } else { + // `syncConfigFromMain` clears the component cache; a component it just reconfigured + // can only now show as tag-pending. Nothing to re-verify or expand here: the snap below + // always resolves its own tag-pending set fresh (no `legacyBitIds` scoping), so it picks + // up any component this sync just reconfigured without a staleness window. await this.syncConfigFromMain(laneId); } } else { @@ -1139,6 +1078,7 @@ export class CiMain { build, exitOnFirstFailedTask: true, skipTasks, + ignoreIssues: snapIgnoreIssues, }); if (!results) { @@ -1185,6 +1125,7 @@ export class CiMain { dryRun, skipCleanup, skipTasks, + snapIgnoreIssues, }: { laneId: LaneId; originalLane: Lane | undefined; @@ -1193,6 +1134,8 @@ export class CiMain { dryRun?: boolean; skipCleanup: boolean; skipTasks?: string; + /** Tag-blocker issue names to tolerate on this snap; see `snapPrCommit`'s `driftIds`. */ + snapIgnoreIssues?: string; }) { // Use unique temp lane name to avoid race conditions when multiple CI jobs run concurrently const tempLaneName = `${laneId.name}-${generateRandomStr(5)}`; @@ -1224,6 +1167,7 @@ export class CiMain { build, exitOnFirstFailedTask: true, skipTasks, + ignoreIssues: snapIgnoreIssues, }); if (!results) { diff --git a/scopes/git/ci/sync/context-drift-detector.spec.ts b/scopes/git/ci/sync/context-drift-detector.spec.ts new file mode 100644 index 000000000000..60f786cf27c5 --- /dev/null +++ b/scopes/git/ci/sync/context-drift-detector.spec.ts @@ -0,0 +1,46 @@ +import { expect } from 'chai'; +import type { SourceFile } from '@teambit/component.sources'; +import { compareFiles } from './context-drift-detector'; + +/** + * Minimal SourceFile stand-in: only `.relative` and the hash chain `compareFiles` reads. + * A regression fixture for the bug this covers: SourceFile has no `.relativePath` (Vinyl exposes + * `.relative`); reading the wrong property collapsed every file to one `undefined`-keyed map + * entry, so only the last file's hash was ever compared. + */ +function file(relative: string, hash: string): SourceFile { + return { relative, toSourceAsLinuxEOL: () => ({ hash: () => ({ hash }) }) } as unknown as SourceFile; +} + +describe('compareFiles', () => { + const recorded = [file('index.js', 'hash-index'), file('utils.js', 'hash-utils')]; + + // same paths, same content hash — a real fixture's deprecated `test`/`name` prop discrepancy + // plays no part here, since this function reads only path and hash. That is what lets the + // caller attribute an unchanged-content, files/specs-only field diff to those deprecated props. + it('reports unchanged for identical multi-file sets', () => { + const workspace = [file('index.js', 'hash-index'), file('utils.js', 'hash-utils')]; + expect(compareFiles(recorded, workspace)).to.deep.equal({ filesChanged: false, pathSetsEqual: true }); + }); + + it('catches a content edit on a NON-last file — the exact shape the relative/relativePath bug hid', () => { + const workspace = [file('index.js', 'hash-index-EDITED'), file('utils.js', 'hash-utils')]; + const res = compareFiles(recorded, workspace); + expect(res.filesChanged).to.equal(true); + expect(res.pathSetsEqual).to.equal(true); + }); + + it('catches a file added, keeping the recorded pair otherwise identical', () => { + const workspace = [file('index.js', 'hash-index'), file('utils.js', 'hash-utils'), file('new.js', 'hash-new')]; + const res = compareFiles(recorded, workspace); + expect(res.filesChanged).to.equal(true); + expect(res.pathSetsEqual).to.equal(false); + }); + + it('catches a file deleted, keeping the remaining file identical', () => { + const workspace = [file('index.js', 'hash-index')]; + const res = compareFiles(recorded, workspace); + expect(res.filesChanged).to.equal(true); + expect(res.pathSetsEqual).to.equal(false); + }); +}); diff --git a/scopes/git/ci/sync/context-drift-detector.ts b/scopes/git/ci/sync/context-drift-detector.ts new file mode 100644 index 000000000000..80e74277b5e5 --- /dev/null +++ b/scopes/git/ci/sync/context-drift-detector.ts @@ -0,0 +1,128 @@ +import chalk from 'chalk'; +import { ComponentIdList } from '@teambit/component-id'; +import type { ComponentID } from '@teambit/component-id'; +import type { Workspace } from '@teambit/workspace'; +import type { Logger } from '@teambit/logger'; +import type { Scope as LegacyScope } from '@teambit/legacy.scope'; +import type { Repository } from '@teambit/objects'; +import type { SourceFile } from '@teambit/component.sources'; +import { pathNormalizeToLinux } from '@teambit/toolbox.path.path'; +import { pMapPool } from '@teambit/toolbox.promise.map-pool'; +import { concurrentComponentsLimit } from '@teambit/harmony.modules.concurrency'; +import { diffBetweenComponentsObjects } from '@teambit/legacy.component-diff'; +import type { FileComparison } from './context-drift'; +import { classifyDiffFields } from './context-drift'; + +type DriftCheckResult = + | { id: ComponentID; kind: 'git-authored' } + | { id: ComponentID; kind: 'drift'; recordedBitVersion?: string; changedKeys: string[] }; + +export type ContextDriftReport = { + /** dep-only diff vs the recorded version — a lane run snaps these too, as surfaced side effects */ + drift: { id: ComponentID; recordedBitVersion?: string; changedKeys: string[] }[]; + /** pending minus drift: new components and file/config-diff components */ + gitAuthored: ComponentID[]; +}; + +// SourceFile is Vinyl-based: the path property is `.relative`, not `.relativePath` (that name +// belongs to SourceFileModel). Both `componentFromModel.files` and the workspace component's +// `.files` are SourceFile — `Component.toConsumerComponent` already converts model files to it. +function relPath(file: SourceFile): string { + return pathNormalizeToLinux(file.relative); +} + +/** relativePath -> content hash, using the same hashing the model persists files with. */ +function fileHashes(files: SourceFile[]): Map { + return new Map(files.map((file) => [relPath(file), file.toSourceAsLinuxEOL().hash().hash])); +} + +function pathsEqual(a: SourceFile[], b: SourceFile[]): boolean { + const bPaths = new Set(b.map(relPath)); + return a.length === b.length && a.every((file) => bPaths.has(relPath(file))); +} + +/** + * Content truth, independent of the field diff: a files/specs field diff can fire on unchanged + * content (deprecated per-file props). `pathSetsEqual` is computed structurally, never derived + * from the hash compare, so a broken hash compare can't fake it. Exported for unit coverage — + * this is the multi-file content compare, not the diff engine. + */ +export function compareFiles(recordedFiles: SourceFile[], workspaceFiles: SourceFile[]): FileComparison { + const recorded = fileHashes(recordedFiles); + const workspace = fileHashes(workspaceFiles); + const filesChanged = + recorded.size !== workspace.size || [...recorded].some(([path, hash]) => workspace.get(path) !== hash); + return { filesChanged, pathSetsEqual: pathsEqual(recordedFiles, workspaceFiles) }; +} + +/** Attribution only: which bit version recorded this component. A lookup failure must not affect the verdict. */ +async function getRecordedBitVersion( + legacyScope: LegacyScope, + repo: Repository, + id: ComponentID +): Promise { + try { + const modelComponent = await legacyScope.getModelComponent(id); + const recorded = await modelComponent.loadVersion(id.version as string, repo); + return recorded.bitVersion; + } catch { + return undefined; + } +} + +/** + * Split the tag-pending set into git-authored changes and dependency-context drift. Drift = the + * diff against the recorded version is confined to dependency data; files and config are identical. + */ +export async function detectContextDrift(workspace: Workspace, logger: Logger): Promise { + const pendingIds = await workspace.listTagPendingIds(); + // `export` refuses local-only components, and so does the snap's own tag-pending resolution + // (`Snapping.getTagPendingComponentsIds`). Filtering them here keeps this split aligned with what + // the snap actually snaps, or the drift/git-authored ids reported wouldn't match its outcome. + const localOnly = ComponentIdList.fromArray(workspace.filter.byLocalOnly(pendingIds)); + const pending = pendingIds.filter((id) => !localOnly.hasWithoutVersion(id)); + const legacyScope = workspace.scope.legacyScope; + const repo = legacyScope.objects; + // pMapPool preserves input order, keeping the split deterministic. + const results = await pMapPool( + pending, + async (id) => { + if (!id.hasVersion()) { + return { id, kind: 'git-authored' }; // new component: git-authored by definition + } + try { + const comp = await workspace.get(id); + const consumerComp = comp.state._consumer; + const fromModel = consumerComp.componentFromModel; + if (!fromModel) return { id, kind: 'git-authored' }; // nothing recorded to diff against + const fileComparison = compareFiles(fromModel.files ?? [], consumerComp.files ?? []); + // content alone settles it; skip the diff engine (it shells out to `git diff --no-index` + // per differing aspect config) rather than spend that cost on an already-decided verdict. + if (fileComparison.filesChanged) return { id, kind: 'git-authored' }; + // diffBetweenComponentsObjects sorts extension config keys in place; the mutation is + // shallow and hash-idempotent (Version.id() sorts the same way), so it is acceptable on + // the live object. + const fieldsDiff = await diffBetweenComponentsObjects(fromModel, consumerComp, { verbose: true }); + const fieldNames = (fieldsDiff ?? []).map((f) => f.fieldName); + const { drift, changedKeys, anomaly } = classifyDiffFields(fieldNames, fileComparison); + if (anomaly) logger.console(chalk.yellow(` ${id.toStringWithoutVersion()}: ${anomaly}`)); + if (!drift) return { id, kind: 'git-authored' }; + const recordedBitVersion = await getRecordedBitVersion(legacyScope, repo, id); + return { id, kind: 'drift', recordedBitVersion, changedKeys }; + } catch (e: any) { + // an unreadable model must not kill the run; degrade to git-authored + logger.console(chalk.yellow(` ${id.toStringWithoutVersion()}: drift check skipped (${e?.message || e})`)); + return { id, kind: 'git-authored' }; + } + }, + { concurrency: concurrentComponentsLimit() } + ); + const drift: ContextDriftReport['drift'] = []; + const gitAuthored: ComponentID[] = []; + for (const r of results) { + if (r.kind === 'drift') + drift.push({ id: r.id, recordedBitVersion: r.recordedBitVersion, changedKeys: r.changedKeys }); + else gitAuthored.push(r.id); + } + return { drift, gitAuthored }; +} diff --git a/scopes/git/ci/sync/context-drift.spec.ts b/scopes/git/ci/sync/context-drift.spec.ts new file mode 100644 index 000000000000..9452868ecab2 --- /dev/null +++ b/scopes/git/ci/sync/context-drift.spec.ts @@ -0,0 +1,139 @@ +import { expect } from 'chai'; +import { + classifyDiffFields, + convergenceMessage, + blockerNamesUnion, + driftReportCommentBody, + driftClearedCommentBody, + DRIFT_COMMENT_MARKER, +} from './context-drift'; + +describe('classifyDiffFields', () => { + const unchanged = { filesChanged: false, pathSetsEqual: true }; + const changed = { filesChanged: true, pathSetsEqual: true }; + + it('classifies a change confined to dependency-ish fields as drift', () => { + const res = classifyDiffFields(['packageDependencies', 'dependencies'], unchanged); + expect(res.drift).to.equal(true); + expect(res.changedKeys).to.deep.equal(['packageDependencies', 'dependencies']); + }); + + it('classifies overridesDevDependencies alone as drift', () => { + expect(classifyDiffFields(['overridesDevDependencies'], unchanged).drift).to.equal(true); + }); + + it('rejects overridesPackageJsonProps as a non-dependency field', () => { + expect(classifyDiffFields(['overridesPackageJsonProps'], unchanged).drift).to.equal(false); + }); + + it('rejects an aspect configuration field name', () => { + const res = classifyDiffFields(['teambit.envs/envs configuration'], unchanged); + expect(res.drift).to.equal(false); + expect(res.changedKeys).to.deep.equal(['teambit.envs/envs configuration']); + }); + + it('rejects any field set once file content changed, regardless of which fields fired', () => { + const res = classifyDiffFields(['packageDependencies'], changed); + expect(res.drift).to.equal(false); + expect(res.changedKeys).to.deep.equal(['packageDependencies']); + }); + + it('classifies files+specs only, with unchanged content and equal path sets, as drift from deprecated file props', () => { + const res = classifyDiffFields(['files', 'specs'], unchanged); + expect(res.drift).to.equal(true); + expect(res.changedKeys).to.deep.equal(['deprecated-file-props']); + }); + + it('refuses the deprecated-file-props default when the path sets are not provably equal', () => { + const res = classifyDiffFields(['files', 'specs'], { filesChanged: false, pathSetsEqual: false }); + expect(res.drift).to.equal(false); + expect(res.changedKeys).to.deep.equal(['files', 'specs']); + }); + + it('flags an empty field list with unchanged content as an anomaly, not drift', () => { + const res = classifyDiffFields([], unchanged); + expect(res.drift).to.equal(false); + expect(res.changedKeys).to.deep.equal([]); + expect(res.anomaly).to.equal('modified without a visible diff'); + }); +}); + +describe('convergenceMessage', () => { + it('names the recorded and running bit versions', () => { + const msg = convergenceMessage(['1.12.61', '1.12.61', undefined], '2.0.69'); + expect(msg).to.equal('chore: align dependency context (recorded with bit 1.12.61, workspace runs bit 2.0.69)'); + }); + it('lists distinct recorded versions', () => { + const msg = convergenceMessage(['1.12.61', '2.0.10'], '2.0.69'); + expect(msg).to.include('1.12.61, 2.0.10'); + }); + it('handles no recorded versions', () => { + expect(convergenceMessage([undefined], '2.0.69')).to.equal( + 'chore: align dependency context (workspace runs bit 2.0.69)' + ); + }); +}); + +describe('blockerNamesUnion', () => { + const issue = (name: string, isTagBlocker: boolean) => ({ isTagBlocker, constructor: { name } }); + const entry = (idStr: string, issues: { isTagBlocker: boolean; constructor: { name: string } }[]) => ({ + id: { toStringWithoutVersion: () => idStr }, + issues: { + getAllIssues: () => issues, + hasTagBlockerIssues: () => issues.some((i) => i.isTagBlocker), + }, + }); + + it('unions blocker issue names of in-set components only', () => { + const res = blockerNamesUnion( + [entry('s/a', [issue('CircularDependencies', true)]), entry('s/b', [issue('MissingDists', true)])], + new Set(['s/a']) + ); + expect(res).to.equal('CircularDependencies'); + }); + it('returns undefined when no in-set component has blockers', () => { + expect(blockerNamesUnion([entry('s/a', [issue('X', false)])], new Set(['s/a']))).to.equal(undefined); + }); + it('carries only the blocker issue name, not a non-blocker issue on the same component', () => { + const res = blockerNamesUnion( + [entry('s/a', [issue('CircularDependencies', true), issue('MissingDists', false)])], + new Set(['s/a']) + ); + expect(res).to.equal('CircularDependencies'); + }); +}); + +describe('driftReportCommentBody', () => { + const drift = [ + { + id: { toStringWithoutVersion: () => 'scope/comp2' }, + recordedBitVersion: '1.12.61', + changedKeys: ['packageDependencies'], + }, + ]; + const anchors = { + branch: 'drift-lane', + branchTipSha: 'abcdef0123456789', + laneIdStr: 'scope/drift-lane', + laneHead: '0123456789abcdef', + }; + + it('carries the marker, the cause, the synced pair, and the drifted component', () => { + const body = driftReportCommentBody(drift, '2.0.69', anchors); + expect(body).to.include(DRIFT_COMMENT_MARKER); + expect(body).to.include('recorded with bit 1.12.61, running bit 2.0.69'); + // the anchors: which push, which lane snap + expect(body).to.include('drift-lane'); + expect(body).to.include('abcdef012'); + expect(body).to.include('scope/drift-lane'); + expect(body).to.include('0123456789ab'.slice(0, 9)); + expect(body).to.include('scope/comp2'); + expect(body).to.include('packageDependencies'); + }); +}); + +describe('driftClearedCommentBody', () => { + it('carries the marker, so the upsert can find and update this exact comment', () => { + expect(driftClearedCommentBody()).to.include(DRIFT_COMMENT_MARKER); + }); +}); diff --git a/scopes/git/ci/sync/context-drift.ts b/scopes/git/ci/sync/context-drift.ts new file mode 100644 index 000000000000..6a6764b1ae6c --- /dev/null +++ b/scopes/git/ci/sync/context-drift.ts @@ -0,0 +1,156 @@ +// Field names diffBetweenComponentsObjects (verbose) can emit for a dependency-only change. Any +// other field name means the change is git-authored, not workspace/engine drift. +// `extensionDependencies` alone (a config-less aspect added or removed) also classifies as +// drift: inherent ambiguity, but load-bearing for a config-less aspect version bump that comes +// from an engine template rather than a developer edit. +export const DRIFT_FIELD_NAMES = [ + 'dependencies', + 'devDependencies', + 'peerDependencies', + 'extensionDependencies', + 'packageDependencies', + 'devPackageDependencies', + 'peerPackageDependencies', + 'overridesDependencies', + 'overridesDevDependencies', + 'overridesPeerDependencies', +] as const; + +// files/specs field diffs on unchanged file content come from deprecated per-file props (name, +// test); content truth is the hash compare done by the caller, not this field name. +const CONTENT_FIELDS = ['files', 'specs']; + +export type DriftClassification = { + drift: boolean; + changedKeys: string[]; + /** set when the component changed but the diff engine names nothing that explains it */ + anomaly?: string; +}; + +export type FileComparison = { + /** any file added, removed, or hash-changed — from a content compare, never from field names */ + filesChanged: boolean; + /** the two file sets cover the exact same relative paths, independently of any hash */ + pathSetsEqual: boolean; +}; + +/** + * Classify a diffBetweenComponentsObjects field-name list as dependency-context drift or + * git-authored. `filesChanged`/`pathSetsEqual` must come from a content compare, not from this + * field list: a files/specs field diff can fire on unchanged content (deprecated per-file props). + */ +export function classifyDiffFields( + fieldNames: string[], + { filesChanged, pathSetsEqual }: FileComparison +): DriftClassification { + if (filesChanged) return { drift: false, changedKeys: fieldNames }; + const effective = fieldNames.filter((f) => !CONTENT_FIELDS.includes(f)); + if (!effective.length) { + // files/specs fired with unchanged content: only a deprecated per-file prop (name/test) can + // explain it, and only if the two sides name the exact same files. Without that second check, + // this branch is a blanket default that would paper over a broken content compare. + if (fieldNames.length && pathSetsEqual) return { drift: true, changedKeys: ['deprecated-file-props'] }; + if (fieldNames.length) return { drift: false, changedKeys: fieldNames }; + // no field diff at all, yet the caller reached us because the component is tag-pending. + return { drift: false, changedKeys: [], anomaly: 'modified without a visible diff' }; + } + const drift = effective.every((f) => (DRIFT_FIELD_NAMES as readonly string[]).includes(f)); + return { drift, changedKeys: effective }; +} + +export function convergenceMessage(recordedBitVersions: (string | undefined)[], runningBitVersion: string): string { + const distinct = [...new Set(recordedBitVersions.filter(Boolean))] as string[]; + const recordedPart = distinct.length ? `recorded with bit ${distinct.join(', ')}, ` : ''; + return `chore: align dependency context (${recordedPart}workspace runs bit ${runningBitVersion})`; +} + +export function blockerNamesUnion( + componentsWithIssues: { + id: { toStringWithoutVersion(): string }; + issues: { + getAllIssues(): { isTagBlocker: boolean; constructor: { name: string } }[]; + hasTagBlockerIssues(): boolean; + }; + }[], + inSet: Set +): string | undefined { + const names = new Set(); + for (const entry of componentsWithIssues) { + if (!inSet.has(entry.id.toStringWithoutVersion())) continue; + if (!entry.issues.hasTagBlockerIssues()) continue; + // A non-blocker issue name in `ignoreIssues` is a no-op; only tag-blocker names belong here. + entry.issues + .getAllIssues() + .filter((issue) => issue.isTagBlocker) + .forEach((issue) => names.add(issue.constructor.name)); + } + return names.size ? [...names].join(',') : undefined; +} + +/** Marks the one PR comment `bit ci sync` owns for the drift report (spec 5.6). Not shown to a reader. */ +export const DRIFT_COMMENT_MARKER = ''; + +/** How many drifted components a report lists before it summarizes the rest, to stay under a git host's comment size limit. */ +const MAX_LISTED_DRIFT_COMPONENTS = 50; + +type DriftEntry = { id: { toStringWithoutVersion(): string }; recordedBitVersion?: string; changedKeys: string[] }; + +/** The synced pair a drift report names, so the comment identifies which push and which lane snap it covers. */ +export type DriftReportAnchors = { + branch: string; + /** the branch's tip commit after this run's sync commit */ + branchTipSha: string; + laneIdStr: string; + /** the lane's content fingerprint after this run's export (see `laneHeadFingerprint`) */ + laneHead: string; +}; + +/** + * The PR comment body for a lane snap that carried dependency-context drift (spec 5.6). States the + * cause, names the synced branch/lane pair, lists the affected components and their changed + * fields, and points at the change request for the full diff. `driftClearedCommentBody` is the + * counterpart once the drift is gone. + */ +export function driftReportCommentBody( + drift: DriftEntry[], + runningBitVersion: string, + anchors: DriftReportAnchors +): string { + const recorded = [...new Set(drift.map((d) => d.recordedBitVersion).filter(Boolean))].join(', '); + const causeLine = recorded + ? `Cause: the dependency context changed (recorded with bit ${recorded}, running bit ${runningBitVersion}).` + : `Cause: the dependency context changed.`; + const anchorLine = + `Branch \`${anchors.branch}\` @ \`${anchors.branchTipSha.slice(0, 9)}\` synced onto lane ` + + `\`${anchors.laneIdStr}\` @ \`${anchors.laneHead.slice(0, 9)}\`.`; + const entries = drift.map((d) => `- \`${d.id.toStringWithoutVersion()}\` — ${d.changedKeys.join(', ')}`); + const list = + entries.length > MAX_LISTED_DRIFT_COMPONENTS + ? [ + ...entries.slice(0, MAX_LISTED_DRIFT_COMPONENTS), + `- …and ${entries.length - MAX_LISTED_DRIFT_COMPONENTS} more`, + ] + : entries; + return [ + DRIFT_COMMENT_MARKER, + '### Dependency-context drift', + '', + causeLine, + anchorLine, + 'This snap carries the components below as a side effect of the committed context. Git does not show this change.', + '', + ...list, + '', + 'The change request on Bit Cloud shows the full diff.', + ].join('\n'); +} + +/** The drift-report comment, updated in place once its drift clears. Never deleted (spec 5.6). */ +export function driftClearedCommentBody(): string { + return [ + DRIFT_COMMENT_MARKER, + '### Dependency-context drift', + '', + 'The dependency-context drift this comment reported is gone. The lane matches its dependency context.', + ].join('\n'); +} diff --git a/scopes/git/ci/sync/git-host-provider.ts b/scopes/git/ci/sync/git-host-provider.ts index ae6705add384..a484347eb9e9 100644 --- a/scopes/git/ci/sync/git-host-provider.ts +++ b/scopes/git/ci/sync/git-host-provider.ts @@ -33,6 +33,14 @@ export interface GitHostProvider { comment(prNumber: number, body: string): Promise; addLabel(prNumber: number, label: string): Promise; + + /** + * Update the comment containing `marker` in place, or post a new one when none exists yet + * (`options.createIfAbsent` defaults to true). Optional: a provider that doesn't implement this + * is treated as not supporting the drift-report surface — callers skip rather than fall back to a + * plain, never-updated `comment`, which would leave one stale copy per push. + */ + upsertComment?(prNumber: number, marker: string, body: string, options?: { createIfAbsent?: boolean }): Promise; } /** The git host to use this run, or nothing plus the reason PR operations are being skipped. */ diff --git a/scopes/git/ci/sync/github-client.spec.ts b/scopes/git/ci/sync/github-client.spec.ts index 93ab4d18a0b4..964dec694364 100644 --- a/scopes/git/ci/sync/github-client.spec.ts +++ b/scopes/git/ci/sync/github-client.spec.ts @@ -171,4 +171,92 @@ describe('GitHubClient', () => { const client = new GitHubClient({ token: 'tok', repo: 'acme/shop', fetchImpl: fakeFetch }); expect(await client.findPrByBranch('lane-x')).to.equal(undefined); }); + + describe('upsertComment', () => { + // named `upsertComment`, matching `GitHostProvider.upsertComment` exactly — see the method's + // own doc comment for why a differently-named method would be a silent-no-op trap. + function fakeFetchOver(existingComments: Array<{ id: number; body: string }>) { + const calls: Array<{ url: string; init: any }> = []; + const fetchImpl = (async (url: any, init: any) => { + calls.push({ url: String(url), init }); + const method = init?.method ?? 'GET'; + if (method === 'GET') { + return new Response(JSON.stringify(existingComments), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + return new Response(JSON.stringify({}), { status: 200, headers: { 'content-type': 'application/json' } }); + }) as typeof fetch; + return { calls, fetchImpl }; + } + + it('posts a new comment when no marked comment exists', async () => { + const { calls, fetchImpl } = fakeFetchOver([]); + const client = new GitHubClient({ token: 'tok', repo: 'acme/shop', fetchImpl }); + await client.upsertComment(7, '', '\nbody'); + const posts = calls.filter((c) => c.init?.method === 'POST'); + expect(posts).to.have.lengthOf(1); + expect(posts[0].url).to.equal('https://api.github.com/repos/acme/shop/issues/7/comments'); + }); + + it('patches the marked comment in place, and never posts a second one', async () => { + const { calls, fetchImpl } = fakeFetchOver([ + { id: 42, body: 'unrelated comment' }, + { id: 99, body: '\nold report' }, + ]); + const client = new GitHubClient({ token: 'tok', repo: 'acme/shop', fetchImpl }); + await client.upsertComment(7, '', '\nnew report'); + const patches = calls.filter((c) => c.init?.method === 'PATCH'); + expect(patches).to.have.lengthOf(1); + expect(patches[0].url).to.equal('https://api.github.com/repos/acme/shop/issues/comments/99'); + expect(JSON.parse(patches[0].init.body)).to.deep.equal({ body: '\nnew report' }); + expect(calls.some((c) => c.init?.method === 'POST')).to.equal(false); + }); + + it('skips silently when createIfAbsent is false and no marked comment exists', async () => { + const { calls, fetchImpl } = fakeFetchOver([]); + const client = new GitHubClient({ token: 'tok', repo: 'acme/shop', fetchImpl }); + await client.upsertComment(7, '', 'cleared', { createIfAbsent: false }); + expect(calls.some((c) => c.init?.method === 'POST' || c.init?.method === 'PATCH')).to.equal(false); + }); + + it('lists at per_page=100 and follows Link-header pagination to find a comment past page 1', async () => { + // GitHub defaults to per_page=30; a naive single-page list would miss this comment and post a + // duplicate report on every push instead of updating it. + const calls: Array<{ url: string; init: any }> = []; + const fetchImpl = (async (url: any, init: any) => { + calls.push({ url: String(url), init }); + const method = init?.method ?? 'GET'; + if (method === 'GET' && !String(url).includes('page=2')) { + return new Response(JSON.stringify([{ id: 1, body: 'unrelated' }]), { + status: 200, + headers: { + 'content-type': 'application/json', + link: '; rel="next"', + }, + }); + } + if (method === 'GET') { + return new Response(JSON.stringify([{ id: 99, body: '\nold report' }]), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + return new Response(JSON.stringify({}), { status: 200, headers: { 'content-type': 'application/json' } }); + }) as typeof fetch; + const client = new GitHubClient({ token: 'tok', repo: 'acme/shop', fetchImpl }); + await client.upsertComment(7, '', '\nnew report'); + + const gets = calls.filter((c) => (c.init?.method ?? 'GET') === 'GET'); + expect(gets).to.have.lengthOf(2); + expect(gets[0].url).to.include('per_page=100'); + expect(gets[1].url).to.equal('https://api.github.com/repos/acme/shop/issues/7/comments?per_page=100&page=2'); + const patches = calls.filter((c) => c.init?.method === 'PATCH'); + expect(patches).to.have.lengthOf(1); + expect(patches[0].url).to.equal('https://api.github.com/repos/acme/shop/issues/comments/99'); + // the found-on-page-2 comment was updated, not duplicated + expect(calls.some((c) => c.init?.method === 'POST')).to.equal(false); + }); + }); }); diff --git a/scopes/git/ci/sync/github-client.ts b/scopes/git/ci/sync/github-client.ts index 82a07c1e535f..9f3e3c3e419c 100644 --- a/scopes/git/ci/sync/github-client.ts +++ b/scopes/git/ci/sync/github-client.ts @@ -32,6 +32,13 @@ export function isGitHubRemote(remoteUrl: string): boolean { const API = 'https://api.github.com'; +/** The `rel="next"` URL out of a GitHub `Link` response header, or undefined on the last page. */ +function nextPageUrl(linkHeader: string | null): string | undefined { + if (!linkHeader) return undefined; + const next = linkHeader.split(',').find((part) => part.includes('rel="next"')); + return next?.match(/<([^>]+)>/)?.[1]; +} + /** Warning sink; a plain callback rather than a `Logger` so this module needs no logger aspect. */ export type WarnFn = (message: string) => void; @@ -85,8 +92,10 @@ export class GitHubClient implements GitHostProvider { return Boolean(this.token && this.repo); } - private async request(method: string, path: string, body?: unknown): Promise { - const res = await this.fetchImpl(`${API}/repos/${this.repo}${path}`, { + /** `pathOrUrl` may be a path relative to this repo, or an absolute URL (a `Link` header's next page). */ + private async requestRaw(method: string, pathOrUrl: string, body?: unknown): Promise { + const url = /^https?:\/\//i.test(pathOrUrl) ? pathOrUrl : `${API}/repos/${this.repo}${pathOrUrl}`; + const res = await this.fetchImpl(url, { method, headers: { authorization: `Bearer ${this.token}`, @@ -98,8 +107,13 @@ export class GitHubClient implements GitHostProvider { }); if (!res.ok) { const text = await res.text().catch(() => ''); - throw new Error(`GitHub API ${method} ${path} failed: ${res.status} ${text}`); + throw new Error(`GitHub API ${method} ${pathOrUrl} failed: ${res.status} ${text}`); } + return res; + } + + private async request(method: string, path: string, body?: unknown): Promise { + const res = await this.requestRaw(method, path, body); return res.status === 204 ? undefined : res.json(); } @@ -135,6 +149,52 @@ export class GitHubClient implements GitHostProvider { async addLabel(prNumber: number, label: string): Promise { await this.request('POST', `/issues/${prNumber}/labels`, { labels: [label] }); } + + /** + * Every comment on the PR, across all pages. GitHub defaults `per_page` to 30; a marked comment + * living past page 1 would otherwise read as absent, and `upsertComment` would post a duplicate on + * every push instead of updating the existing one. Follows the `Link: rel="next"` header — a page + * with no such link is the last one. + */ + private async listIssueComments(prNumber: number): Promise<{ id: number; body: string }[]> { + const comments: { id: number; body: string }[] = []; + let next: string | undefined = `/issues/${prNumber}/comments?per_page=100`; + // Defensive cap: 20 pages (2,000 comments) is far past any real PR; without it a malformed or + // adversarial `Link` header could loop this call forever. + for (let page = 0; next && page < 20; page += 1) { + const res: Response = await this.requestRaw('GET', next); + const body = (await res.json()) as any[]; + comments.push(...body.map((c: any) => ({ id: c.id, body: c.body ?? '' }))); + next = nextPageUrl(res.headers.get('link')); + } + return comments; + } + + /** + * Find the comment carrying `marker` (an HTML comment embedded in the body) and replace its body, + * or post `body` as a new comment when none exists and `options.createIfAbsent` is not false. + * GitHub's comment PATCH endpoint is `/issues/comments/{id}`, not `/issues/{pr}/comments/{id}` — + * a comment id is unique per repository, not scoped under the issue/PR that carries it. + * + * Named to match `GitHostProvider.upsertComment` exactly (not e.g. `upsertIssueComment`): a + * `GitHubClient` registered directly as a provider must satisfy the interface's optional method + * under its real name, or callers that feature-test via `gitHost.upsertComment` would silently + * treat a fully-capable client as unsupported. + */ + async upsertComment( + prNumber: number, + marker: string, + body: string, + options: { createIfAbsent?: boolean } = {} + ): Promise { + const existing = (await this.listIssueComments(prNumber)).find((c) => c.body.includes(marker)); + if (existing) { + await this.request('PATCH', `/issues/comments/${existing.id}`, { body }); + return; + } + if (options.createIfAbsent === false) return; + await this.comment(prNumber, body); + } } /** @@ -191,6 +251,15 @@ export class GitHubHostProvider implements GitHostProvider { return this.requireClient().addLabel(prNumber, label); } + async upsertComment( + prNumber: number, + marker: string, + body: string, + options?: { createIfAbsent?: boolean } + ): Promise { + return this.requireClient().upsertComment(prNumber, marker, body, options); + } + private resolveClient(remoteUrl?: string): GitHubClient | undefined { if (!this.client) this.client = GitHubClient.fromEnv(remoteUrl ?? this.remoteHint, this.onWarning); return this.client; diff --git a/scopes/git/ci/sync/lane-sync-executor.spec.ts b/scopes/git/ci/sync/lane-sync-executor.spec.ts index 60082a2d2be7..e346840fc392 100644 --- a/scopes/git/ci/sync/lane-sync-executor.spec.ts +++ b/scopes/git/ci/sync/lane-sync-executor.spec.ts @@ -15,6 +15,7 @@ import { laneSyncPrBody, } from './lane-sync-executor'; import { resolveSyncConfig } from './sync-config'; +import { DRIFT_COMMENT_MARKER } from './context-drift'; type LaneComponents = Parameters[0]; @@ -256,6 +257,105 @@ describe('syncLane outer catch under --dry-run', () => { }); }); +describe('surfaceDriftOnPr', () => { + const ANCHORS = { + branch: 'drift-lane', + branchTipSha: 'abcdef0123456789', + laneIdStr: 'acme.shop/drift-lane', + laneHead: '0123456789abcdef', + }; + const DRIFT = [{ id: { toStringWithoutVersion: () => 'acme.shop/comp2' }, changedKeys: ['packageDependencies'] }]; + + /** `upsertCalls` records every `upsertComment` invocation; `findPrCalls` every PR lookup. */ + function executorWith(opts: { + gitHost?: 'none' | 'no-upsert' | 'ok' | 'throws'; + upsertCalls?: any[]; + findPrCalls?: string[]; + }) { + const upsertCalls = opts.upsertCalls ?? []; + const findPrCalls = opts.findPrCalls ?? []; + const warnings: string[] = []; + const noopLogger = { + console: () => {}, + consoleWarning: (m: string) => warnings.push(m), + error: () => {}, + debug: () => {}, + }; + const findPrByBranch = async (branch: string) => { + findPrCalls.push(branch); + return { number: 7, htmlUrl: 'https://example.test/pr/7', labels: [] }; + }; + const gitHost = + opts.gitHost === 'none' + ? undefined + : opts.gitHost === 'no-upsert' + ? ({ name: 'stub', findPrByBranch } as any) + : ({ + name: 'stub', + findPrByBranch, + upsertComment: async (...args: any[]) => { + upsertCalls.push(args); + if (opts.gitHost === 'throws') throw new Error('rate limited'); + }, + } as any); + const executor = new LaneSyncExecutor({ + lanes: {} as any, + ci: { getRunningBitVersion: () => '2.0.69' } as any, + logger: noopLogger as any, + gitHost, + cfg: resolveSyncConfig({}), + defaultScope: 'acme.shop', + }); + return { executor, upsertCalls, findPrCalls, warnings }; + } + + it('drift present: upserts a report body naming the synced branch/lane pair and the drifted component', async () => { + const { executor, upsertCalls } = executorWith({ gitHost: 'ok' }); + await (executor as any).surfaceDriftOnPr({ ...ANCHORS, drift: DRIFT }); + expect(upsertCalls).to.have.lengthOf(1); + const [prNumber, marker, body, options] = upsertCalls[0]; + expect(prNumber).to.equal(7); + expect(marker).to.equal(DRIFT_COMMENT_MARKER); + expect(body).to.include(ANCHORS.branch); + expect(body).to.include(ANCHORS.branchTipSha.slice(0, 9)); + expect(body).to.include(ANCHORS.laneIdStr); + expect(body).to.include(ANCHORS.laneHead.slice(0, 9)); + expect(body).to.include('acme.shop/comp2'); + expect(options).to.equal(undefined); // createIfAbsent defaults to true — a report must always land + }); + + it('drift empty: upserts the cleared body with createIfAbsent false — never posts a fresh "cleared" comment', async () => { + const { executor, upsertCalls } = executorWith({ gitHost: 'ok' }); + await (executor as any).surfaceDriftOnPr({ ...ANCHORS, drift: [] }); + expect(upsertCalls).to.have.lengthOf(1); + const [, marker, , options] = upsertCalls[0]; + expect(marker).to.equal(DRIFT_COMMENT_MARKER); + expect(options).to.deep.equal({ createIfAbsent: false }); + }); + + it('no configured git host: skips silently, without even looking up the PR', async () => { + const { executor, upsertCalls, findPrCalls } = executorWith({ gitHost: 'none' }); + await (executor as any).surfaceDriftOnPr({ ...ANCHORS, drift: DRIFT }); + expect(upsertCalls).to.deep.equal([]); + expect(findPrCalls).to.deep.equal([]); + }); + + it('a provider without upsertComment: skips silently, the same as no provider', async () => { + const { executor, upsertCalls, findPrCalls } = executorWith({ gitHost: 'no-upsert' }); + await (executor as any).surfaceDriftOnPr({ ...ANCHORS, drift: DRIFT }); + expect(upsertCalls).to.deep.equal([]); + expect(findPrCalls).to.deep.equal([]); + }); + + it('the git host API throws: warns, and does not fail the caller', async () => { + const { executor, warnings } = executorWith({ gitHost: 'throws' }); + await (executor as any).surfaceDriftOnPr({ ...ANCHORS, drift: DRIFT }); // must not reject + expect(warnings).to.have.lengthOf(1); + expect(warnings[0]).to.include('rate limited'); + expect(warnings[0]).to.include('#7'); + }); +}); + // bit's lane-name charset admits `$`, `-`, `_` and `!`, so an unquoted runbook shell-expands when pasted. describe('haltCommentBody', () => { const LANE_NAME = 'fix-$home-and-!bang'; diff --git a/scopes/git/ci/sync/lane-sync-executor.ts b/scopes/git/ci/sync/lane-sync-executor.ts index 4be0260411b0..68050859cdf1 100644 --- a/scopes/git/ci/sync/lane-sync-executor.ts +++ b/scopes/git/ci/sync/lane-sync-executor.ts @@ -1,14 +1,16 @@ import chalk from 'chalk'; -import { formatWarningSummary } from '@teambit/cli'; +import { formatWarningSummary, formatSection, formatItem } from '@teambit/cli'; import type { Logger } from '@teambit/logger'; import type { LanesMain } from '@teambit/lanes'; import type { LaneData } from '@teambit/legacy.scope'; import { getCloudDomain } from '@teambit/legacy.constants'; import { FileStatus, type MergeStrategy } from '@teambit/component.modules.merge-helper'; import { git } from '../git'; -import type { CiMain } from '../ci.main.runtime'; +import type { CiMain, ContextDriftReport } from '../ci.main.runtime'; import type { CiSyncConfig, LaneTarget } from './sync-config'; import { laneNameToBranch } from './sync-config'; +import { DRIFT_COMMENT_MARKER, driftClearedCommentBody, driftReportCommentBody } from './context-drift'; +import type { DriftReportAnchors } from './context-drift'; import type { BranchSyncState } from './sync-state'; import { CONFLICT_LABEL, @@ -445,6 +447,17 @@ export class LaneSyncExecutor { pr, }); case 'export-branch': + // The tip is already this reconciler's own commit — it already confirmed everything up to + // it (that is what writing it means), and `hasDevCommits`/`stateCommit` cannot tell a real + // dev commit from one that touches no bit-tracked file (docs, CI config): `stateCommit` is + // derived from `.bitmap`'s content, never from commit messages (sync-state.ts), so a commit + // that leaves `.bitmap` byte-identical never advances it, however many runs re-confirm + // "nothing to snap" on top. Recognizing our own tip here — not by loosening `hasDevCommits` + // itself, which the ownership/retirement path also reads and must stay strict — is what + // makes that settle instead of re-planning `export-branch` forever. + if (tipIsSyncCommit) { + return `${laneName} -> noop (converged; branch tip is already this reconciler's own sync commit)`; + } return this.executeExportBranch({ target, laneIdStr, branch, defaultBranch }); case 'merge-diverged': return this.executeMergeDiverged({ target, laneIdStr, branch, defaultBranch }); @@ -610,7 +623,7 @@ export class LaneSyncExecutor { await this.checkoutFromRemote(branch, `origin/${branch}`); try { - const exportErr = await this.snapAndExportOntoLane(laneIdStr, message); + const { error: exportErr, drift, noop } = await this.snapAndExportOntoLane(laneIdStr, message); if (exportErr) { // Halt rather than propagate: one lane's failed snap/export must not abort the lanes after it. return await this.executeHalt({ @@ -621,17 +634,29 @@ export class LaneSyncExecutor { pr: await this.findPr(branch), }); } - - const laneHead = await this.recordLaneHeadOnBranch(target, laneIdStr, branch); - if (!laneHead) { + // Still write the sync ledger on a noop: without it, `stateCommit` (sync-state.ts) never + // advances past a commit that touches no bit-tracked file (docs, CI config), and the planner + // re-plans `export-branch` on every future run forever — cheap per run, but a standing loop + // that never settles. `recordLaneHeadOnBranch`'s commit is `--allow-empty`. + const recorded = await this.recordLaneHeadOnBranch(target, laneIdStr, branch); + if (!recorded) { return await this.executeHalt({ laneName, laneIdStr, branch, - reason: `lane ${laneIdStr} could not be read back from the remote after export`, + reason: noop + ? `lane ${laneIdStr} could not be read back from the remote while recording the no-op sync ledger` + : `lane ${laneIdStr} could not be read back from the remote after export`, pr: await this.findPr(branch), }); } + if (noop) { + return `${laneName} -> export-branch (branch ${branch} has no bit-tracked change; nothing snapped onto lane ${laneIdStr})`; + } + const { laneHead, branchTipSha } = recorded; + // After recordLaneHeadOnBranch, not before: the report names the branch tip and lane head this + // run just produced, and neither exists until the commit+push above lands. + await this.surfaceDriftOnPr({ branch, branchTipSha, laneIdStr, laneHead, drift }); return `${laneName} -> export-branch (lane ${laneIdStr} @ ${laneHead.slice(0, 9)}, branch ${branch} updated)`; } finally { await this.restoreWorkspace(defaultBranch); @@ -728,7 +753,12 @@ export class LaneSyncExecutor { } // ---- step 2: snap + export the merged tree onto the lane --------------------------------- - const exportErr = await this.snapAndExportOntoLane( + // `noop` (see `snapAndExportOntoLane`) is deliberately IGNORED here, unlike in + // `executeExportBranch`: step 1's checkout can rewrite `.bitmap` and file content to match an + // ALREADY-recorded version (e.g. a full-file conflict resolved by "theirs") — nothing new to + // snap, but a real git-level change the branch does not have yet. Skipping the branch commit + // on `noop` would silently drop that change. + const { error: exportErr, drift } = await this.snapAndExportOntoLane( laneIdStr, `merge remote lane ${laneIdStr} into ${branch} ${SYNC_COMMIT_MARKER}` ); @@ -740,10 +770,13 @@ export class LaneSyncExecutor { } // ---- step 3: record the new lane head on the branch -------------------------------------- - const laneHead = await this.recordLaneHeadOnBranch(target, laneIdStr, branch); - if (!laneHead) { + const recorded = await this.recordLaneHeadOnBranch(target, laneIdStr, branch); + if (!recorded) { return await halt(`lane ${laneIdStr} could not be read back from the remote after the merge export`); } + const { laneHead, branchTipSha } = recorded; + // After recordLaneHeadOnBranch, not before — see executeExportBranch for why. + await this.surfaceDriftOnPr({ branch, branchTipSha, laneIdStr, laneHead, drift }); return ( `${laneName} -> merge-diverged (${policyClause}merged lane into branch, then exported; lane ${laneIdStr} @ ` + `${laneHead.slice(0, 9)}, branch ${branch} updated)` @@ -759,17 +792,51 @@ export class LaneSyncExecutor { } /** - * Snap the workspace's current tree onto the lane and export it; returns the error so the caller can - * halt. Snaps WHATEVER is in the workspace (the switch uses `forceOurs` and never merges files), so - * a diverged tree must already hold the merged content. `keepLane` preserves the lane's history, - * `skipCleanup` keeps the snap's `.bitmap` for the caller to commit, `noDestructiveRecovery` turns - * stale-lane recovery (delete + re-fork the remote lane) into a throw. The lane object must be - * imported BEFORE delegating: a switch onto the lane the workspace is already on no-ops before any - * fetch, so it never warms a cold scope. + * Snap the workspace's current tree onto the lane and export it; returns the error (if any) and the + * drift the run detected, so the caller can halt or surface the report. Snaps WHATEVER is in the + * workspace (the switch uses `forceOurs` and never merges files), so a diverged tree must already + * hold the merged content. `keepLane` preserves the lane's history, `skipCleanup` keeps the snap's + * `.bitmap` for the caller to commit, `noDestructiveRecovery` turns stale-lane recovery (delete + + * re-fork the remote lane) into a throw. The lane object must be imported BEFORE delegating: a + * switch onto the lane the workspace is already on no-ops before any fetch, so it never warms a + * cold scope. + * + * Lanes carry the fan-out (spec decision 6): the snap covers the full pending set — the + * git-authored change and any dependency-context drift — under the developer's own message. Status + * verification still scopes to the git-authored subset (`verifyIds`); the drifted subset's + * pre-existing blockers are tolerated at snap time via `driftIds` (see `blockerNamesUnion`), never + * at verification time — a *new* blocker on a component the developer touched still fails the run. + * + * `noop: true` when neither set has anything (the old cheap path, restored): a docs-only commit or + * an already-converged merge touches no bit component, and `snapPrCommit` — full status, import, + * lane creation/reuse, config sync — is pure overhead for that case, minutes of it on a large + * workspace. The caller skips its own follow-up (drift comment, `.bitmap` sync commit) too: there + * is nothing to report and nothing new to record on the branch. */ - private async snapAndExportOntoLane(laneIdStr: string, message: string): Promise { + private async snapAndExportOntoLane( + laneIdStr: string, + message: string + ): Promise<{ error?: Error; drift: ContextDriftReport['drift']; noop?: boolean }> { try { await ensureCurrentLaneObject(this.deps.lanes); + const { drift, gitAuthored } = await this.deps.ci.detectContextDrift(); + if (!gitAuthored.length && !drift.length) { + this.deps.logger.console(chalk.yellow('No changes detected, nothing to snap')); + return { drift: [], noop: true }; + } + if (drift.length) { + const running = this.deps.ci.getRunningBitVersion(); + const recorded = [...new Set(drift.map((d) => d.recordedBitVersion).filter(Boolean))].join(', '); + this.deps.logger.console( + formatSection( + 'dependency-context drift', + `${drift.length} component(s) carry dependency-context drift` + + `${recorded ? ` (recorded with bit ${recorded}, running bit ${running})` : ''}` + + ` — snapped as side effects of the committed context; see the PR report:`, + drift.map((d) => formatItem(`${d.id.toStringWithoutVersion()} (${d.changedKeys.join(', ')})`)) + ) + ); + } await this.deps.ci.snapPrCommit({ laneIdStr, message, @@ -778,28 +845,72 @@ export class LaneSyncExecutor { keepLane: true, skipCleanup: true, noDestructiveRecovery: true, + verifyIds: gitAuthored.map((id) => id.toStringWithoutVersion()), + driftIds: drift.map((d) => d.id.toStringWithoutVersion()), }); - return undefined; + return { drift }; + } catch (e: any) { + return { error: e instanceof Error ? e : new Error(String(e?.message ?? e)), drift: [] }; + } + } + + /** + * Upsert the drift-report comment on the branch's PR (spec 5.6): the enumerated effect of the + * fan-out the snap just committed. Silently a no-op with no configured git host (the run-log + * report above is the fallback surface), no open PR yet, or a provider that doesn't implement + * comment upserts. Never fails the lane — a report is a courtesy, not a gate. `anchors` must come + * from `recordLaneHeadOnBranch`'s result — the branch tip and lane head it names don't exist + * until that commit+push lands, so callers invoke this AFTER it, never before. + */ + private async surfaceDriftOnPr({ + branch, + branchTipSha, + laneIdStr, + laneHead, + drift, + }: DriftReportAnchors & { drift: ContextDriftReport['drift'] }): Promise { + const { gitHost, logger } = this.deps; + const upsertComment = gitHost?.upsertComment?.bind(gitHost); + if (!upsertComment) return; + const pr = await this.findPr(branch); + if (!pr) return; + try { + if (drift.length) { + const running = this.deps.ci.getRunningBitVersion(); + await upsertComment( + pr.number, + DRIFT_COMMENT_MARKER, + driftReportCommentBody(drift, running, { branch, branchTipSha, laneIdStr, laneHead }) + ); + } else { + // Only update a report that already exists — a lane with no drift that never had one must + // not gain a "drift cleared" comment out of nowhere. + await upsertComment(pr.number, DRIFT_COMMENT_MARKER, driftClearedCommentBody(), { createIfAbsent: false }); + } } catch (e: any) { - return e instanceof Error ? e : new Error(String(e?.message ?? e)); + logger.consoleWarning( + `Could not update the dependency-context drift report on PR #${pr.number}: ${e?.message || e}` + ); } } /** * Record on the branch which lane state it now mirrors: re-query the lane (the export just moved it, * so any earlier fingerprint is stale), commit — crucially the `.bitmap` the snap rewrote — and push. - * Returns undefined when the lane can no longer be read, in which case the caller halts. + * Returns undefined when the lane can no longer be read, in which case the caller halts. The + * returned `branchTipSha` is the pushed commit — the anchor a drift report names alongside + * `laneHead`, so the two only exist together once this call succeeds (see `surfaceDriftOnPr`). */ private async recordLaneHeadOnBranch( target: LaneTarget, laneIdStr: string, branch: string - ): Promise { + ): Promise<{ laneHead: string; branchTipSha: string } | undefined> { const remoteLane = await this.getRemoteLane(target); if (!remoteLane) return undefined; const laneHead = laneHeadFingerprint(remoteLane.components); - await this.commitAllAndPush(branch, buildSyncCommitMessage(laneIdStr, laneHead)); - return laneHead; + const branchTipSha = await this.commitAllAndPush(branch, buildSyncCommitMessage(laneIdStr, laneHead)); + return { laneHead, branchTipSha }; } /** @@ -1213,13 +1324,16 @@ export class LaneSyncExecutor { * push means someone pushed concurrently and the next run should re-plan rather than clobber. * `--allow-empty` is only insurance against `git commit` failing the lane outright. */ - private async commitAllAndPush(branch: string, message: string) { + /** Returns the pushed commit's sha — the branch tip anchor a drift report names. */ + private async commitAllAndPush(branch: string, message: string): Promise { await addAllExceptScopeAndModules(); await commitWithIdentity(message, { extraArgs: ['--allow-empty'] }); + const tipSha = (await git.revparse(['HEAD'])).trim(); // `HEAD:refs/heads/`: a full-ref destination cannot be resolved as a tag or reinterpreted — // the configured branch name is user input (see `sync-config.ts`). await git.push(['origin', `HEAD:refs/heads/${branch}`]); this.deps.logger.console(chalk.green(`Pushed ${branch}`)); + return tipSha; } private async openPrForLane({ diff --git a/scopes/git/ci/sync/main-config-sync.ts b/scopes/git/ci/sync/main-config-sync.ts new file mode 100644 index 000000000000..e1ba2d64e83e --- /dev/null +++ b/scopes/git/ci/sync/main-config-sync.ts @@ -0,0 +1,246 @@ +import chalk from 'chalk'; +import { compact } from 'lodash'; +import type { ComponentID } from '@teambit/component-id'; +import type { Workspace } from '@teambit/workspace'; +import type { LanesMain } from '@teambit/lanes'; +import type { ImporterMain } from '@teambit/importer'; +import type { Logger } from '@teambit/logger'; +import type { LaneId } from '@teambit/lane-id'; +import type { MergeStrategy } from '@teambit/component.modules.merge-helper'; +import { getDivergeData } from '@teambit/component.snap-distance'; +import { ComponentConfigMerger } from '@teambit/config-merger'; +import { DependencyResolverAspect } from '@teambit/dependency-resolver'; +import { pMapPool } from '@teambit/toolbox.promise.map-pool'; +import { concurrentComponentsLimit } from '@teambit/harmony.modules.concurrency'; + +export type MainConfigSyncDeps = { + workspace: Workspace; + lanes: LanesMain; + importer: ImporterMain; + logger: Logger; +}; + +/** + * Batch-fetch main-side Version objects the config merge needs (heads, then common ancestors), + * mirroring the lane-merge flows (merge-status-provider / merge-lanes). Best-effort: a fetch + * hiccup shouldn't abort `bit ci pr` — the merge loop still runs and any component whose object is + * still missing just logs the existing per-component skip. `label` names which objects for the log. + */ +async function prefetchFromMainForConfigSync( + { importer, logger }: MainConfigSyncDeps, + ids: ComponentID[], + label: string +) { + if (!ids.length) return; + try { + await importer.importObjectsFromMainIfExist(ids, { cache: true }); + } catch (e: any) { + logger.console( + chalk.yellow(`Could not pre-fetch main's ${label} for config sync (continuing): ${e?.message || e}`) + ); + } +} + +/** + * Copied from `merging.main.runtime` (`filterDeletedDependenciesFromConfig`): the config merge + * can emit deletion markers (`version: '-'`) for deps removed on main. The aspects-merger applies + * `mergedConfig` verbatim, so strip those here to avoid writing a policy entry with version '-'. + */ +function filterDeletedDependenciesFromConfig(mergeConfig?: Record): void { + const policy: Record> | undefined = + mergeConfig?.[DependencyResolverAspect.id]?.policy; + if (!policy) return; + Object.keys(policy).forEach((depType) => { + const filtered = policy[depType].filter((dep) => dep.version !== '-'); + if (filtered.length === 0) delete policy[depType]; + else policy[depType] = filtered; + }); +} + +/** + * Sync *config-only* changes from main onto the lane — without a full `bit lane merge`. + * + * In this workflow git is the source of truth for files: the PR author merges the default branch + * into their PR branch, so source changes arrive via git. The one thing git can't carry is + * config that's already been *tagged into objects* on main — e.g. another PR ran `bit env set` / + * `bit deps set`; those records lived in `.bitmap`, rode git into main, and `bit ci merge` baked + * them into the component's Version (clearing them from `.bitmap`). A long-running PR's lane + * would otherwise miss them. + * + * A full lane merge is the wrong tool here: it does a 3-way *file* merge and refuses to run while + * the workspace has modified components — but in `bit ci pr` the workspace is always dirty (the + * PR's changes, not yet snapped). So instead we do a per-component 3-way merge of the aspect + * *config only* (base = common ancestor, ours = lane, theirs = main), keeping the PR's config on + * conflict, and stash the result on an `unmergedComponents` entry's `mergedConfig`. The + * subsequent `snap` reads it (via the aspects-merger on component load) and bakes main's config + * into the new snap, while the snap's files still come from the workspace (git). No file + * checkout, so no clean-workspace requirement. + */ +export async function syncConfigFromMain(deps: MainConfigSyncDeps, laneId: LaneId) { + const { workspace, lanes, logger } = deps; + const legacyScope = workspace.scope.legacyScope; + const repo = legacyScope.objects; + const mainLaneId = lanes.getDefaultLaneId(); + const currentLane = await lanes.getCurrentLane(); + if (!currentLane) return; + const workspaceIds = workspace.listIds(); + + logger.console(chalk.blue(`Syncing config changes from ${mainLaneId.toString()} into ${laneId.toString()}`)); + + // Resolve each lane component's head on main once, keeping only those that are on main and whose + // lane head differs from it (the rest have nothing to sync). This single pass feeds both the + // pre-fetch below and the merge loop, so we never load the same ModelComponent twice. + const componentsToSync = compact( + await Promise.all( + currentLane.components.map(async (laneComp) => { + try { + const modelComponent = await legacyScope.getModelComponentIfExist(laneComp.id); + const mainHead = modelComponent?.head; // the component's head on main + if (!modelComponent || !mainHead || mainHead.isEqual(laneComp.head)) return undefined; + return { laneComp, modelComponent, mainHead }; + } catch (e: any) { + // Best-effort per component (same contract as the merge loop below): one component's + // load failure shouldn't reject Promise.all and abort the whole config sync. + logger.console( + chalk.yellow( + ` ${laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})` + ) + ); + return undefined; + } + }) + ) + ); + + // The lane import (switchToLane) brought each component's lane history plus the lightweight + // version-history (the parent graph) — that's enough for the diverge check below to see that + // main is ahead — but NOT the full Version object for main's head wherever main advanced past + // the lane's fork point. Those objects live only on main and were never fetched. Without them + // `loadVersion(mainHead)` throws VersionNotFoundOnFS, the per-component catch swallows it as + // "skipping config sync from main", and the sync silently degrades to a no-op for every + // diverged component. Pre-fetch main's head objects in one batched remote call (mirroring the + // lane-merge flows — see merge-status-provider / merge-lanes). Pass the *specific* main-head + // version so `cache: true` still fetches it: the component already exists locally at its lane + // version, so a version-less id would look satisfied and skip the remote. + const mainHeadIds = componentsToSync.map(({ laneComp, mainHead }) => laneComp.id.changeVersion(mainHead.toString())); + await prefetchFromMainForConfigSync(deps, mainHeadIds, 'head objects'); + + // Resolve each component's diverge state up front — before the merge loop — so we can also + // pre-fetch the common-ancestor objects below. getDivergeData only walks the parent graph, + // which is already local (switchToLane brings the version-history, and the head pre-fetch above + // reinforced it), so no full Version object is needed yet. Keep only components where main is + // actually ahead or diverged; the rest have nothing to bring in from main. Bound the fan-out + // (getDivergeData traverses each component's version graph) so a lane with many components + // doesn't spawn one unbounded burst of concurrent graph walks. + const componentsToMerge = compact( + await pMapPool( + componentsToSync, + async (item) => { + try { + const divergeData = await getDivergeData({ + repo, + modelComponent: item.modelComponent, + sourceHead: item.laneComp.head, + targetHead: item.mainHead, + throws: false, + }); + if (!divergeData.isTargetAhead() && !divergeData.isDiverged()) return undefined; + return { ...item, divergeData }; + } catch (e: any) { + // Best-effort per component (same contract as the merge loop below). + logger.console( + chalk.yellow( + ` ${item.laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})` + ) + ); + return undefined; + } + }, + { concurrency: concurrentComponentsLimit() } + ) + ); + + // The head pre-fetch above brought main's head Version plus the version-history (parent graph), + // but NOT the full Version object of the common ancestor (the lane's fork point) for components + // where the lane and main have BOTH snapped since the fork. The 3-way config merge below loads + // that base Version (`baseVersion.extensions`); without it, `loadVersion(baseSnap)` throws + // VersionNotFoundOnFS, the per-component catch swallows it as "skipping config sync from main", + // and the sync silently no-ops for every diverged component. The fork point lives on main and, + // like the head, was never fetched (includeVersionHistory carries the graph, not each ancestor's + // Version). Batch-fetch the bases in one call — same best-effort contract as the head pre-fetch. + const baseIds = compact( + componentsToMerge.map(({ laneComp, divergeData }) => { + const baseSnap = divergeData.commonSnapBeforeDiverge; + return baseSnap ? laneComp.id.changeVersion(baseSnap.toString()) : undefined; + }) + ); + await prefetchFromMainForConfigSync(deps, baseIds, 'common-ancestor objects'); + + const syncedIds: ComponentID[] = []; + for (const { laneComp, modelComponent, mainHead, divergeData } of componentsToMerge) { + try { + const laneHead = laneComp.head; + const currentVersion = await modelComponent.loadVersion(laneHead.toString(), repo); + const otherVersion = await modelComponent.loadVersion(mainHead.toString(), repo); + // base = common ancestor. When the lane is strictly behind main (no divergence) the common + // ancestor IS the lane head, so the lane's own aspects serve as the base. + const baseSnap = divergeData.commonSnapBeforeDiverge; + const baseVersion = baseSnap ? await modelComponent.loadVersion(baseSnap.toString(), repo) : currentVersion; + + const configMerger = new ComponentConfigMerger( + laneComp.id.toStringWithoutVersion(), + workspaceIds, + undefined, // merging from main (the default lane) — there's no Lane object for it + currentVersion.extensions, + baseVersion.extensions, + otherVersion.extensions, + laneId.toString(), + mainLaneId.toString(), + logger, + 'ours' as MergeStrategy // keep the PR's config on a genuine conflict + ); + 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. + filterDeletedDependenciesFromConfig(mergedConfig); + + // Upsert: addEntry throws if an entry for this component already exists. A prior + // --keep-lane run that crashed mid-snap (or otherwise left unmerged.json entries behind) + // would otherwise make every later run throw here, skip the component, and keep serving + // stale config. Remove any existing entry first so repeated runs converge on main's latest. + legacyScope.objects.unmergedComponents.removeComponent(laneComp.id); + legacyScope.objects.unmergedComponents.addEntry({ + id: { scope: laneComp.id.scope, name: laneComp.id.fullName }, + head: mainHead, + laneId: mainLaneId, + mergedConfig, + }); + syncedIds.push(laneComp.id); + logger.console( + chalk.blue( + ` ${laneComp.id.toStringWithoutVersion()}: applying main's config (${Object.keys(mergedConfig).join(', ')})` + ) + ); + } catch (e: any) { + // Best-effort per component: one component's config-merge quirk shouldn't abort the whole + // `bit ci pr`. Log and move on — the build just won't reflect that component's main-side + // config this run. + logger.console( + chalk.yellow(` ${laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})`) + ); + } + } + + if (!syncedIds.length) { + logger.console(chalk.blue('No config changes from main to sync')); + return; + } + await legacyScope.objects.unmergedComponents.write(); + // The components were already loaded (and their aspects cached) earlier in this run, before the + // unmergedComponents entries existed. Clear the cache so the upcoming `snap` reloads them and + // the aspects-merger folds in the synced `mergedConfig`. + workspace.clearAllComponentsCache(); + logger.console(chalk.green(`Synced config from main for ${syncedIds.length} component(s)`)); +} diff --git a/scopes/git/ci/sync/main-sync-executor.ts b/scopes/git/ci/sync/main-sync-executor.ts index 2198ea023ffd..5984c536924e 100644 --- a/scopes/git/ci/sync/main-sync-executor.ts +++ b/scopes/git/ci/sync/main-sync-executor.ts @@ -129,10 +129,22 @@ export class MainSyncExecutor { ); } + // Consume dependency-context drift before diffing. The tag's .bitmap/lockfile writes then + // ride the same file diff `driftFiles()` computes below; there is no separate commit path. + await this.deps.ci.reloadWorkspaceFromDisk(); + const convergence = await this.deps.ci.convergeContextDrift({ dryRun: opts.dryRun }); + if (convergence.detected) logger.console(convergence.summary); + const drift = await this.driftFiles(); // Direct-push stays bare: asking the host about `mainSyncBranch`'s PR would be the one // interaction with it this mode promises not to make. - if (!drift.length) return directPush ? CONVERGED_SUMMARY : await this.convergedSummary(branch); + if (!drift.length) { + // A dry-run, or a detected-but-nothing-taggable convergence (converged: 0), also produces + // no file diff. Return `convergence.summary` for both, or the CONVERGED summary + // contradicts the line just logged. + if (convergence.detected && (opts.dryRun || !convergence.converged)) return `main -> ${convergence.summary}`; + return directPush ? CONVERGED_SUMMARY : await this.convergedSummary(branch); + } logger.console( formatWarningSummary(`main -> drift in ${drift.length} file(s): ${drift.slice(0, 20).join(', ')}`) diff --git a/scopes/git/ci/sync/sync-state.ts b/scopes/git/ci/sync/sync-state.ts index c0555d39f206..cc33efefeea3 100644 --- a/scopes/git/ci/sync/sync-state.ts +++ b/scopes/git/ci/sync/sync-state.ts @@ -86,8 +86,9 @@ export function hasSyncMarker(message: string): boolean { /** * Strict probe for "we wrote this commit": the marker alone on its own line. This is an input to branch - * deletion, so a developer merely quoting the marker must never count as authorship. `\r?` tolerates CRLF; - * a recognition failure errs toward keeping the branch. + * deletion and to the export-branch withhold (a branch whose tip is already our own commit settles + * instead of re-exporting), so a developer merely quoting the marker must never count as authorship. + * `\r?` tolerates CRLF; a recognition failure errs toward keeping the branch / re-attempting the export. */ export function isSyncAuthoredMessage(message: string): boolean { return new RegExp(`^${SYNC_COMMIT_MARKER.replace(/[[\]]/g, '\\$&')}\\r?$`, 'm').test(message);