Skip to content

Fix saved account loss across concurrent CLI sessions - #8973

Draft
isaacroldan wants to merge 1 commit into
mainfrom
codex/preserve-auth-sessions
Draft

isaacroldan wants to merge 1 commit into
mainfrom
codex/preserve-auth-sessions

Conversation

@isaacroldan

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Overlapping CLI processes can replace the saved account list with an older copy after a login or token refresh. A refresh can also change the selected account, and some authentication failures remove every saved account.

WHAT is this pull request doing?

Merge only changed accounts into the latest store under a short process lock. Use the same lock for alias updates and logout, and keep the selected account unchanged during a refresh.

Preserve saved accounts when a refresh returns invalid_request, authentication cannot prompt for a new sign-in, or the stored session format cannot be read. Unreadable data produces a recovery message instead of being deleted.

Existing CLI versions that do not contain this fix can still overwrite the shared store; both processes must use the fix for coordinated writes.

How to manually test your changes?

  1. In two terminals, run pnpm shopify auth login and use “Log in with a different account” to sign in with separate accounts at the same time.
  2. Run pnpm shopify auth login again. Confirm that both accounts remain available and select one.
  3. With pnpm shopify app dev --path /path/to/your/app running in one terminal, select the other saved account in the second terminal. After the dev process refreshes its credentials, confirm that both accounts remain available and the selected account has not changed.

Validation

  • 296 auth and storage tests pass, including real-file tests for overlapping writes, alias updates, unreadable content, and lock release after failure.
  • Restoring the faulty behavior causes 11 regression checks to fail.
  • A two-process reproduction with fake tokens confirms that refresh preserves an account saved and selected by the other process.
  • CLI Kit type check, changed-file ESLint, CLI Kit Knip, and git diff --check pass on Node 24.15.0.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

@isaacroldan
isaacroldan requested a review from a team as a code owner October 21, 2026 09:58
@github-actions github-actions Bot added the Area: @shopify/cli @shopify/cli package issues label Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/private/node/conf-store.d.ts
@@ -26,6 +26,8 @@ export interface ConfSchema {
     cache?: Cache;
     autoUpgradeEnabled?: boolean;
 }
+/** Serializes session mutations across CLI processes without holding a lock during authentication. */
+export declare function withSessionStoreLock<T>(action: () => Promise<T>, config?: LocalStorage<ConfSchema>): Promise<T>;
 /**
  * Get session.
  *
packages/cli-kit/dist/public/node/local-storage.d.ts
@@ -8,6 +8,12 @@ export declare class LocalStorage<T extends Record<string, any>> {
         projectName?: string;
         cwd?: string;
     });
+    /**
+     * Returns the path of the local storage file.
+     *
+     * @returns The absolute path of the local storage file.
+     */
+    get path(): string;
     /**
      * Get a value from the local storage.
      *
packages/cli-kit/dist/private/node/session/store.d.ts
@@ -1,19 +1,22 @@
+import type { ConfSchema } from '../conf-store.js';
+import type { LocalStorage } from '../../../public/node/local-storage.js';
 import type { Sessions } from './schema.js';
 /**
- * Serializes the session as a JSON and stores it in the system.
- * @param session - the session to store.
+ * Merges changed sessions into the latest stored accounts.
+ * @param sessions - Only the sessions changed by the caller.
+ * @param config - Optional local storage instance.
  */
-export declare function store(sessions: Sessions): Promise<void>;
+export declare function store(sessions: Sessions, config?: LocalStorage<ConfSchema>): Promise<void>;
 /**
  * Fetches the sessions from the local storage and returns it.
- * If the format of the object is invalid, the method will discard it.
+ * Unreadable data is preserved instead of removing every saved account.
  * @returns Returns a promise that resolves with the sessions object if it exists and is valid.
  */
-export declare function fetch(): Promise<Sessions | undefined>;
+export declare function fetch(config?: LocalStorage<ConfSchema>): Promise<Sessions | undefined>;
 /**
  * Removes a session from the system.
  */
-export declare function remove(): Promise<void>;
+export declare function remove(config?: LocalStorage<ConfSchema>): Promise<void>;
 /**
  * Gets the session alias for a given user ID.
  *
@@ -27,7 +30,7 @@ export declare function getSessionAlias(userId: string): Promise<string | undefi
  * @param userId - The user ID of the session to update.
  * @param alias - The new alias to set.
  */
-export declare function setSessionAlias(userId: string, alias: string): Promise<void>;
+export declare function setSessionAlias(userId: string, alias: string, config?: LocalStorage<ConfSchema>): Promise<void>;
 /**
  * Finds a session by its alias.
  *

@isaacroldan
isaacroldan marked this pull request as draft October 21, 2026 10:05

This branch has not been deployed

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

Labels

Area: @shopify/cli @shopify/cli package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant