Skip to content

Commit e8fe51a

Browse files
authored
fix(ci): pre-fetch common-ancestor objects for config sync in 'ci pr' (#10464)
`bit ci pr --keep-lane` kept logging `... was not found on the filesystem` / `skipping config sync from main` for every component, even after #10460. #10460 pre-fetched main's **head** Version object before `syncConfigFromMain`'s 3-way config merge. But that merge loads **three** Version objects per component: the lane head, main's head, and their **common ancestor** (the lane's fork point). On a fresh CI runner the fork-point object is never fetched — switching to the lane brings the parent *graph* (version-history), not each ancestor's full Version — so `loadVersion(baseSnap)` throws `VersionNotFoundOnFS`, the per-component catch swallows it, and the merge silently no-ops for every diverged component (which is all of them on a reused lane once main has advanced). Confirmed from a failing run: every reported hash is the fork-point tag (e.g. `1.0.1038`), not the current main head (`1.0.1041`) — main's head loaded fine, the base is what was missing. **Fix:** after the existing head pre-fetch, resolve each component's diverge state (the graph is already local) and batch-fetch the common-ancestor base objects too, before the merge loop. Same best-effort contract as the head pre-fetch; the base is always an ancestor of main's head, so it's always fetchable. Adds an e2e regression that reproduces the fresh-runner state by dropping the fork-point object before the second `ci pr` and asserts config sync still lands.
1 parent c905910 commit e8fe51a

2 files changed

Lines changed: 150 additions & 26 deletions

File tree

e2e/harmony/ci-commands.e2e.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,81 @@ describe('ci commands', function () {
365365
});
366366
});
367367

