Skip to content

fix(security): scope renderer local storage per user - #1880

Open
paustint wants to merge 1 commit into
mainfrom
fix/per-user-local-db
Open

fix(security): scope renderer local storage per user#1880
paustint wants to merge 1 commit into
mainfrom
fix/per-user-local-db

Conversation

@paustint

Copy link
Copy Markdown
Contributor

Per-user Dexie (getDexieDb/initDexieDb) + localforage (getLocalStore) stores so a shared machine no longer leaks the previous account's history on logout. Adopt-once migration hands existing shared data to the first user then locks it; nothing is deleted. Desktop reloads on logout; canvas scopes by production username so data follows a user across prod and sandboxes.

Copilot AI review requested due to automatic review settings July 26, 2026 22:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Scopes renderer-side persisted data to the authenticated user to prevent data leakage on shared machines, adding an adopt-once migration path for legacy unscoped stores and updating consumers to use the scoped accessors.

Changes:

  • Introduces per-user Dexie DB instances (initDexieDb/getDexieDb) and an adopt-once migration from the legacy shared DB/localforage data.
  • Adds per-user localforage instances via initLocalStore/getLocalStore, updating many reads/writes to use the scoped store.
  • Adds canvas-specific scoping logic to keep a user’s data consistent across production + sandboxes, plus targeted unit tests.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
libs/shared/ui-db/src/lib/ui-db.ts Reworks Dexie into a per-user, lazily-initialized instance and centralizes schema/hooks.
libs/shared/ui-db/src/lib/recent-history-items.db.ts Switches recent history Dexie access to getDexieDb().
libs/shared/ui-db/src/lib/query-history.db.ts Switches query history Dexie access to getDexieDb() and removes module-level hooks.
libs/shared/ui-db/src/lib/query-history-object.db.ts Switches query-history-object writes to getDexieDb() and removes module-level hook.
libs/shared/ui-db/src/lib/client-data.db.ts Implements per-user Dexie init + legacy adopt-once migration + sync lifecycle adjustments.
libs/shared/ui-db/src/lib/client-data-sync.db.ts Switches sync helper to use getDexieDb().
libs/shared/ui-db/src/lib/api-request-history.db.ts Switches API history Dexie access to getDexieDb().
libs/shared/ui-db/src/lib/analysis-job-history-retention.ts Switches retention pruning to getDexieDb().
libs/shared/ui-db/src/lib/tests/ui-db.spec.ts Adds tests for deterministic, non-raw-id scoped Dexie DB naming.
libs/shared/ui-core/src/record/record-utils.ts Routes recent-record storage through getLocalStore().
libs/shared/ui-core/src/query/QuickQueryPopover.tsx Switches query history live queries to getDexieDb().
libs/shared/ui-core/src/query/QueryHistory/QueryHistoryModal.tsx Switches live queries to getDexieDb().
libs/shared/ui-core/src/query/QueryHistory/QueryHistoryExportPopover.tsx Switches export read to getDexieDb().
libs/shared/ui-core/src/jobs/JobWorker.ts Switches analysis-job-history writes/transactions to getDexieDb().
libs/shared/ui-core/src/app/ThemeApplier.tsx Loads theme prefs via getLocalStore().
libs/shared/ui-core/src/analysis/PermissionAnalysisHistoryModal.tsx Switches analysis history live query to getDexieDb().
libs/shared/ui-app-state/src/lib/ui-app-state.ts Stores user preferences via getLocalStore().
libs/shared/data/src/lib/local-store.ts Adds per-user localforage instance management (initLocalStore/getLocalStore).
libs/shared/data/src/lib/client-data-cache.ts Routes cache/query-history cleanup and cache IO through getLocalStore().
libs/shared/data/src/lib/tests/local-store.spec.ts Adds unit tests validating per-user scoping and fallback behavior.
libs/shared/data/src/index.ts Exposes local-store utilities via shared data barrel exports.
libs/features/sobject-export/src/SObjectExport.tsx Switches saved selection storage to getLocalStore().
libs/features/salesforce-api/src/SalesforceApiHistoryModal.tsx Switches Dexie reads to getDexieDb().
libs/features/salesforce-api/src/salesforceApi.state.ts Switches local history persistence to getLocalStore().
libs/features/permission-analysis/src/PermissionAnalysisView.tsx Switches analysis-job-history lookup to getDexieDb().
libs/features/load-records/src/components/load-mapping-storage/SaveMappingPopover.tsx Switches saved mapping writes to getDexieDb().
libs/features/load-records/src/components/load-mapping-storage/LoadMappingPopover.tsx Switches saved mapping reads/deletes to getDexieDb().
libs/features/deploy/src/utils/deploy-metadata.utils.tsx Switches deploy history storage to getLocalStore().
libs/features/data-analysis/src/FieldUsageAnalysisView.tsx Switches analysis-job-history lookup to getDexieDb().
libs/features/anon-apex/src/apex.state.ts Switches apex history persistence to getLocalStore().
libs/features/anon-apex/src/AnonymousApex.tsx Switches apex history persistence to getLocalStore().
apps/jetstream/src/app/components/settings/Settings.tsx Clears scoped local store via getLocalStore().clear().
apps/jetstream/src/app/components/core/AppInitializer.tsx Initializes per-user localforage + per-user Dexie init on login.
apps/jetstream-web-extension/src/core/AppInitializer.tsx Initializes per-user localforage + per-user Dexie init for extension profile.
apps/jetstream-desktop-client/src/app/components/core/Login.tsx Forces renderer reload on logout to reset per-user state and sockets.
apps/jetstream-desktop-client/src/app/components/core/AppInitializer.tsx Initializes per-user localforage + per-user Dexie init, disconnects socket on teardown.
apps/jetstream-canvas/src/utils/canvas.utils.ts Adds sandbox/username-based scope derivation for canvas storage.
apps/jetstream-canvas/src/utils/tests/canvas.utils.spec.ts Adds tests for sandbox extraction + scope derivation rules.
apps/jetstream-canvas/src/app/core/AppInitializer.tsx Initializes per-user localforage/Dexie for canvas using derived scope id.
Comments suppressed due to low confidence (1)

