Refactor identity client code execution #6654
Open
+285
−268
Conversation
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
Contributor
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
This was referenced Nov 21, 2025
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
alexanderMontague
force-pushed
the
11-21-refactor_identity_client_code_execution
branch
from
81f2261 to
95f8fe3
Compare
Contributor
Differences in type declarationsWe 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:
New type declarationspackages/cli-kit/dist/private/node/session/token-utils.d.tsimport { ApplicationToken, IdentityToken } from './schema.js';
import { TokenRequestResult } from '../clients/identity/identity-client.js';
import { AbortError, ExtendableError } from '../../../public/node/error.js';
export declare class InvalidGrantError extends ExtendableError {
}
export declare class InvalidRequestError extends ExtendableError {
}
/**
* Handles errors returned from token requests to the identity service.
* Maps specific error codes to appropriate error types for proper error handling.
*
* @param error - The error code returned from the identity service
* @param store - Optional store name for contextual error messages
* @returns An appropriate error instance based on the error code
*/
export declare function tokenRequestErrorHandler({ error, store }: {
error: string;
store?: string;
}): AbortError | InvalidGrantError | InvalidRequestError;
/**
* Builds an IdentityToken from a token request result.
* Extracts the user ID from the id_token JWT if not provided.
*
* @param result - The token request result from the identity service
* @param existingUserId - Optional existing user ID to preserve
* @param existingAlias - Optional existing alias to preserve
* @returns A complete IdentityToken with all required fields
*/
export declare function buildIdentityToken(result: TokenRequestResult, existingUserId?: string, existingAlias?: string): IdentityToken;
/**
* Builds an ApplicationToken from a token request result.
*
* @param result - The token request result from the identity service
* @returns An ApplicationToken with access token, expiration, and scopes
*/
export declare function buildApplicationToken(result: TokenRequestResult): ApplicationToken;
packages/cli-kit/dist/private/node/clients/identity/identity-client.d.tsimport { API } from '../../api.js';
import { Result } from '../../../../public/node/result.js';
export interface TokenRequestResult {
access_token: string;
expires_in: number;
refresh_token: string;
scope: string;
id_token?: string;
}
export interface DeviceAuthorizationResponse {
deviceCode: string;
userCode: string;
verificationUri: string;
expiresIn: number;
verificationUriComplete?: string;
interval?: number;
}
export declare abstract class IdentityClient {
abstract tokenRequest(params: {
[key: string]: string;
}): Promise<Result<TokenRequestResult, {
error: string;
store?: string;
}>>;
abstract requestDeviceAuthorization(scopes: string[]): Promise<DeviceAuthorizationResponse>;
abstract clientId(): string;
applicationId(api: API): string;
}
packages/cli-kit/dist/private/node/clients/identity/identity-mock-client.d.tsimport { IdentityClient, type TokenRequestResult, type DeviceAuthorizationResponse } from './identity-client.js';
import { Result } from '../../../../public/node/result.js';
export declare class IdentityMockClient extends IdentityClient {
private readonly mockUserId;
private readonly authTokenPrefix;
requestDeviceAuthorization(_scopes: string[]): Promise<DeviceAuthorizationResponse>;
tokenRequest(params: {
[key: string]: string;
}): Promise<Result<TokenRequestResult, {
error: string;
store?: string;
}>>;
clientId(): string;
private readonly generateTokens;
private getFutureDate;
private getCurrentUnixTimestamp;
private encodeTokenPayload;
private generateMockJWT;
}
packages/cli-kit/dist/private/node/clients/identity/identity-service-client.d.tsimport { IdentityClient, type TokenRequestResult, type DeviceAuthorizationResponse } from './identity-client.js';
import { Result } from '../../../../public/node/result.js';
export declare class IdentityServiceClient extends IdentityClient {
tokenRequest(params: {
[key: string]: string;
}): Promise<Result<TokenRequestResult, {
error: string;
store?: string;
}>>;
clientId(): string;
/**
* Initiate a device authorization flow.
* This will return a DeviceAuthorizationResponse containing the URL where user
* should go to authorize the device without the need of a callback to the CLI.
*
* Also returns a `deviceCode` used for polling the token endpoint in the next step.
*
* @param scopes - The scopes to request
* @returns An object with the device authorization response.
*/
requestDeviceAuthorization(scopes: string[]): Promise<DeviceAuthorizationResponse>;
}
packages/cli-kit/dist/private/node/clients/identity/instance.d.tsimport { IdentityClient } from './identity-client.js';
export declare function getIdentityClient(): IdentityClient;
Existing type declarationspackages/cli-kit/dist/public/node/environment.d.ts@@ -53,7 +53,7 @@ export declare function jsonOutputEnabled(environment?: NodeJS.ProcessEnv): bool
/**
* If true, the CLI should not use the Partners API.
*
- * @returns True when SHOPIFY_CLI_NEVER_USE_PARTNERS_API is set or SHOPIFY_CLI_1P_DEV is not set.
+ * @returns True if the SHOPIFY_CLI_NEVER_USE_PARTNERS_API environment variable is set.
*/
export declare function blockPartnersAccess(): boolean;
/**
packages/cli-kit/dist/public/node/session.d.ts@@ -122,15 +122,4 @@ export declare function ensureAuthenticatedBusinessPlatform(scopes?: BusinessPla
* @returns A promise that resolves when the logout is complete.
*/
export declare function logout(): Promise<void>;
-/**
- * Ensure that we have a valid Admin session for the given store, with access on behalf of the app.
- *
- * See for access on behalf of a user.
- *
- * @param storeFqdn - Store fqdn to request auth for.
- * @param clientId - Client ID of the app.
- * @param clientSecret - Client secret of the app.
- * @returns The access token for the Admin API.
- */
-export declare function ensureAuthenticatedAdminAsApp(storeFqdn: string, clientId: string, clientSecret: string): Promise<AdminSession>;
export {};
\ No newline at end of file
packages/cli-kit/dist/private/node/session/device-authorization.d.ts@@ -1,32 +1,14 @@
-import { IdentityToken } from './schema.js';
-export interface DeviceAuthorizationResponse {
- deviceCode: string;
- userCode: string;
- verificationUri: string;
- expiresIn: number;
- verificationUriComplete?: string;
- interval?: number;
-}
+import { Response } from 'node-fetch';
+export declare function convertRequestToParams(queryParams: {
+ client_id: string;
+ scope: string;
+}): string;
/**
- * Initiate a device authorization flow.
- * This will return a DeviceAuthorizationResponse containing the URL where user
- * should go to authorize the device without the need of a callback to the CLI.
+ * Build a detailed error message for JSON parsing failures from the authorization service.
+ * Provides context-specific error messages based on response status and content.
*
- * Also returns a used for polling the token endpoint in the next step.
- *
- * @param scopes - The scopes to request
- * @returns An object with the device authorization response.
- */
-export declare function requestDeviceAuthorization(scopes: string[]): Promise<DeviceAuthorizationResponse>;
-/**
- * Poll the Oauth token endpoint with the device code obtained from a DeviceAuthorizationResponse.
- * The endpoint will return until the user completes the auth flow in the browser.
- * Once the user completes the auth flow, the endpoint will return the identity token.
- *
- * Timeout for the polling is defined by the server and is around 600 seconds.
- *
- * @param code - The device code obtained after starting a device identity flow
- * @param interval - The interval to poll the token endpoint
- * @returns The identity token
+ * @param response - The HTTP response object
+ * @param responseText - The raw response body text
+ * @returns Detailed error message about the failure
*/
-export declare function pollForDeviceAuthorization(code: string, interval?: number): Promise<IdentityToken>;
\ No newline at end of file
+export declare function buildAuthorizationParseErrorMessage(response: Response, responseText: string): string;
\ No newline at end of file
packages/cli-kit/dist/private/node/session/exchange.d.ts@@ -1,11 +1,6 @@
import { ApplicationToken, IdentityToken } from './schema.js';
import { API } from '../api.js';
-import { Result } from '../../../public/node/result.js';
-import { ExtendableError } from '../../../public/node/error.js';
-export declare class InvalidGrantError extends ExtendableError {
-}
-export declare class InvalidRequestError extends ExtendableError {
-}
+export { InvalidGrantError, InvalidRequestError } from './token-utils.js';
export interface ExchangeScopes {
admin: string[];
partners: string[];
@@ -13,6 +8,13 @@ export interface ExchangeScopes {
businessPlatform: string[];
appManagement: string[];
}
+/**
+ * Request an identity token using the device authorization flow.
+ * This initiates the full flow: request device code, show to user, and poll for completion.
+ * @param scopes - The scopes to request
+ * @returns An identity token
+ */
+export declare function requestAccessToken(scopes: string[]): Promise<IdentityToken>;
/**
* Given an identity token, request an application token.
* @param identityToken - access token obtained in a previous step
@@ -22,9 +24,6 @@ export interface ExchangeScopes {
export declare function exchangeAccessForApplicationTokens(identityToken: IdentityToken, scopes: ExchangeScopes, store?: string): Promise<{
[x: string]: ApplicationToken;
}>;
-/**
- * Given an expired access token, refresh it to get a new one.
- */
export declare function refreshAccessToken(currentToken: IdentityToken): Promise<IdentityToken>;
/**
* Given a custom CLI token passed as ENV variable, request a valid Partners API token
@@ -54,15 +53,6 @@ export declare function exchangeCliTokenForBusinessPlatformAccessToken(token: st
accessToken: string;
userId: string;
}>;
-type IdentityDeviceError = 'authorization_pending' | 'access_denied' | 'expired_token' | 'slow_down' | 'unknown_failure';
-/**
- * Given a deviceCode obtained after starting a device identity flow, request an identity token.
- * @param deviceCode - The device code obtained after starting a device identity flow
- * @param scopes - The scopes to request
- * @returns An instance with the identity access tokens.
- */
-export declare function exchangeDeviceCodeForAccessToken(deviceCode: string): Promise<Result<IdentityToken, IdentityDeviceError>>;
export declare function requestAppToken(api: API, token: string, scopes?: string[], store?: string): Promise<{
[x: string]: ApplicationToken;
-}>;
-export {};
\ No newline at end of file
+}>;
\ No newline at end of file
packages/cli-kit/dist/private/node/session/schema.d.ts@@ -12,8 +12,8 @@ declare const IdentityTokenSchema: zod.ZodObject<{
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -34,8 +34,8 @@ declare const ApplicationTokenSchema: zod.ZodObject<{
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -54,8 +54,8 @@ declare const SessionSchema: zod.ZodObject<{
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -73,8 +73,8 @@ declare const SessionSchema: zod.ZodObject<{
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -88,8 +88,8 @@ declare const SessionSchema: zod.ZodObject<{
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -103,8 +103,8 @@ declare const SessionSchema: zod.ZodObject<{
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -116,16 +116,16 @@ declare const SessionSchema: zod.ZodObject<{
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -166,8 +166,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -185,8 +185,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -200,8 +200,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -215,8 +215,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -228,16 +228,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -269,8 +269,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -288,8 +288,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -303,8 +303,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -318,8 +318,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -331,16 +331,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -372,8 +372,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -391,8 +391,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -406,8 +406,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -421,8 +421,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -434,16 +434,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -475,8 +475,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -494,8 +494,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -509,8 +509,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -524,8 +524,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -537,16 +537,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -578,8 +578,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -597,8 +597,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -612,8 +612,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -627,8 +627,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -640,16 +640,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -681,8 +681,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -700,8 +700,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -715,8 +715,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -730,8 +730,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -743,16 +743,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -784,8 +784,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -803,8 +803,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -818,8 +818,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -833,8 +833,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -846,16 +846,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -887,8 +887,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -906,8 +906,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -921,8 +921,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -936,8 +936,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -949,16 +949,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
@@ -990,8 +990,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
}, "strip", zod.ZodTypeAny, {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
}, {
@@ -1009,8 +1009,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -1024,8 +1024,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -1039,8 +1039,8 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
storeFqdn: zod.ZodOptional<zod.ZodString>;
}, "strip", zod.ZodTypeAny, {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
}, {
accessToken: string;
@@ -1052,16 +1052,16 @@ export declare const SessionsSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{}
identity: {
accessToken: string;
refreshToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
userId: string;
alias?: string | undefined;
};
applications: {} & {
[k: string]: {
accessToken: string;
- scopes: string[];
expiresAt: Date;
+ scopes: string[];
storeFqdn?: string | undefined;
};
};
packages/cli-kit/dist/private/node/ui/components/Tasks.d.ts@@ -1,8 +1,7 @@
import { AbortSignal } from '../../../../public/node/abort.js';
-import { TokenizedString } from '../../../../public/node/output.js';
import React from 'react';
export interface Task<TContext = unknown> {
- title: string | TokenizedString;
+ title: string;
task: (ctx: TContext, task: Task<TContext>) => Promise<void | Task<TContext>[]>;
retry?: number;
retryCount?: number;
packages/cli-kit/dist/public/node/vendor/dev_server/dev-server-2024.d.ts@@ -3,7 +3,4 @@ export declare function createServer(projectName: string): {
host: (options?: HostOptions) => string;
url: (options?: HostOptions) => string;
};
-declare function assertRunning2024(projectName: string): void;
-declare let assertRunningOverride: typeof assertRunning2024 | undefined;
-export declare function setAssertRunning(override: typeof assertRunningOverride): void;
-export {};
\ No newline at end of file
+export declare function isRunning2024(projectName: string): boolean;
\ No newline at end of file
packages/cli-kit/dist/public/node/vendor/dev_server/types.d.ts@@ -8,4 +8,5 @@ export interface DevServer {
}
export interface HostOptions {
nonstandardHostPrefix?: string;
+ useMockIfNotRunning?: boolean;
}
\ No newline at end of file
|
Contributor
Author
There was a problem hiding this comment.
I've moved only flows pertaining to network requests specifically to the client to keep it super lightweight. Shared modules in session/* are then pulled in to existing flows as needed, with the client only being used for the token exchanges/fetches
Contributor
There was a problem hiding this comment.
IMO thinning out the client here weakens the abstraction:
- There's code in sesssion/exchange that is actually part of the service client "stack" but isn't part of the service client (i.e. polling code that doesn't execute if you are using the mock client)
- There's more temporal coupling going on (call this method, then that method).
Light weight isn't necessarily good in this context - I think the "right weight" (ahaha rhyming) is what we want, where the implementation code that "belongs" behind the interface lives behind the interface.
Having an abstraction just over the network part is better than having none (we do need the local dev solution!), but my gut here is that the circular dependency issue led us to making other changes that weakened the rest of the architecture you had established.
I strongly suspect there would be another solution to the circular dependency issue that would still allow us to keep the higher level interface with more encapsulated code behind the interface. My guess is there would be even more things we could pull behind the interface as a way to fix the circular dependency but I can't be sure without reviewing what the circular relationship actually was (is this written up somewhere I can 👀 ?)
Contributor
Author
There was a problem hiding this comment.
This shared modules prevents circular dependencies as both the existing modules and our client pulls from them.
Contributor
Author
There was a problem hiding this comment.
This is also fully tested by the other existing shared module tests, but I can add another layer of coverage here if we wanted
Contributor
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3366 tests passing in 1381 suites. Report generated by 🧪jest coverage report action from 95f8fe3 |
MitchDickinson
requested changes
Nov 24, 2025
Contributor
MitchDickinson
left a comment
MitchDickinson
left a comment
There was a problem hiding this comment.
I'd like to explore alternate solutions to the circular dep issue before we merge this - comments in line
| abstract refreshAccessToken(currentToken: IdentityToken): Promise<IdentityToken> | ||
| abstract requestDeviceAuthorization(scopes: string[]): Promise<DeviceAuthorizationResponse> | ||
|
|
||
| abstract clientId(): string |
Contributor
There was a problem hiding this comment.
clientId() doesn't need to be abstract here. Instead, we should leave it as a class-specific implementation for the mock/production clients and then fix the temporal coupling issues related to it by changing the interface
| * @returns An identity token | ||
| */ | ||
| export async function requestAccessToken(scopes: string[]): Promise<IdentityToken> { | ||
| const identityClient = getIdentityClient() |
Contributor
There was a problem hiding this comment.
I think there's more temporal coupling stuff going on. Example: refreshAccessToken in exchange requires clientId() to be called and then passed into tokenRequest following a specific shape.
This method invokes pollForDeviceAuthorization which a few levels down invokes tokenRequest. For this one, we aren't propagating the identityClient that we resolve in this method (line 29), so getIdentityClient() is assumed to be effectively global from the POV of this class (since everyone calls out to it) which I don't think is the right thing. If it's important that the code here interacts with the same identity client through its flows, it should resolve it once and propagate it
| * @param interval - The interval to poll the token endpoint | ||
| * @returns The identity token | ||
| */ | ||
| private async pollForDeviceAuthorization(code: string, interval = 5): Promise<IdentityToken> { |
Contributor
There was a problem hiding this comment.
This feels like a loss to me - this code is directly related to the service client and the encapsulation was nice IMO
Contributor
There was a problem hiding this comment.
IMO thinning out the client here weakens the abstraction:
- There's code in sesssion/exchange that is actually part of the service client "stack" but isn't part of the service client (i.e. polling code that doesn't execute if you are using the mock client)
- There's more temporal coupling going on (call this method, then that method).
Light weight isn't necessarily good in this context - I think the "right weight" (ahaha rhyming) is what we want, where the implementation code that "belongs" behind the interface lives behind the interface.
Having an abstraction just over the network part is better than having none (we do need the local dev solution!), but my gut here is that the circular dependency issue led us to making other changes that weakened the rest of the architecture you had established.
I strongly suspect there would be another solution to the circular dependency issue that would still allow us to keep the higher level interface with more encapsulated code behind the interface. My guess is there would be even more things we could pull behind the interface as a way to fix the circular dependency but I can't be sure without reviewing what the circular relationship actually was (is this written up somewhere I can 👀 ?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
WHY are these changes introduced?
Part of: shop/issues-develop#21594
WHAT is this pull request doing?
How to test your changes?
pnpm shopify auth logout && SHOPIFY_CLI_NEVER_USE_PARTNERS_API=1 SHOPIFY_SERVICE_ENV=local pnpm shopify app infopnpm shopify auth logoutto test without the entire flowWe are still blocked on BP + Management API auth checks, so these patches are required to allow the CLI to authenticate correctly in all non-prod scenarios
BP Patch
In
db/data/access/api_clients.ymladdCore Patch
In
components/apps/app/controllers/concerns/organization_authorization_concern.rb#organization_user