Skip to content

fix(app): skip app prompt when config is already linked - #6662

Merged
gonzaloriestra merged 8 commits into
Shopify:mainfrom
anakafeel:feature/skip-prompts-config-link-6293
Dec 2, 2025
Merged

gonzaloriestra merged 8 commits into
Shopify:mainfrom
anakafeel:feature/skip-prompts-config-link-6293

Conversation

@anakafeel

@anakafeel anakafeel commented Nov 24, 2025 •

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

The original issue (#6293) was about reducing unnecessary prompts when working with existing app configuration files.

After clarifications, it became clear that the behavior of:

shopify app config link --config <file>

should not be changed — even when a valid client_id exists, the command must still prompt for org/app selection.

The correct solution is to introduce a new command that pulls the remote app information for an already-linked configuration file, without any prompts.


WHAT is this pull request doing?

This PR introduces a new command:

shopify app config pull

This new command:

  • Detects the currently selected config (or the one specified with --config)
  • Reads and validates the existing client_id
  • Resolves the remote app using the already-linked org/app
  • Pulls the remote configuration data without any interactive prompts
  • Leaves the existing app config link behavior unchanged

This approach aligns with the expected behavior discussed in issue #6293.


How to test your changes

1. Prepare a config file with a valid client_id

Example:

shopify.app.staging.toml

2. Run the new command

pnpm shopify app config pull --config shopify.app.staging.toml

Expected result

  • No prompts appear
  • CLI resolves the remote app from the existing client_id
  • The remote app info is pulled successfully
  • A success message is printed

3. Run without providing --config

pnpm shopify app config pull

  • Should automatically use the currently selected config file.

4. Ensure app config link behavior is unchanged

pnpm shopify app config link --config shopify.app.staging.toml

  • Prompts should still appear as they do today.

Post-release steps

No additional steps required — the new command is self-contained.

@anakafeel

Copy link
Copy Markdown
Contributor Author

I have signed the CLA ✅

@anakafeel
anakafeel requested a review from a team as a code owner November 27, 2025 03:38

@gonzaloriestra gonzaloriestra left a comment

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.

It's looking good, thanks a lot! I added a refactor suggestion to simplify.

You don't have to worry about some of the CI failures, because some don't work on forks. But there are a few of things to fix:

  • Run pnpm test:regenerate-snapshots to add the new command for some tests
  • Run pnpm refresh-manifests to update some auto-generated stuff
  • Run pnpm knip to find some unused exports (and manually fix)

@anakafeel

Copy link
Copy Markdown
Contributor Author

Hey @gonzaloriestra

I’ve completed the refactor based on your suggestion:

  • app config pull now uses linkedAppContext and extends AppLinkedCommand, aligning with the pattern from app generate extension.

  • The pull service now receives configuration and remoteApp from the command and no longer performs duplicated app resolution internally.

  • Ran pnpm refresh-manifests and pnpm knip, and cleaned up the unused exports.

  • Confirmed locally that pnpm shopify app config pull works as expected (no prompts, pulls latest config).

Everything should now be ready for another review, thanks again for the guidance :)

@gonzaloriestra gonzaloriestra left a comment •

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.

Nice work! I've tested it and works great.

Almost there, you just need to run one more command to generate the documentation for the new command (sorry, I forgot to mention it before):

  • pnpm build-dev-docs

Other than that, I added a couple of suggestions to improve the messaging and simplify the code a bit.

@anakafeel

Copy link
Copy Markdown
Contributor Author

@gonzaloriestra
Thanks for the review!
I’ve applied all the suggestions:
– Updated success message
– Removed deploy next step
– Simplified config file name logic
– Rebuilt dev docs
Let me know if you’d like any further adjustments or if this is ready for merge :)

@gonzaloriestra gonzaloriestra left a comment

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.

Thanks a lot for applying the suggestions! Everything looks great now.

The only pending step is to fix the linter: there's an unused import. You can check it with pnpm lint.

Once that's fixed, I'll merge it. Thanks again! 👏

@anakafeel

Copy link
Copy Markdown
Contributor Author

@gonzaloriestra
Oh my bad ! i completely forgot to run the linter to check for unused imports.
i've removed the unused imports now and everything should be good for a merge now :)

@gonzaloriestra
gonzaloriestra merged commit 756d78a into Shopify:main Dec 2, 2025
20 of 25 checks passed
@gonzaloriestra gonzaloriestra mentioned this pull request Dec 2, 2025
3 of 5 tasks
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.

2 participants