Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-reset-spurious-toml-prompt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/app': patch
---

Avoid spurious config prompts:
- Skip TOML selection prompt when using --reset flag
- Use default shopify.app.toml without prompting when running `config link --client-id` with no existing TOML files
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,13 @@ export type Scalars = {
URL: { input: string; output: string; }
};

/** Operators for filter queries. */
/** Operators for user filter queries. */
export type Operator =
/** Between operator. */
| 'BETWEEN'
/** Equals operator. */
| 'EQUALS'
/**
* In operator. Accepts a comma-separated string of values (e.g.
* "value1,value2,value3"). Not supported for all filter fields.
*/
/** In operator. */
| 'IN';

export type OrganizationUserProvisionShopAccessInput = {
Expand Down Expand Up @@ -118,23 +115,17 @@ export type ShopFilterField =
* `inactive`, `cancelled`, `client_transfer`, `plus_client_transfer`,
* `development_legacy`, `custom`, `fraudulent`, `staff`, `trial`,
* `plus_development`, `retail`, `shop_pay_commerce_components`, `non_profit`.
* With the `In` operator, use raw plan names (e.g. "professional,shopify_plus").
*/
| 'SHOP_PLAN'
/** The active/inactive status of the shop. Values: `active`, `inactive`. */
| 'STORE_STATUS'
/**
* The type of the shop. Does not support the `In` operator. Values:
* `development`, `production`, `app_development`, `development_superset`,
* `client_transfer`, `collaborator`.
* The type of the shop. Values: `development`, `production`, `app_development`,
* `development_superset`, `client_transfer`, `collaborator`.
*/
| 'STORE_TYPE';

/**
* Represents a single filter option for shop queries. When using the `In`
* operator, pass a comma-separated string of values (e.g. "value1,value2").
* Maximum 20 values.
*/
/** Represents a single filter option for shop queries. */
export type ShopFilterInput = {
field: ShopFilterField;
operator: Operator;
Expand Down
17 changes: 14 additions & 3 deletions packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,13 @@ export async function loadApp<TModuleSpec extends ExtensionSpecification = Exten
specifications: TModuleSpec[]
remoteFlags?: Flag[]
ignoreUnknownExtensions?: boolean
skipPrompts?: boolean
}): Promise<AppInterface<CurrentAppConfiguration, TModuleSpec>> {
const {project, activeConfig} = await getAppConfigurationContext(options.directory, options.userProvidedConfigName)
const {project, activeConfig} = await getAppConfigurationContext(
options.directory,
options.userProvidedConfigName,
options.skipPrompts ? {skipPrompts: true} : undefined,
)
return loadAppFromContext({
project,
activeConfig,
Expand Down Expand Up @@ -395,11 +400,16 @@ export async function loadOpaqueApp(options: {
configName?: string
specifications: ExtensionSpecification[]
remoteFlags?: Flag[]
skipPrompts?: boolean
}): Promise<OpaqueAppLoadResult> {
// Try to load the app normally first — the loader always collects validation errors,
// so only structural failures (TOML parse, missing files) will throw.
try {
const {project, activeConfig} = await getAppConfigurationContext(options.directory, options.configName)
const {project, activeConfig} = await getAppConfigurationContext(
options.directory,
options.configName,
options.skipPrompts ? {skipPrompts: true} : undefined,
)
const app = await loadAppFromContext({
project,
activeConfig,
Expand Down Expand Up @@ -913,9 +923,10 @@ type ConfigurationLoaderResult<
export async function getAppConfigurationContext(
workingDirectory: string,
userProvidedConfigName?: string,
options?: {skipPrompts?: boolean},
): Promise<{project: Project; activeConfig: ActiveConfig}> {
const project = await Project.load(workingDirectory)
const activeConfig = await selectActiveConfig(project, userProvidedConfigName)
const activeConfig = await selectActiveConfig(project, userProvidedConfigName, options)
return {project, activeConfig}
}

Expand Down
12 changes: 9 additions & 3 deletions packages/app/src/cli/models/project/active-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,20 @@ export interface ActiveConfig {
* config's client_id.
* @public
*/
export async function selectActiveConfig(project: Project, userProvidedConfigName?: string): Promise<ActiveConfig> {
export async function selectActiveConfig(
project: Project,
userProvidedConfigName?: string,
options?: {skipPrompts?: boolean},
): Promise<ActiveConfig> {
let configName = userProvidedConfigName

// Check cache for previously selected config
const cachedConfigName = getCachedAppInfo(project.directory)?.configFile
const cachedConfigPath = cachedConfigName ? joinPath(project.directory, cachedConfigName) : null
const cacheIsStale = Boolean(!configName && cachedConfigPath && !fileExistsSync(cachedConfigPath))

// Handle stale cache: cached config file no longer exists
if (!configName && cachedConfigPath && !fileExistsSync(cachedConfigPath)) {
if (cacheIsStale && !options?.skipPrompts) {
const warningContent = {
headline: `Couldn't find ${cachedConfigName}`,
body: [
Expand All @@ -71,7 +76,8 @@ export async function selectActiveConfig(project: Project, userProvidedConfigNam
configName = await use({directory: project.directory, warningContent, shouldRenderSuccess: false})
}

configName = configName ?? cachedConfigName
// Don't fall back to stale cached name — it points to a non-existent file
configName = configName ?? (cacheIsStale ? undefined : cachedConfigName)

// Determine source after resolution so it reflects the actual selection path
let source: ConfigSource
Expand Down
38 changes: 38 additions & 0 deletions packages/app/src/cli/services/app-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,44 @@ client_id="test-api-key"`
})
})

test('forceRelink skips config selection before link to avoid spurious TOML prompt', async () => {
await inTemporaryDirectory(async (tmp) => {
// Given — no config file on disk, so getAppConfigurationContext would prompt for TOML selection
// if called before link. We verify link is called first (without a prior config load).
const content = `
name = "test-app"
client_id="test-api-key"`
await writeAppConfig(tmp, content)

const getAppConfigSpy = vi.spyOn(loader, 'getAppConfigurationContext')

vi.mocked(link).mockResolvedValue({
remoteApp: mockRemoteApp,
configFileName: 'shopify.app.toml',
configuration: {
client_id: 'test-api-key',
name: 'test-app',
path: normalizePath(joinPath(tmp, 'shopify.app.toml')),
} as any,
})

// When
await linkedAppContext({
directory: tmp,
forceRelink: true,
userProvidedConfigName: undefined,
clientId: undefined,
})

// Then — getAppConfigurationContext should only be called AFTER link, not before
const linkCallOrder = vi.mocked(link).mock.invocationCallOrder[0]!
const configCallOrders = getAppConfigSpy.mock.invocationCallOrder
expect(configCallOrders.every((order) => order > linkCallOrder)).toBe(true)

getAppConfigSpy.mockRestore()
})
})

test('logs metadata', async () => {
await inTemporaryDirectory(async (tmp) => {
// Given
Expand Down
31 changes: 22 additions & 9 deletions packages/app/src/cli/services/app-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,34 @@ export async function linkedAppContext({
userProvidedConfigName,
unsafeTolerateErrors = false,
}: LoadedAppContextOptions): Promise<LoadedAppContextOutput> {
let {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName)
let project: Project
let activeConfig: ActiveConfig
let remoteApp: OrganizationApp | undefined

if (activeConfig.file.errors.length > 0) {
throw new AbortError(activeConfig.file.errors.map((err) => err.message).join('\n'))
}

if (!activeConfig.isLinked || forceRelink) {
const configName = forceRelink ? undefined : basename(activeConfig.file.path)
const result = await link({directory, apiKey: clientId, configName})
if (forceRelink) {
// Skip getAppConfigurationContext() when force-relinking — it may prompt the
// user to select a TOML file that will be immediately discarded by link().
const result = await link({directory, apiKey: clientId})
remoteApp = result.remoteApp
// Re-load project and re-select active config since link may have written new config
const reloaded = await getAppConfigurationContext(directory, result.configFileName)
project = reloaded.project
activeConfig = reloaded.activeConfig
} else {
const loaded = await getAppConfigurationContext(directory, userProvidedConfigName)
project = loaded.project
activeConfig = loaded.activeConfig

if (activeConfig.file.errors.length > 0) {
throw new AbortError(activeConfig.file.errors.map((err) => err.message).join('\n'))
}

if (!activeConfig.isLinked) {
const result = await link({directory, apiKey: clientId, configName: basename(activeConfig.file.path)})
remoteApp = result.remoteApp
const reloaded = await getAppConfigurationContext(directory, result.configFileName)
project = reloaded.project
activeConfig = reloaded.activeConfig
}
}

// Determine the effective client ID
Expand Down
59 changes: 46 additions & 13 deletions packages/app/src/cli/services/app/config/link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,16 @@ url = "https://api-client-config.com/preferences"
},
} as CurrentAppConfiguration,
}
// Write actual TOML file so getTomls() finds it and reuses existing config
const filePath = joinPath(tmp, 'shopify.app.development.toml')
writeFileSync(filePath, 'client_id = "12345"\nname = "my app"')
await mockLoadOpaqueAppWithApp(tmp, localApp, [], 'current')
vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(
testOrganizationApp({
apiKey: '12345',
developerPlatformClient,
}),
)
vi.mocked(selectConfigName).mockResolvedValue('shopify.app.staging.toml')
const remoteConfiguration = {
...DEFAULT_REMOTE_CONFIGURATION,
name: 'my app',
Expand All @@ -472,8 +474,9 @@ url = "https://api-client-config.com/preferences"
// When
const {configuration} = await link(options)

// Then
const content = await readFile(joinPath(tmp, 'shopify.app.staging.toml'))
// Then - since client_id matches and file exists, reuse shopify.app.development.toml
expect(selectConfigName).not.toHaveBeenCalled()
const content = await readFile(joinPath(tmp, 'shopify.app.development.toml'))
const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration

client_id = "12345"
Expand Down Expand Up @@ -501,14 +504,14 @@ embedded = false
`

expect(setCurrentConfigPreference).toHaveBeenCalledWith(configuration, {
configFileName: 'shopify.app.staging.toml',
configFileName: 'shopify.app.development.toml',
directory: tmp,
})
expect(renderSuccess).toHaveBeenCalledWith({
headline: 'shopify.app.staging.toml is now linked to "my app" on Shopify',
body: 'Using shopify.app.staging.toml as your default config.',
headline: 'shopify.app.development.toml is now linked to "my app" on Shopify',
body: 'Using shopify.app.development.toml as your default config.',
nextSteps: [
[`Make updates to shopify.app.staging.toml in your local project`],
[`Make updates to shopify.app.development.toml in your local project`],
['To upload your config, run', {command: 'yarn shopify app deploy'}],
],
reference: [
Expand Down Expand Up @@ -569,6 +572,9 @@ embedded = false
},
},
}
// Write actual TOML file so getTomls() finds it (needed for selectConfigName logic)
const filePath = joinPath(tmp, 'shopify.app.toml')
writeFileSync(filePath, 'client_id = "12345"\nname = "my app"')
await mockLoadOpaqueAppWithApp(tmp, localApp, [], 'current')
vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(
testOrganizationApp({
Expand Down Expand Up @@ -981,6 +987,30 @@ embedded = false
},
})
expect(content).toEqual(expectedContent)
// Should use default filename without prompting when no TOMLs exist
expect(selectConfigName).not.toHaveBeenCalled()
})
})

test('uses default shopify.app.toml without prompting when no config files exist and client-id is provided', async () => {
await inTemporaryDirectory(async (tmp) => {
// Given - no TOML files in directory, but client-id is provided
// This simulates: shopify app config link --client-id=12345
const developerPlatformClient = buildDeveloperPlatformClient()
const options: LinkOptions = {
directory: tmp,
apiKey: '12345',
developerPlatformClient,
}
mockLoadOpaqueAppWithError()
vi.mocked(appFromIdentifiers).mockResolvedValue(mockRemoteApp({developerPlatformClient}))

// When
const {configFileName} = await link(options)

// Then - should use default filename without prompting
expect(configFileName).toBe('shopify.app.toml')
expect(selectConfigName).not.toHaveBeenCalled()
})
})

Expand Down Expand Up @@ -1323,14 +1353,16 @@ embedded = false
embedded: true,
},
}
// Write actual TOML file so getTomls() finds it and reuses the existing config
const filePath = joinPath(tmp, 'shopify.app.toml')
writeFileSync(filePath, 'client_id = "12345"\nname = "my app"')
await mockLoadOpaqueAppWithApp(tmp, localApp, [], 'current')
vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(
testOrganizationApp({
apiKey: '12345',
developerPlatformClient,
}),
)
vi.mocked(selectConfigName).mockResolvedValue('shopify.app.staging.toml')
const remoteConfiguration = {
...DEFAULT_REMOTE_CONFIGURATION,
name: 'my app',
Expand All @@ -1347,8 +1379,9 @@ embedded = false
// When
await link(options)

// Then
const content = await readFile(joinPath(tmp, 'shopify.app.staging.toml'))
// Then - since client_id matches, the existing shopify.app.toml is reused (no prompt)
expect(selectConfigName).not.toHaveBeenCalled()
const content = await readFile(joinPath(tmp, 'shopify.app.toml'))
const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration

client_id = "12345"
Expand All @@ -1375,10 +1408,10 @@ embedded = true
`
expect(content).toEqual(expectedContent)
expect(renderSuccess).toHaveBeenCalledWith({
headline: 'shopify.app.staging.toml is now linked to "my app" on Shopify',
body: 'Using shopify.app.staging.toml as your default config.',
headline: 'shopify.app.toml is now linked to "my app" on Shopify',
body: 'Using shopify.app.toml as your default config.',
nextSteps: [
[`Make updates to shopify.app.staging.toml in your local project`],
[`Make updates to shopify.app.toml in your local project`],
['To upload your config, run', {command: 'yarn shopify app deploy'}],
],
reference: [
Expand Down
5 changes: 5 additions & 0 deletions packages/app/src/cli/services/app/config/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ async function getAppCreationDefaultsFromLocalApp(options: LinkOptions): Promise
directory: options.directory,
userProvidedConfigName: options.configName,
remoteFlags: undefined,
skipPrompts: true,
})

return {creationOptions: app.creationDefaultOptions(), appDirectory: app.directory}
Expand Down Expand Up @@ -231,6 +232,7 @@ export async function loadLocalAppOptions(
configName: options.configName,
specifications,
remoteFlags,
skipPrompts: true,
})

switch (result.state) {
Expand Down Expand Up @@ -308,6 +310,9 @@ async function loadConfigurationFileName(
const currentToml = existingTomls[remoteApp.apiKey]
if (currentToml) return currentToml

// If no TOML files exist at all, use the default filename without prompting
if (isEmpty(existingTomls)) return 'shopify.app.toml'

return selectConfigName(localAppInfo.appDirectory ?? options.directory, remoteApp.title)
}

Expand Down
Loading