libs/features/deploy/src/utils/deploy-metadata.utils.tsx:152

  • Same issue as above: getLocalStore().INDEXEDDB may be undefined on a scoped localforage instance, so this can incorrectly skip writing the deploy file blob.
      if (file && newItem.fileKey && getLocalStore().driver() === getLocalStore().INDEXEDDB) {
        await getLocalStore().setItem(newItem.fileKey, file);
      }

Comment thread libs/features/deploy/src/utils/deploy-metadata.utils.tsx Outdated
Comment thread apps/jetstream/src/app/components/core/AppInitializer.tsx Outdated
Comment thread apps/jetstream-canvas/src/app/core/AppInitializer.tsx
Comment thread libs/shared/ui-db/src/lib/ui-db.ts Outdated
Comment thread apps/jetstream-web-extension/src/core/AppInitializer.tsx
Copilot AI review requested due to automatic review settings July 28, 2026 03:16
@paustint
paustint force-pushed the fix/per-user-local-db branch from 937f5c6 to 5b14f00 Compare July 28, 2026 03:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comment thread apps/jetstream/src/app/components/settings/Settings.tsx Outdated
Copilot AI review requested due to automatic review settings July 28, 2026 15:16
@paustint
paustint force-pushed the fix/per-user-local-db branch from 5b14f00 to ce03f2a Compare July 28, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

libs/shared/ui-db/src/lib/ui-db.ts:203

  • When switching users, setDexieDbInstance() replaces the active Dexie instance but never closes the previous one. Over time (or in account-switch flows without a hard reload) this can leave multiple IndexedDB connections and observable subscriptions hanging around.

Comment thread libs/shared/ui-core/src/record/record-utils.ts Outdated
Copilot AI review requested due to automatic review settings July 28, 2026 23:07
@paustint
paustint force-pushed the fix/per-user-local-db branch from ce03f2a to 79e393b Compare July 28, 2026 23:07
@paustint
paustint force-pushed the fix/per-user-local-db branch from 79e393b to 956ad1a Compare July 28, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (5)

