Skip to content
7 changes: 7 additions & 0 deletions .changeset/fix-reset-spurious-toml-prompt.md
Comment thread
timothytenzin20 marked this conversation as resolved.
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
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