Skip to content

Store execute restructure#7188

Open
dmerand wants to merge 1 commit intodlm-store-auth-restructurefrom
dlm-store-restructure
Open

Store execute restructure#7188
dmerand wants to merge 1 commit intodlm-store-auth-restructurefrom
dlm-store-restructure

Conversation

@dmerand
Copy link
Copy Markdown
Contributor

@dmerand dmerand commented Apr 3, 2026

What

Restructure shopify store execute under packages/cli/src/cli/services/store/execute/ and keep the full stored auth session alive through execute context.

This keeps the CLI contract the same while making execute ownership and re-auth behavior more honest.

Why

The lower auth PR gives store auth a cleaner boundary, but store execute was still flattening auth state too early.

In practice that meant execute kept only sessionUserId in its context and fell back to placeholder scopes in its 401 re-auth path. That is the wrong shape for:

  • store-scoped auth invalidation
  • future multi-user local auth
  • low-context maintenance of the execute/auth seam

The goal here is to make execute depend on the auth domain using the real stored auth identity rather than lossy fragments.

How

This PR moves execute code under services/store/execute/ and separates:

  • execute/index.ts — execute orchestration
  • execute/request.ts — request preparation
  • execute/result.ts — result writing/output
  • execute/targets.ts — target selection
  • execute/admin-context.ts — admin context loading
  • execute/admin-transport.ts — admin GraphQL execution

It also changes execute context to carry the full stored auth session instead of only sessionUserId, so execute invalidation can clear the right stored user session and prompt re-auth with the real stored scopes.

High-level comparison:

before: execute context -> sessionUserId + placeholder scopes
after:  execute context -> full stored session identity + real scopes

This PR stays internal to execute ownership and auth-context shape. Broader command-contract work stays in the top stacked PR.

Testing

Manual checks:

pnpm run shopify store execute --store donaldmerand.myshopify.com --query 'query { shop { name id } }'
pnpm run shopify store execute --store donaldmerand.myshopify.com --query 'query { shop { name } }' --version 2025-07

Then invalidate the stored app auth for that store and re-run shopify store execute ....

Verify that:

  • execute still succeeds with valid stored auth
  • the re-auth path points at the same store
  • the re-auth path uses the real stored scopes instead of placeholder text
  • the correct stored user session is invalidated

Considered

  • Fold this into the auth restructure PR: deferred so the lower PR can stay auth-only.
  • Broaden this into more shopify store * contract cleanup: out of scope for this PR.
  • Keep execute context at sessionUserId and patch the 401 path in place: rejected because it preserves the same lossy seam.

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

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

Copy link
Copy Markdown
Contributor Author

dmerand commented Apr 3, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@dmerand dmerand mentioned this pull request Apr 3, 2026
5 tasks
@dmerand dmerand force-pushed the dlm-store-restructure branch from 4edaf22 to b8cace6 Compare April 3, 2026 20:05
@dmerand dmerand force-pushed the dlm-store-auth-restructure branch from ec9fc5b to 176cf3b Compare April 3, 2026 20:05
@dmerand dmerand changed the title store execute restructure Store execute restructure Apr 3, 2026
@dmerand dmerand marked this pull request as ready for review April 3, 2026 20:49
@dmerand dmerand requested a review from a team as a code owner April 3, 2026 20:49
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@dmerand dmerand force-pushed the dlm-store-auth-restructure branch from 176cf3b to 81f77d7 Compare April 3, 2026 20:50
@dmerand dmerand force-pushed the dlm-store-restructure branch from b8cace6 to 2259cc1 Compare April 3, 2026 20:51
Copy link
Copy Markdown
Contributor

@ryancbahan ryancbahan left a comment

Choose a reason for hiding this comment

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

Changes make sense to me. i'm ready to ✅ but realize that I left non-approving comments on the dependent pr below. LMK if you want to ship this stack just to get the initial refactor in, or if you want to investigate any of my comments in below stack.

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