apps/jetstream/src/app/components/core/AppInitializer.tsx:70

  • When activeUserId becomes null (logout), the per-user localforage instance created earlier is left bound in module state, so getLocalStore() will continue to return the previous user’s store until the next login re-initializes it. Reset the local store when there is no active user to avoid leaking prior-user preferences/history into logged-out surfaces.
  const activeUserId = userProfile.id && userProfile.id !== DEFAULT_PROFILE.id ? userProfile.id : null;
  if (activeUserId) {
    initLocalStore({ dbName: environment.name, userId: activeUserId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.

apps/jetstream-desktop-client/src/app/components/core/AppInitializer.tsx:61

  • If activeUserId flips to null (logout) before the renderer reloads, getLocalStore() will still point at the previous user’s scoped store. Reset it when there is no active user so any intermediate logged-out rendering can’t read prior-user data.
  const activeUserId = userProfile.id && userProfile.id !== DEFAULT_PROFILE.id ? userProfile.id : null;
  if (activeUserId) {
    initLocalStore({ dbName: environment.name, userId: activeUserId });
  }

apps/jetstream-web-extension/src/core/AppInitializer.tsx:60

  • When chromeUserProfile?.id becomes falsy (logout/account switch), the previously-initialized per-user localforage instance stays active, so getLocalStore() can still return the previous user’s store. Add an else branch to reset the store when there is no user id.
  if (chromeUserProfile?.id) {
    initLocalStore({ dbName: 'jetstream-web-ext-no-sync', userId: chromeUserProfile.id });
  }

apps/jetstream-canvas/src/app/core/AppInitializer.tsx:56

  • If storageScopeId is null, the code currently leaves any previously-created scoped localforage instance bound in module state, so getLocalStore() can still point at an old user’s store. Reset the local store when there is no active scope id.
  const storageScopeId = selectedOrg?.uniqueId ? getCanvasStorageScopeId() : null;
  if (storageScopeId) {
    initLocalStore({ dbName: 'jetstream-canvas', userId: storageScopeId });
  }

libs/shared/ui-db/src/lib/client-data.db.ts:207

  • clearLocalStorageRetainingLegacyClaim() calls localStorage.clear() without a try/catch. In environments where localStorage throws (privacy mode, blocked storage, extension contexts), this can throw and break the delete-account flow that calls it. Wrap the clear in a try/catch (similar to the other helpers here).

Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
Comment thread apps/jetstream/src/app/components/core/AppInitializer.tsx Outdated
Comment thread apps/jetstream-desktop-client/src/app/components/core/AppInitializer.tsx Outdated
Comment thread apps/jetstream-web-extension/src/core/AppInitializer.tsx Outdated
Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
Comment thread apps/jetstream-canvas/src/app/core/AppInitializer.tsx Outdated
Copilot AI review requested due to automatic review settings July 28, 2026 23:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (5)

apps/jetstream/src/app/components/core/AppInitializer.tsx:72

  • initLocalStore() is only called when activeUserId is truthy, but there is no reset when the user logs out (activeUserId becomes null). Because the per-user store is cached in a module-level variable, getLocalStore() will keep returning the previous user's instance during the logged-out state, which can still expose prior user preferences/history on a shared machine.
  const activeUserId = userProfile.id && userProfile.id !== DEFAULT_PROFILE.id ? userProfile.id : null;
  if (activeUserId) {
    initLocalStore({ dbName: environment.name, userId: activeUserId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.
  use(ensureDexieDbReady(activeUserId));

apps/jetstream-web-extension/src/core/AppInitializer.tsx:63

  • initLocalStore() is only called when chromeUserProfile.id exists, but there is no reset when the user becomes unauthenticated. Because the store is cached at module scope, getLocalStore() can remain bound to the prior user's store while the extension is in a logged-out state, which defeats the per-user isolation goal on shared machines.
  if (chromeUserProfile?.id) {
    initLocalStore({ dbName: 'jetstream-web-ext-no-sync', userId: chromeUserProfile.id });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.
  use(ensureDexieDbReady(chromeUserProfile?.id ?? null));

apps/jetstream-canvas/src/app/core/AppInitializer.tsx:59

  • initLocalStore() is only called when storageScopeId is truthy, but the module-level store is never reset when storageScopeId becomes null (e.g. canvas context error or selectedOrg cleared). That can leave getLocalStore() pointing at a previous user's scoped store in a state where the app believes it is unauthenticated/uninitialized.
  const storageScopeId = selectedOrg?.uniqueId ? getCanvasStorageScopeId() : null;
  if (storageScopeId) {
    initLocalStore({ dbName: 'jetstream-canvas', userId: storageScopeId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.
  use(ensureDexieDbReady(storageScopeId));

libs/shared/ui-db/src/lib/client-data.db.ts:66

  • When switching users, ensureInstanceReady() creates a new Dexie instance but never closes/clears any previously active instance. That can leave the prior user's DB (and any sync connection) alive in-memory during the same renderer process, undermining per-user isolation and potentially leaking data on logout/account-switch without a reload. Also, if DB creation fails, the current implementation logs and resolves; ensureDexieDbReady() is used as a render gate specifically to make getDexieDb() safe, so failures should reject the readiness promise instead of allowing descendants to crash later.
    libs/shared/ui-core/src/query/QuickQueryPopover.tsx:75
  • This useLiveQuery uses an empty dependency array while calling getDexieDb() inside the query function. If the user switches accounts without a full reload (which this PR explicitly supports), the liveQuery subscription can remain attached to the previous user's Dexie instance and continue showing the old user's history.
  // Note: this is not scoped to the current org, which I guess is fine for now (could give user an option)
  const queryHistory = useLiveQuery(
    // Since we want to sort by lastRun, we cannot use a normal where clause
    () => getDexieDb().query_history.orderBy('lastRun').reverse().limit(NUM_HISTORY_ITEMS).toArray(),
    [],
    [] as QueryHistoryItem[],
  );

Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
Copilot AI review requested due to automatic review settings July 29, 2026 15:28
@paustint
paustint force-pushed the fix/per-user-local-db branch from 956ad1a to 6c3c3d6 Compare July 29, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (10)

libs/shared/ui-db/src/lib/client-data.db.ts:206

  • clearLocalStorageRetainingLegacyClaim() calls localStorage.clear() without a try/catch. localStorage access can throw (e.g. privacy mode / disabled storage), which would break the delete-account flow that calls this helper.
    apps/jetstream/src/app/components/core/AppInitializer.tsx:17
  • AppInitializer initializes user-scoped localforage/Dexie, but it never imports the teardown helpers needed to reset them on logout. Without resetting, anonymous surfaces (or interim UI during logout) can keep reading/writing the previous user's scoped stores until the next init.
import {
  checkHeartbeat,
  disconnectSocket,
  initLocalStore,
  initSocket,
  registerMiddleware,
  updateUserProfile,
} from '@jetstream/shared/data';
import { initErrorTracker, setErrorTrackerUser, tracker, useObservable } from '@jetstream/shared/ui-utils';
import { Announcement, JetstreamEventSaveSoqlQueryFormatOptionsPayload, SalesforceOrgUi } from '@jetstream/types';
import { fireToast } from '@jetstream/ui';
import { fromJetstreamEvents, useAmplitude } from '@jetstream/ui-core';
import { DEFAULT_PROFILE, fromAppState } from '@jetstream/ui/app-state';
import { CookieConsentBanner, useConditionalGoogleAnalytics } from '@jetstream/ui/cookie-consent-banner';
import { ensureDexieDbReady, initDexieDb, pruneAnalysisJobHistory } from '@jetstream/ui/db';

apps/jetstream/src/app/components/core/AppInitializer.tsx:70

  • When activeUserId becomes null (logout), the per-user localforage instance and Dexie instance remain bound to the previous user. Resetting them here prevents any logged-out UI or racey in-flight effects from touching the prior account’s local data.
  const activeUserId = userProfile.id && userProfile.id !== DEFAULT_PROFILE.id ? userProfile.id : null;
  if (activeUserId) {
    initLocalStore({ dbName: environment.name, userId: activeUserId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.

apps/jetstream-desktop-client/src/app/components/core/AppInitializer.tsx:10

  • This initializer sets up user-scoped localforage/Dexie but does not import the teardown helpers to reset them when the user profile returns to DEFAULT_PROFILE. Without a reset, the previous user’s scoped stores can stay bound until the renderer reloads.
import { disconnectSocket, initLocalStore, initSocket, registerMiddleware } from '@jetstream/shared/data';
import { initErrorTracker, setErrorTrackerUser, tracker, useObservable } from '@jetstream/shared/ui-utils';
import { Announcement, JetstreamEventSaveSoqlQueryFormatOptionsPayload, SalesforceOrgUi } from '@jetstream/types';
import { fireToast } from '@jetstream/ui';
import { fromJetstreamEvents, useAmplitude } from '@jetstream/ui-core';
import { DEFAULT_PROFILE, fromAppState } from '@jetstream/ui/app-state';
import { ensureDexieDbReady, initDexieDb, pruneAnalysisJobHistory } from '@jetstream/ui/db';

apps/jetstream-desktop-client/src/app/components/core/AppInitializer.tsx:62

  • On logout (activeUserId -> null) the per-user localforage/Dexie bindings are left pointing at the previous user. Even if the desktop flow usually reloads, resetting here avoids cross-user leakage if logout occurs without an immediate reload or during intermediate UI states.
  const activeUserId = userProfile.id && userProfile.id !== DEFAULT_PROFILE.id ? userProfile.id : null;
  if (activeUserId) {
    initLocalStore({ dbName: environment.name, userId: activeUserId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.

apps/jetstream-web-extension/src/core/AppInitializer.tsx:12

  • The extension initializer scopes localforage/Dexie per user, but it doesn’t import teardown helpers to clear those bindings when chromeUserProfile becomes undefined (logout / token purge). Without a reset, the previous user’s local data can remain accessible in logged-out UI.
import { disconnectSocket, initLocalStore, initSocket } from '@jetstream/shared/data';
import { applyVerifiedFeatureFlags, getBrowserExtensionVersion, useNonInitialEffect } from '@jetstream/shared/ui-utils';
import { getDefaultAppState, getErrorMessage } from '@jetstream/shared/utils';
import { JetstreamEventSaveSoqlQueryFormatOptionsPayload, UserProfileUi } from '@jetstream/types';
import { ScopedNotification } from '@jetstream/ui';
import { AppLoading, fromJetstreamEvents } from '@jetstream/ui-core';
import { fromAppState } from '@jetstream/ui/app-state';
import { ensureDexieDbReady, initDexieDb } from '@jetstream/ui/db';
import { useObservable } from 'dexie-react-hooks';

apps/jetstream-web-extension/src/core/AppInitializer.tsx:62

  • If chromeUserProfile.id becomes falsy (logged out), the previously-initialized per-user localforage/Dexie instances remain active. Resetting them prevents any logged-out UI from reading/writing the prior user’s scoped stores.
  if (chromeUserProfile?.id) {
    initLocalStore({ dbName: 'jetstream-web-ext-no-sync', userId: chromeUserProfile.id });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.
  use(ensureDexieDbReady(chromeUserProfile?.id ?? null));

apps/jetstream-canvas/src/app/core/AppInitializer.tsx:9

  • Canvas initializes user-scoped localforage/Dexie but doesn’t import teardown helpers to reset those bindings if storageScopeId becomes null (e.g. canvas context reload / init error). Resetting avoids stale cross-user state being reused unexpectedly.
import { getCanvasPreferences, initLocalStore, updateCanvasPreferences } from '@jetstream/shared/data';
import { useObservable } from '@jetstream/shared/ui-utils';
import { JetstreamEventSaveSoqlQueryFormatOptionsPayload } from '@jetstream/types';
import { AppLoading, fromJetstreamEvents } from '@jetstream/ui-core';
import { fromAppState } from '@jetstream/ui/app-state';
import { ensureDexieDbReady, initDexieDb } from '@jetstream/ui/db';
import { useAtomValue, useSetAtom } from 'jotai';

apps/jetstream-canvas/src/app/core/AppInitializer.tsx:58

  • When storageScopeId is null, the previously-initialized per-user localforage/Dexie instances (if any) remain bound. Resetting here prevents stale scoped state from a prior canvas session/user from being used unintentionally.
  const storageScopeId = selectedOrg?.uniqueId ? getCanvasStorageScopeId() : null;
  if (storageScopeId) {
    initLocalStore({ dbName: 'jetstream-canvas', userId: storageScopeId });
  }
  // Suspend until the per-user database exists so no descendant calls getDexieDb() before it is ready.
  use(ensureDexieDbReady(storageScopeId));

libs/shared/ui-db/src/lib/client-data.db.ts:75

  • ensureDexieDbReady() is documented as making getDexieDb() safe, but ensureInstanceReady() swallows errors and resolves anyway. In that failure case React use(ensureDexieDbReady(...)) will unblock rendering, and any descendant call to getDexieDb() will throw.

Comment thread libs/shared/ui-db/src/lib/ui-db.ts
Copilot AI review requested due to automatic review settings July 29, 2026 15:39
@paustint
paustint force-pushed the fix/per-user-local-db branch from 6c3c3d6 to cb44f90 Compare July 29, 2026 15:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

libs/shared/ui-db/src/lib/client-data.db.ts:95

  • clearInstance() drops the global Dexie reference without closing the current DB first. On user switch (or logout without an immediate full reload), this can leave the prior user's IndexedDB connection (and potentially sync background work) alive longer than intended, increasing resource usage and making it easier for stale references to keep operating on the old DB. Close the active instance before nulling it out.
    libs/shared/ui-db/src/lib/ui-db.ts:14
  • ui-db.ts now imports queryHistoryObjectDb, but query-history-object.db.ts imports getDexieDb/wrapApiWithReopenOnDatabaseClosed from ui-db.ts, creating a circular module dependency (ui-dbquery-history-object). Cycles like this are easy to break accidentally (e.g. by converting a function export into a const), and can lead to undefined exports during initialization depending on bundling order. Consider breaking the cycle by moving the _query_history_object write logic into ui-db.ts (direct db._query_history_object.put(...) inside the hook) or extracting the shared getQueryHistoryObject helper into a third module with no ui-db imports.

Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
@paustint
paustint force-pushed the fix/per-user-local-db branch from cb44f90 to 258a866 Compare July 29, 2026 15:59
Copilot AI review requested due to automatic review settings July 30, 2026 02:33
@paustint
paustint force-pushed the fix/per-user-local-db branch from 258a866 to 3340528 Compare July 30, 2026 02:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 03:23
@paustint
paustint force-pushed the fix/per-user-local-db branch from 3340528 to 1e178ed Compare July 30, 2026 03:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

libs/shared/ui-db/src/lib/client-data.db.ts:86

  • ensureInstanceReady() catches errors creating the per-user Dexie instance but still resolves instanceReadyPromise. Call sites use(ensureDexieDbReady(...)) then proceed to render children under the assumption that getDexieDb() is safe, but dexieDbInstance may still be unset and will throw at first access.

To keep the gating contract correct, either reject this promise on failure (so the Suspense boundary can surface an error state) or change ensureDexieDbReady to only resolve once hasDexieDb() is true.
libs/shared/ui-db/src/lib/ui-db.ts:212

  • ui-db.ts now imports queryHistoryObjectDb from query-history-object.db.ts, but that module also imports getDexieDb/wrapApiWithReopenOnDatabaseClosed from ui-db.ts, creating a circular dependency. This is fragile (module init order / bundler output) and also makes this hook depend on a separate API wrapper when it can write the derived row directly.

Consider removing the cross-module call here and instead put the derived QueryHistoryObject into db._query_history_object directly (or move the hook registration into query-history-object.db.ts so ui-db.ts doesn’t need to import it).

Comment thread libs/shared/ui-db/src/lib/client-data.db.ts Outdated
Copilot AI review requested due to automatic review settings July 31, 2026 15:17
@paustint
paustint force-pushed the fix/per-user-local-db branch from 1e178ed to d0a25d7 Compare July 31, 2026 15:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

Suppressed comments (1)

libs/shared/data/src/lib/local-store.ts:26

  • toStoreName() embeds the raw userId into the localforage storeName (object store name). In canvas, userId is derived from a Salesforce username (often an email), so this leaves user identifiers/PII discoverable in IndexedDB metadata (and potentially in key prefixes if a non-IndexedDB driver is used). Dexie already hashes the user id for the per-user DB name; consider applying a similar hashing strategy for the localforage storeName so user ids aren’t directly exposed.
function toStoreName(userId: string): string {
  return `u_${encodeURIComponent(userId)}`;
}

Per-user Dexie (getDexieDb/initDexieDb) + localforage (getLocalStore) stores so a
shared machine no longer leaks the previous account's history on logout. Adopt-once
migration hands existing shared data to the first user then locks it; nothing is
deleted. Desktop reloads on logout; canvas scopes by production username so data
follows a user across prod and sandboxes.
Copilot AI review requested due to automatic review settings August 1, 2026 15:11
@paustint
paustint force-pushed the fix/per-user-local-db branch from d0a25d7 to f6f790f Compare August 1, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (3)

libs/shared/data/src/lib/tests/local-store.spec.ts:54

  • This test currently uses the real localforage.createInstance() (only spied), which can try to initialize unsupported storage in the Node test environment. Mock the instance so initLocalStore() is deterministic and doesn’t depend on IndexedDB/localStorage availability.
    const createInstance = vi.spyOn(localforage, 'createInstance');

libs/shared/data/src/lib/tests/local-store.spec.ts:62

  • This test counts calls to localforage.createInstance() but currently lets the real implementation run (only spied). In the Node Vitest environment that can fail or hang depending on driver detection. Mock two distinct instances so the test only exercises initLocalStore’s scoping logic.
    const createInstance = vi.spyOn(localforage, 'createInstance');

libs/shared/data/src/lib/tests/local-store.spec.ts:47

  • These tests spy on localforage.createInstance but (in Node-based Vitest) still call the real implementation, which can attempt to initialize browser storage and make the test flaky/fail. Mock the return value so initLocalStore() never touches real localforage drivers here.

This issue also appears in the following locations of the same file:

  • line 54
  • line 62
    const createInstance = vi.spyOn(localforage, 'createInstance');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants