fix: dedupe concurrent file store creation attempts - #1727
Conversation
File store creation is detached from the request that triggers it, so every retry of the "try again later" error minted another store, each costing coins and orphaning the previous one. Track in-flight creations in a set shared between the v1 and v2 models (an upgraded org keeps the same file store on both sides) and start at most one creation per org. Also persist the minted store id before pushing it to the org store, so a sync failure cannot cause the next request to mint a replacement, and log post-persist sync failures distinctly from mint failures.
Re-check both models' persisted ids inside the creation flow: a caller holding a pre-creation org snapshot can race a creation that has since finished, and an upgraded org keeps one file store across the v1 and v2 models, so an id recorded by either side means the store exists.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The guard exists to stop a second store from being minted, and the new store id is written to the org record before the push runs. Holding the guard across the push kept it set for as long as that push retries (up to 60 attempts at 30s), during which the peer model could not adopt the id. On an upgraded org that meant the other API version kept returning the retry-later error even though the store already existed and was recorded. Release the guard once the id is persisted and run the push after it, so a request arriving mid-push adopts the id instead of waiting the window out. The detached work now runs in a single async function with one terminal error boundary rather than a promise chain.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 627cf31. Configure here.
| fileStoreId, | ||
| pushInProgress: false, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Failed push resurrects cleared store
High Severity
markFileStoreOrgStorePushFailed only bails when an entry exists with a different id. If unsubscribeFromFileStore already cleared the map, a late push failure recreates the entry, so the next creation re-adopts the abandoned store instead of minting a replacement.
Reviewed by Cursor Bugbot for commit 627cf31. Configure here.
| try { | ||
| await datalayer.syncDataLayer(orgUid, { fileStoreId: newFileStoreId }); | ||
| await datalayer.waitForAllTransactionsToConfirm(); | ||
| clearFileStoreIdPendingOrgStorePush(orgUid, newFileStoreId); |
There was a problem hiding this comment.
Creation ignores unsubscribe clear
High Severity
markFileStoreOrgStorePushStarted lets unsubscribeFromFileStore see and clear an in-flight id, but mintAndPersist / run never check that the pending entry was removed. Persist and syncDataLayer keep going, rewriting the org row and org store after unsubscribe appeared to succeed.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 627cf31. Configure here.
| ); | ||
| } catch (error) { | ||
| clearFileStoreIdPendingOrgStorePush(orgUid, newFileStoreId); | ||
| throw error; |
There was a problem hiding this comment.
Persist failure orphans minted store
Medium Severity
After a successful mint, a failed org-row update calls clearFileStoreIdPendingOrgStorePush, dropping the only remaining reference to the paid store. The next request cannot re-adopt it and mints another store instead.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 627cf31. Configure here.
1 participant
Summary
Behavior notes
get_fileendpoint previously blocked the HTTP request through coin-wait + store mint (and never persisted the id, so every such call minted another store). It now returns the same retry-later error the other filestore endpoints already return.Test plan
tests/v2/integration/file-store-creation-dedupe.spec.js(9 tests): concurrent-caller dedup for v1 and v2, persist-before-sync, failed-creation retry, pending-set guard, stale-snapshot adoption, and cross-model adoption in both directions.Merge order
Independent of the other two coin-management PRs; can merge anytime.
Note
Medium Risk
Changes on-chain store minting and org-store registration timing across v1/v2; incorrect dedupe or push retry logic could still orphan stores or leave orgs without a registered file store, though the goal is to reduce duplicate coin spend.
Overview
Stops retry-later file store traffic from minting multiple paid DataLayer stores per org by routing v1 and v2 through a shared
startFileStoreCreationflow backed by process-localpending-file-store-creations.One mint per org: a shared pending set blocks overlapping creation for the same org across v1 and v2. Callers still get the existing retry-later error immediately instead of waiting on coins or mint.
Persist before org-store sync: the new store id is written to the org row before
syncDataLayerregisters it on the org store, with a separate map tracking ids pending that push. Later requests adopt an id from either model’s DB row or that map (including when v2 reconciliation clears the column mid-push), and can retry a failed push without minting again.v1
getFileStoreItemnow matches other filestore endpoints: starts creation in the background and throws retry-later instead of blocking the HTTP request through mint.Unsubscribe on both models now awaits datalayer/DB work, considers in-flight pending-push ids, and clears pending-push state so unsubscribed stores are not re-adopted.
Reviewed by Cursor Bugbot for commit 627cf31. Bugbot is set up for automated code reviews on this repo. Configure here.