Skip to content

Commit 1c125b9

Browse files
authored
fix(export): make export clientId collision-safe to prevent concurrent lost-update (#10458)
## Problem `export`'s `clientId` is both the pending-dir name **and** the cross-client export lock — `export-validate`'s `waitIfNeeded` queue sorts pending-dir names and lets only the first proceed to validate+persist. It was generated as `Date.now().toString()`, which isn't collision-safe. When two exports hit the same remote within the same millisecond (e.g. concurrent CI runners pushing the same lane), they get the **same clientId**, share one pending-dir, collapse the queue to a single entry, and both validate against the pre-persist state — silently losing one runner's update (no divergence detected, no rebase). This surfaced as an intermittently-failing e2e test (`ci-commands.e2e.ts` → "concurrent runners snapping the SAME component"): both runners exported cleanly and one snap was dropped from the lane. The outcome depended purely on OS scheduling of the two processes. ## Fix Append a random suffix to the generated clientId so same-millisecond exports can't collide. The timestamp prefix is kept so the sorted queue still roughly preserves arrival order. `--resume` is unaffected (it echoes back a user-supplied id; clientId is never parsed numerically).
1 parent 86433a4 commit 1c125b9

1 file changed

Lines changed: 11 additions & 1 deletion

File tree

scopes/scope/export/export.main.runtime.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import crypto from 'crypto';
12
import fs from 'fs-extra';
23
import type { CLIMain } from '@teambit/cli';
34
import { CLIAspect, MainRuntime } from '@teambit/cli';
@@ -678,7 +679,16 @@ if the scope name is wrong and you've already snapped/tagged, run "bit reset" to
678679

679680
async pushToRemotesCarefully(manyObjectsPerRemote: ObjectsPerRemote[], resumeExportId?: string) {
680681
const remotes = manyObjectsPerRemote.map((o) => o.remote);
681-
const clientId = resumeExportId || Date.now().toString();
682+
// The clientId is both the pending-dir name AND the cross-client export lock: `export-validate`'s
683+
// waitIfNeeded queue sorts pending-dir names and lets only the first proceed to validate+persist.
684+
// A pure `Date.now()` is not collision-safe — two exports to the same remote within the same
685+
// millisecond (e.g. concurrent CI runners pushing the same lane) get the same clientId, share one
686+
// pending-dir, collapse the queue to a single entry, and both validate against the pre-persist
687+
// state, silently losing one runner's update. A random suffix keeps the timestamp prefix (so the
688+
// sorted queue still roughly preserves arrival order) while making a same-millisecond collision
689+
// vanishingly unlikely (64 bits of randomness). Use node's built-in `crypto` rather than a
690+
// component helper so this core aspect doesn't gain a new component dependency.
691+
const clientId = resumeExportId || `${Date.now()}-${crypto.randomBytes(8).toString('hex')}`;
682692
await this.pushRemotesPendingDir(clientId, manyObjectsPerRemote, resumeExportId);
683693
await validateRemotes(remotes, clientId, Boolean(resumeExportId));
684694
// Intentionally no cleanup on `persistRemotes` failure: pending dirs are the substrate for

0 commit comments

Comments
 (0)