Skip to content

Fix --reset flag regression: skip spurious TOML selection prompt before link flow#7238

Open
timothytenzin20 wants to merge 1 commit intomainfrom
04-08-fix_reset_spurious_toml_selection_prompt
Open

Fix --reset flag regression: skip spurious TOML selection prompt before link flow#7238
timothytenzin20 wants to merge 1 commit intomainfrom
04-08-fix_reset_spurious_toml_selection_prompt

Conversation

@timothytenzin20
Copy link
Copy Markdown

@timothytenzin20 timothytenzin20 commented Apr 9, 2026

Bug

shopify app dev --reset spuriously prompts TOML file selection before the reset flow (org → app → config name). The selected TOML is immediately discarded.

Issue: shop/issues-develop#22531

Root Cause

Regression introduced by #6612 ("Remove legacy app schema", merged Mar 2026). Commit 1f8dcceaec ("Simplify code") merged the forceRelink and !isLinked branches back together, undoing a prior fix from #5676 (Apr 2025) which fixed #5653 by separating the forceRelink path so it skips config state loading.

getAppConfigurationContext() is called unconditionally before checking forceRelink. When the cached config file doesn't exist on disk, it calls selectConfigFile(), prompting the user to pick a TOML — even though forceRelink will discard the result.

Fix

Separate the two paths so forceRelink skips getAppConfigurationContext() entirely and calls link() directly:

if (forceRelink) {
  const result = await link({directory, apiKey: clientId})
  remoteApp = result.remoteApp
  // load config context from link result
} else {
  // load config context first, then link if not linked
}

Test

Added test verifying getAppConfigurationContext is only called after link() when forceRelink is true (using invocationCallOrder tracking).

All 12 tests pass ✅

…re link flow

When running `shopify app dev --reset`, `getAppConfigurationContext()` was
called unconditionally before checking `forceRelink`. If the cached config
file didn't exist on disk, this triggered `selectConfigFile()`, prompting
the user to pick a TOML file that would be immediately discarded by `link()`.

Regression introduced by #6612 ("Remove legacy app schema", commit 1f8dcce)
which merged the forceRelink and !isLinked branches back together, undoing
the prior fix from #5676.

Fix: separate the two paths so forceRelink skips `getAppConfigurationContext()`
entirely and calls `link()` directly.

Fixes shop/issues-develop#22531
@timothytenzin20 timothytenzin20 requested review from a team as code owners April 9, 2026 21:02
Copy link
Copy Markdown
Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@gonzaloriestra
Copy link
Copy Markdown
Contributor

/snapit

@github-actions
Copy link
Copy Markdown
Contributor

🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260410122533

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

Copy link
Copy Markdown
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! The fix makes sense, but I think something is not working. I'm still getting the config prompt when running pnpm shopify app info --reset after deleting the current TOML and having multiple ones.

I think what's missing is to avoid it from link. In 04-08-fix_reset_spurious_toml_selection_prompt_2 I added the missing pieces. Still have to review and test more, but it seems to work. Feel free to add here the changes from there if you want.

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.

This is for the users, I think it's too technical. I'd simplify it to something like Avoid config prompt when using --reset flag

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.

[Bug]: app deploy --reset no longer resets toml file configuration

2 participants