Skip to content

fix(export): make export clientId collision-safe to prevent concurrent lost-update #10458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidfirst wants to merge 1 commit into
base: master
Choose a base branch
from
+11 −1
Open

fix(export): make export clientId collision-safe to prevent concurrent lost-update #10458

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion scopes/scope/export/export.main.runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import crypto from 'crypto';
import fs from 'fs-extra';
import type { CLIMain } from '@teambit/cli';
import { CLIAspect, MainRuntime } from '@teambit/cli';
Expand Down Expand Up @@ -678,7 +679,16 @@ if the scope name is wrong and you've already snapped/tagged, run "bit reset" to

async pushToRemotesCarefully(manyObjectsPerRemote: ObjectsPerRemote[], resumeExportId?: string) {
const remotes = manyObjectsPerRemote.map((o) => o.remote);
const clientId = resumeExportId || Date.now().toString();
// The 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.
// A pure `Date.now()` is not collision-safe — two exports to the same remote within the same
// millisecond (e.g. concurrent CI runners pushing the same lane) 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. A random suffix keeps the timestamp prefix (so the
// sorted queue still roughly preserves arrival order) while making a same-millisecond collision
// vanishingly unlikely (64 bits of randomness). Use node's built-in `crypto` rather than a
// component helper so this core aspect doesn't gain a new component dependency.
const clientId = resumeExportId || `${Date.now()}-${crypto.randomBytes(8).toString('hex')}`;

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

1. Stale clientid messaging 🐞 Bug ⚙ Maintainability

pushToRemotesCarefully() now generates clientId as ${Date.now()}-<randomHex>, but other code
still documents/errors as if clientId were a pure millisecond timestamp. This mismatch can mislead
future callers/maintainers into adding numeric parsing/validation or misunderstanding collision
scenarios when debugging export failures.
Agent Prompt 
## Issue description
`clientId` is no longer a pure millisecond timestamp (it now includes a random suffix), but some comments/error text still describe it as "timestamp in ms" / "within the exact same millisecond".

## Issue Context
The PR intentionally changes the `clientId` format to prevent same-millisecond collisions; keeping surrounding documentation accurate helps prevent future code from making incorrect assumptions about `clientId` being numeric.

## Fix Focus Areas
- components/legacy/scope-api/lib/put.ts[11-14]
- components/legacy/scope/exceptions/client-id-in-use.ts[3-10]
- (optional) scopes/scope/export/export.main.runtime.ts[682-691] (ensure comment remains accurate/consistent)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

await this.pushRemotesPendingDir(clientId, manyObjectsPerRemote, resumeExportId);
await validateRemotes(remotes, clientId, Boolean(resumeExportId));
// Intentionally no cleanup on `persistRemotes` failure: pending dirs are the substrate for
Expand Down