Skip to content

Conversation

@karreiro
Copy link
Contributor

WHY are these changes introduced?

We no longer raise password errors when users use custom app passwords with the shopify theme pull or push commands.

WHAT is this pull request doing?

This change moves logic from packages/theme/src/cli/flags.ts to the specific commands that do not support custom app password authentication. The legacy method is only supported on certain commands.

How to test your changes?

  • Run shopify theme dev --password <custom_app_password>
    • You should see an error.
  • Run shopify theme push --password <custom_app_password>
    • The command should work as expected.

Post-release steps

N/A

Measuring impact

How will we know this change was effective? Please choose one:

  • N/A – this doesn't require measurement, such as a linting rule or a minor bug fix
  • Existing analytics will report on this addition
  • This PR includes analytics changes to measure impact

Checklist

  • I've considered any possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered whether documentation changes are needed

@karreiro karreiro requested review from a team as code owners August 26, 2025 11:35
@github-actions
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.59% (+0.02% 🔼)
13484/17158
🟡 Branches
72.56% (+0.07% 🔼)
6567/9051
🟡 Functions
78.8% (+0.02% 🔼)
3508/4452
🟡 Lines
78.95% (+0.02% 🔼)
12746/16144
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / flags-validation.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / console.ts
41.67% (-3.79% 🔻)
100% 0%
41.67% (-3.79% 🔻)
🔴
... / dev.ts
21.05% (-1.17% 🔻)
0% 0%
21.05% (-1.17% 🔻)
🔴
... / profile.ts
29.41% (-1.84% 🔻)
0% 0%
29.41% (-1.84% 🔻)

Test suite run success

3233 tests passing in 1354 suites.

Report generated by 🧪jest coverage report action from 87bedd2

noCacheDefault: true,
}),
password: Flags.string({
description: 'Password generated from the Theme Access app.',
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if you want this documented or not, but should we write it here so we don't get reports of --password flag being inconsistent?

Suggested change
description: 'Password generated from the Theme Access app.',
description: 'Password generated from the Theme Access app. Legacy: Supports custom app passwords for `push` and `pull` commands.',

Copy link
Contributor

Choose a reason for hiding this comment

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

Spoke about it - we will keep the description as is

@aswamy aswamy added this pull request to the merge queue Aug 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2025
@karreiro karreiro added this pull request to the merge queue Aug 27, 2025
Merged via the queue into main with commit 4a22527 Aug 27, 2025
2 checks passed
@karreiro karreiro deleted the fix-password-errors branch August 27, 2025 07:03
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.

3 participants