368+
/**
369+
* Regression for the "was not found on the filesystem" config-sync failure (#10460 follow-up).
370+
*
371+
* `syncConfigFromMain` does a 3-way merge that loads three Version objects per component: the
372+
* lane head, main's head, and their COMMON ANCESTOR (the lane's fork point). On a real CI runner
373+
* the workspace is fresh: switching to the lane fetches the lane heads plus the version-history
374+
* (the parent graph) — enough to compute divergence — but NOT the full Version object of the fork
375+
* point, which lives only on main. #10460 pre-fetched main's head; this test guards that the fork
376+
* point is pre-fetched too. We reproduce the fresh-runner state by deleting the fork-point object
377+
* from the local scope before the second `bit ci pr` — without the fix, loading it throws
378+
* VersionNotFoundOnFS, the sync silently skips the component, and main's config never lands.
379+
*/
380+
describe('bit ci pr syncs main config even when the fork-point object is not on the filesystem', () => {
381+
let secondPrOutput: string;
382+
let forkPointHash: string;
383+
before(() => {
384+
helper.scopeHelper.setWorkspaceWithRemoteScope();
385+
setupGitRemote();
386+
const defaultBranch = setupComponentsAndInitialCommit();
387+
// The fork point: comp1's head on main right now, before the lane snaps on top of it. This is
388+
// the common ancestor the config merge will need as its base.
389+
forkPointHash = helper.command.getHead('comp1');
390+
391+
// First PR commit + ci pr — lane forks here and snaps comp1 on top of the fork point.
392+
helper.command.runCmd('git checkout -b feature/fork-point-test');
393+
helper.fs.outputFile('comp1/comp1.js', 'console.log("pr commit 1");');
394+
helper.command.runCmd('git add comp1/comp1.js');
395+
helper.command.runCmd('git commit -m "feat: pr commit 1"');
396+
helper.command.runCmd('bit ci pr --keep-lane --message "first pr commit"');
397+
398+
// Main moves ahead with a deps-set config change, so the lane and main have both snapped since
399+
// the fork (they diverge — the merge base is the fork point, not either head).
400+
helper.command.runCmd(`git checkout ${defaultBranch}`);
401+
helper.npm.addFakeNpmPackage('is-positive', '1.0.0');
402+
helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: { 'is-positive': '1.0.0' } });
403+
helper.command.dependenciesSet('comp1', 'is-positive@1.0.0');
404+
helper.command.tagAllWithoutBuild();
405+
helper.command.export();
406+
helper.command.runCmd('git add .');
407+
helper.command.runCmd('git commit -m "chore: bump comp1 deps on main"');
408+
helper.command.runCmd(`git push origin ${defaultBranch}`);
409+
410+
// Simulate the fresh CI runner: it never fetched the fork-point Version object (only the lane
411+
// heads + parent graph). Assert it's actually present first — so a wrong hash or setup change
412+
// can't turn this into a no-op — then drop it so the merge base is missing on disk.
413+
helper.fs.expectFileToExist(helper.general.getHashPathOfObject(forkPointHash, true));
414+
helper.fs.deleteObject(helper.general.getHashPathOfObject(forkPointHash));
415+
416+
// Second PR commit + ci pr — must re-fetch the fork point and sync main's deps change.
417+
helper.command.runCmd('git checkout feature/fork-point-test');
418+
helper.command.runCmd(`git merge ${defaultBranch}`);
419+
helper.fs.outputFile('comp2/comp2.js', 'console.log("pr commit 2");');
420+
helper.command.runCmd('git add comp2/comp2.js');
421+
helper.command.runCmd('git commit -m "feat: pr commit 2"');
422+
secondPrOutput = helper.command.runCmd('bit ci pr --keep-lane --message "second pr commit"');
423+
});
424+
425+
it('should not skip config sync with a "was not found on the filesystem" error', () => {
426+
// The exact production symptom (VersionNotFoundOnFS on the missing fork point). The functional
427+
// outcome is covered by the config assertion below; this just guards the specific regression.
428+
expect(secondPrOutput).to.not.include('was not found on the filesystem');
429+
});
430+
431+
it("should still propagate main's deps-set config change to comp1 on the lane", () => {
432+
helper.command.switchLocalLane('feature-fork-point-test');
433+
const showConfig = helper.command.showAspectConfig('comp1', Extensions.dependencyResolver);
434+
const dep = showConfig.data.dependencies.find((d: any) => d.id === 'is-positive');
435+
expect(
436+
dep,
437+
`expected 'is-positive' on comp1's deps after merging main, got: ${JSON.stringify(showConfig.data.dependencies, null, 2)}`
438+
).to.exist;
439+
expect(dep.version).to.equal('1.0.0');
440+
});
441+
});
442+
368443
/**
369444
* Same as the deps-set propagation test, but for an `bit env set` on main. Env config has its own
370445
* strategy in the config merger, so it's worth covering explicitly: a long-running PR's lane must

scopes/git/ci/ci.main.runtime.ts

Lines changed: 75 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ import type { LaneId } from '@teambit/lane-id';
3333
import type { ConsumerComponent } from '@teambit/legacy.consumer-component';
3434
import { SourceBranchDetector } from './source-branch-detector';
3535
import { generateRandomStr } from '@teambit/toolbox.string.random';
36+
import { pMapPool } from '@teambit/toolbox.promise.map-pool';
37+
import { concurrentComponentsLimit } from '@teambit/harmony.modules.concurrency';
3638
import { extractSkipTasksFromMessage } from './skip-tasks-from-message';
3739

3840
// Two distinct conflicts can surface from the remote on a concurrent `bit ci pr` race.
@@ -420,36 +422,66 @@ export class CiMain {
420422
// lane-merge flows — see merge-status-provider / merge-lanes). Pass the *specific* main-head
421423
// version so `cache: true` still fetches it: the component already exists locally at its lane
422424
// version, so a version-less id would look satisfied and skip the remote.
423-
if (componentsToSync.length) {
424-
const mainHeadIds = componentsToSync.map(({ laneComp, mainHead }) =>
425-
laneComp.id.changeVersion(mainHead.toString())
426-
);
427-
try {
428-
await this.importer.importObjectsFromMainIfExist(mainHeadIds, { cache: true });
429-
} catch (e: any) {
430-
// Best-effort: a fetch hiccup shouldn't abort `bit ci pr`. The merge loop below still runs;
431-
// any component whose main head is still missing just logs the existing skip.
432-
this.logger.console(
433-
chalk.yellow(`Could not pre-fetch main's objects for config sync (continuing): ${e?.message || e}`)
434-
);
435-
}
436-
}
425+
const mainHeadIds = componentsToSync.map(({ laneComp, mainHead }) =>
426+
laneComp.id.changeVersion(mainHead.toString())
427+
);
428+
await this.prefetchFromMainForConfigSync(mainHeadIds, 'head objects');
429+
430+
// Resolve each component's diverge state up front — before the merge loop — so we can also
431+
// pre-fetch the common-ancestor objects below. getDivergeData only walks the parent graph,
432+
// which is already local (switchToLane brings the version-history, and the head pre-fetch above
433+
// reinforced it), so no full Version object is needed yet. Keep only components where main is
434+
// actually ahead or diverged; the rest have nothing to bring in from main. Bound the fan-out
435+
// (getDivergeData traverses each component's version graph) so a lane with many components
436+
// doesn't spawn one unbounded burst of concurrent graph walks.
437+
const componentsToMerge = compact(
438+
await pMapPool(
439+
componentsToSync,
440+
async (item) => {
441+
try {
442+
const divergeData = await getDivergeData({
443+
repo,
444+
modelComponent: item.modelComponent,
445+
sourceHead: item.laneComp.head,
446+
targetHead: item.mainHead,
447+
throws: false,
448+
});
449+
if (!divergeData.isTargetAhead() && !divergeData.isDiverged()) return undefined;
450+
return { ...item, divergeData };
451+
} catch (e: any) {
452+
// Best-effort per component (same contract as the merge loop below).
453+
this.logger.console(
454+
chalk.yellow(
455+
` ${item.laneComp.id.toStringWithoutVersion()}: skipping config sync from main (${e?.message || e})`
456+
)
457+
);
458+
return undefined;
459+
}
460+
},
461+
{ concurrency: concurrentComponentsLimit() }
462+
)
463+
);
464+
465+
// The head pre-fetch above brought main's head Version plus the version-history (parent graph),
466+
// but NOT the full Version object of the common ancestor (the lane's fork point) for components
467+
// where the lane and main have BOTH snapped since the fork. The 3-way config merge below loads
468+
// that base Version (`baseVersion.extensions`); without it, `loadVersion(baseSnap)` throws
469+
// VersionNotFoundOnFS, the per-component catch swallows it as "skipping config sync from main",
470+
// and the sync silently no-ops for every diverged component. The fork point lives on main and,
471+
// like the head, was never fetched (includeVersionHistory carries the graph, not each ancestor's
472+
// Version). Batch-fetch the bases in one call — same best-effort contract as the head pre-fetch.
473+
const baseIds = compact(
474+
componentsToMerge.map(({ laneComp, divergeData }) => {
475+
const baseSnap = divergeData.commonSnapBeforeDiverge;
476+
return baseSnap ? laneComp.id.changeVersion(baseSnap.toString()) : undefined;
477+
})
478+
);
479+
await this.prefetchFromMainForConfigSync(baseIds, 'common-ancestor objects');
437480

438481
const syncedIds: ComponentID[] = [];
439-
for (const { laneComp, modelComponent, mainHead } of componentsToSync) {
482+
for (const { laneComp, modelComponent, mainHead, divergeData } of componentsToMerge) {
440483
try {
441484
const laneHead = laneComp.head;
442-
const divergeData = await getDivergeData({
443-
repo,
444-
modelComponent,
445-
sourceHead: laneHead,
446-
targetHead: mainHead,
447-
throws: false,
448-
});
449-
// Only sync when main has snaps the lane doesn't (target ahead, or diverged). If the lane
450-
// is ahead-only / equal there's nothing on main to bring in.
451-
if (!divergeData.isTargetAhead() && !divergeData.isDiverged()) continue;
452-
453485
const currentVersion = await modelComponent.loadVersion(laneHead.toString(), repo);
454486
const otherVersion = await modelComponent.loadVersion(mainHead.toString(), repo);
455487
// base = common ancestor. When the lane is strictly behind main (no divergence) the common
@@ -515,6 +547,23 @@ export class CiMain {
515547
this.logger.console(chalk.green(`Synced config from main for ${syncedIds.length} component(s)`));
516548
}
517549

550+
/**
551+
* Batch-fetch main-side Version objects the config merge needs (heads, then common ancestors),
552+
* mirroring the lane-merge flows (merge-status-provider / merge-lanes). Best-effort: a fetch
553+
* hiccup shouldn't abort `bit ci pr` — the merge loop still runs and any component whose object is
554+
* still missing just logs the existing per-component skip. `label` names which objects for the log.
555+
*/
556+
private async prefetchFromMainForConfigSync(ids: ComponentID[], label: string) {
557+
if (!ids.length) return;
558+
try {
559+
await this.importer.importObjectsFromMainIfExist(ids, { cache: true });
560+
} catch (e: any) {
561+
this.logger.console(
562+
chalk.yellow(`Could not pre-fetch main's ${label} for config sync (continuing): ${e?.message || e}`)
563+
);
564+
}
565+
}
566+
518567
/**
519568
* Copied from `merging.main.runtime` (`filterDeletedDependenciesFromConfig`): the config merge
520569
* can emit deletion markers (`version: '-'`) for deps removed on main. The aspects-merger applies

0 commit comments

Comments
 (0)