fix: skip import when singleton store is unsynced to prevent 10-min block#1603
Merged
Conversation
…lock importOrganization (V1 and V2) already pre-checks the orgUid store's sync status before the blocking subscribe flow, but once the orgUid store is synced, subscribeToOrganization still reads the singleton (data-model-version) store — and that path uses a 10-minute wait loop on first import when the singleton has not yet propagated to the local datalayer. With N newly-discovered orgs in the default-org list, the sync-default-organizations* task could block for N x 10 min. After confirming the orgUid store is synced, read the org store once (fast, since it's synced) to discover the singleton id, subscribe to the singleton so it begins syncing, and pre-check its sync status. If the singleton is not yet synced, skip this import and let the next task run retry. Matches the skip-on-unsynced pattern used elsewhere.
…sy return Mirrors the pattern established in #1600 af06a65 (for governance sync) and 6d41b71 (for reconcileOrganization). subscribeToStoreOnDataLayer in src/datalayer/persistance.js returns false (without throwing) on the common failure paths: - No storeId provided - getSubscriptions() RPC failure (datalayer unreachable) Without the falsy-return check, the dominant datalayer-unreachable failure would slip past the try/catch, and the subsequent getDataLayerStoreSyncStatus call (which also fails when the store is not subscribed) would produce a misleading "not yet synced" log. Applies to both V1 and V2 importOrganization at two subscribe call sites each: the pre-existing orgUid subscribe at the top of the function, and the new singleton-store subscribe added in this PR. Both are adjacent in the same function; fixing only one would leave an internal inconsistency that a reviewer would flag.
TheLastCicada
added a commit
that referenced
this pull request
Apr 23, 2026
…ned helper Bugbot flagged a High Severity ReferenceError in my previous commit 9cf6cb7: stale refactor code sitting in my worktree from an earlier exploration of a `subscribeOrSkip` helper got swept into `git add -A` and landed in the Organization.importOrganization method. The helper was never defined, never imported, and never exported — so the commit compiled but would crash at runtime with `ReferenceError: subscribeOrSkip is not defined` on the first non-simulator-mode call to importOrganization. importOrganization is out of scope for this PR; its improvements live in #1603. Revert that function's "Subscribe to the org store first, then check sync status" block to match origin/v2-rc2 verbatim. All intentional #1600 changes (reconcileOrganization + skipOnUnsynced, governance sync, task intervals, tests) are untouched. Verified by `git diff origin/v2-rc2 -- src/models/organizations/organizations.model.js` now showing only reconcileOrganization changes.
TheLastCicada
added a commit
that referenced
this pull request
Apr 23, 2026
Adopt the orthogonal pieces of #1590 (which supersedes that PR's governance sync rewrite with #1600's simpler single-attempt-per-run design while keeping the #1600 5-minute cadence). - tasks/sync-default-organizations{,-v2}.js: remove the nested Governance.sync() trigger. Governance sync is now owned exclusively by sync-governance-body{,-v2}.js on its scheduler cadence; this task just reads whatever the local DB has and no-ops if orgList hasn't landed yet. Fixes a race where the task could block on governance sync before doing its own work. - utils/data-loaders{,-v2}.js: drop the 50-retry recursive wait loop in getDefaultOrganizationList{,V2}. On a fresh install where governance data hasn't been synced, return [] instead of retrying internally; the scheduler will re-run us on cadence. Also move the USE_SIMULATOR / USE_DEVELOPMENT_MODE destructure into the functions so config reloads aren't stale. - controllers/governance{,-v2}.controller.js: harden POST /governance/sync and POST /v2/governance/sync as fire-and-forget. The model sync() methods already return cleanly on any failure (subscribe- first pattern, scheduler retries on cadence); attach .catch defensively so future exceptions are logged instead of becoming unhandled rejections. Does NOT adopt #1590's two-tier bootstrap/steady cadence (keeping #1600's flat 300s default), the src/tasks/index.js scheduler refactor (not needed without the promotion machinery), or the importOrganization { allowLongWait } option (that belongs to #1603). Closes #1590 by folding in its compatible pieces.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third and final follow-up to #1600 / #1602. Closes out the last known multi-minute blocking site on the scheduler.
importOrganization(V1 and V2) already pre-checks the orgUid store's sync status before the blocking subscribe flow. But once the orgUid store is synced,subscribeToOrganizationreads the singleton (data-model-version) store — and that path uses a 10-minute wait loop on first import when the singleton hasn't yet propagated to the local datalayer.getRegistryStoreIdFromSingleton(kept intentionally for request-path callers likePOST /v2/organizations/subscribe).subscribeToOrganizationitself (two loops — one for orgUid, one for singleton).With N newly-discovered orgs in the default-org list,
sync-default-organizations*could serialize N × 10 min of blocking.Fix: after confirming the orgUid store is synced, read the org store once (fast, since it's synced) to discover the singleton id, subscribe to the singleton so it begins syncing, and pre-check its sync status. If the singleton is not yet synced, skip this import and let the next task run retry. Same skip-on-unsynced pattern used throughout the sync-blocking-regression work.
What this PR does NOT do
POST /v2/organizations/subscribestill waits for the singleton to sync, because a user who explicitly subscribed expects the call to complete when data is available.subscribeToOrganizationitself — only adds a pre-check in its callerimportOrganization.Test plan
npm run test:v2— newimport-organization-singleton-precheck.spec.js(6 tests) passes; relatedsync-default-organizations-v2.spec.jsandsync-organization-meta-v2.spec.jsalso pass cleanly together.sync-default-organizations*task runs complete within seconds on initial startup, even when many default orgs have not yet synced their singleton stores.Dependencies
Independent of #1600 and #1602. All three fix complementary facets of the same sync-blocking regression and can merge in any order.
Note
Medium Risk
Changes organization import control flow in both V1 and V2 to early-exit based on additional datalayer subscription/sync checks, which could delay imports if the singleton store is slow or the new checks mis-detect state.
Overview
Prevents long blocking waits during first-time organization imports by adding a singleton (data-model-version) store pre-check in
importOrganizationfor both V1 and V2: after confirming the org store is synced, it reads the org store to discover the singleton store id, subscribes to that singleton, and skips the import until the singleton is fully synced.Also hardens the initial org-store subscription step by treating falsy returns from
subscribeToStoreOnDataLayeras a retryable failure (with clearer logs), not just thrown errors. Adds a small contract-style integration test ensuring method signatures remain unchanged and thesync-default-organizations*tasks remain importable.Reviewed by Cursor Bugbot for commit 2c1326b. Bugbot is set up for automated code reviews on this repo. Configure here.