Repository navigation
Normalize store domains for app-admin authentication - #9044
Merged
Merged
Conversation
Use the shared store-domain normalizer so app-admin authentication follows the same input rules as other CLI Kit helpers. Keep the returned session and error messages consistent with the normalized domain. Co-authored-by: River <river@shopify.com>
madmath
marked this pull request as ready for review
October 29, 2026 15:15
Contributor
Author
|
@stephanie-shopify could you review this change? Please see the description for compatibility questions and verification limitations. |
Allow native PowerShell startup on shared Windows runners without applying the in-process unit-test deadline. Bound the subprocess at 25 seconds within a 30-second test budget, and report process errors while retaining the exit-code and UTF-8 assertions.
hwdavidward
reviewed
Oct 30, 2026
hwdavidward
approved these changes
Oct 30, 2026
madmath
enabled auto-merge
October 30, 2026 13:36
dmerand
approved these changes
Oct 30, 2026
dmerand
left a comment
dmerand
left a comment
Contributor
There was a problem hiding this comment.
Thanks for making domain normalization more consistent 👍🏼
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
App-admin authentication should use the same store-domain rules as other CLI Kit APIs.
WHAT is this pull request doing?
Apply the shared store normalizer in the public authentication helper.
Compatibility review needed: callers that use non-default ports or userinfo now receive validation errors, and the returned store domain is normalized. The changeset currently uses
patch; maintainers should confirm the versioning and deprecation requirements before release.Validation
Node 26.12.1, pnpm 10.11.1:
pnpm vitest run packages/cli-kit/src/public/node/session.test.ts packages/cli-kit/src/public/node/context/fqdn.test.ts— 77 passed. All five added cases failed before the fix; 40 existing session tests passed.pnpm nx run-many --all --target=type-check— passed.pnpm nx run-many --all --target=lint— passed.pnpm refresh-code-documentation— passed, no generated changes.git diff --check— passed.The full unit suite, cross-platform matrix, and E2E tests were not run locally.
CI follow-up: f514706be2 gives the Windows PowerShell encoding test a separate startup budget and a shorter subprocess timeout, retaining its exit-code and UTF-8 assertions. The previously failing Windows/Node 26 job and all 12 unit-test matrix jobs now pass, along with the overall unit-test gate. Focused local verification: 102 tests passed, 2 Windows-only tests skipped on macOS; affected-file ESLint and app type checks passed.
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add