Skip to content

Add store command analytics attribution#7428

Open
dmerand wants to merge 1 commit intomainfrom
dlm-store-command-metrics
Open

Add store command analytics attribution#7428
dmerand wants to merge 1 commit intomainfrom
dlm-store-command-metrics

Conversation

@dmerand
Copy link
Copy Markdown
Contributor

@dmerand dmerand commented Apr 29, 2026

WHY are these changes introduced?

shopify store auth and shopify store execute currently under-attribute command analytics compared to app workflows:

  • store auth / execute should set the command issuer user_id
  • StoreCommand-based commands with --store should expose a public store identifier hash

This PR intentionally avoids shop_id because that depends on a Monorail schema update. The shop_id work is split into the stacked PR #7429.

WHAT is this pull request doing?

Adds existing-schema analytics attribution for store commands:

  • records user_id for:
    • store auth from the OAuth associated_user.id
    • store execute from the stored store auth session user
  • records public store_fqdn_hash for StoreCommand-based commands with --store
  • keeps raw store_fqdn in sensitive metadata only

The user ID helper is deliberately narrow: store commands set the command analytics user without introducing a generic public metadata override for user_id.

Post-release steps

None for this PR. It only uses existing Monorail fields.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • This is not user-facing CLI behavior and does not need a changeset

Copy link
Copy Markdown
Contributor Author

dmerand commented Apr 29, 2026

@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Apr 29, 2026
@dmerand dmerand changed the title Stop reporting expected Admin API failures from store execute as CLI bugs Add store command analytics attribution Apr 29, 2026
@dmerand dmerand changed the base branch from main to graphite-base/7428 April 29, 2026 21:18
@dmerand dmerand force-pushed the dlm-store-command-metrics branch from 6d2463a to 4e58f7b Compare April 29, 2026 21:18
@dmerand dmerand force-pushed the graphite-base/7428 branch from dd1278d to 4b99cef Compare April 29, 2026 21:18
@dmerand dmerand changed the base branch from graphite-base/7428 to dlm-store-execute-error-classify April 29, 2026 21:18
Base automatically changed from dlm-store-execute-error-classify to main April 30, 2026 13:45
@dmerand dmerand force-pushed the dlm-store-command-metrics branch 2 times, most recently from bac2759 to c447b80 Compare May 1, 2026 18:57
@dmerand dmerand marked this pull request as ready for review May 1, 2026 19:09
@dmerand dmerand requested a review from a team as a code owner May 1, 2026 19:09
Copilot AI review requested due to automatic review settings May 1, 2026 19:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves analytics attribution for shopify store commands by ensuring store commands set the command analytics user_id and consistently emit a hashed public store identifier (store_fqdn_hash) while keeping the raw store FQDN in sensitive metadata.

Changes:

  • Record store_fqdn_hash (public) + store_fqdn (sensitive) for StoreCommand-based commands when --store is parsed.
  • Record analytics user_id for store auth (from OAuth associated user) and store execute (from stored store auth session).
  • Add a small public cli-kit session helper (setLastSeenUserId) and accompanying tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/store/src/cli/utilities/store-command.ts Adds hashed store attribution recording during --store parsing.
packages/store/src/cli/utilities/store-command.test.ts Verifies StoreCommand parsing records both sensitive and public store metadata.
packages/store/src/cli/services/store/metrics.ts Introduces shared helpers for store fqdn hash metadata and store-command user id attribution.
packages/store/src/cli/services/store/metrics.test.ts Unit tests for hashed store fqdn metadata and setting user id.
packages/store/src/cli/services/store/execute/admin-context.ts Records user_id from stored store session for store execute.
packages/store/src/cli/services/store/execute/admin-context.test.ts Asserts recordStoreCommandUserId is called from stored session user id.
packages/store/src/cli/services/store/auth/index.ts Records user_id from OAuth associated_user.id after auth.
packages/store/src/cli/services/store/auth/index.test.ts Asserts recordStoreCommandUserId is called with the associated user id.
packages/store/src/cli/commands/store/execute.test.ts Mocks metrics module to isolate command tests from attribution side effects.
packages/store/src/cli/commands/store/auth.test.ts Mocks metrics module to isolate command tests from attribution side effects.
packages/cli-kit/src/public/node/session.ts Adds setLastSeenUserId public wrapper used by store commands to set analytics user id.
packages/cli-kit/src/public/node/session.test.ts Tests the new public setLastSeenUserId wrapper delegates to the private setter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/store/src/cli/services/store/metrics.ts Outdated
@dmerand dmerand force-pushed the dlm-store-command-metrics branch from c447b80 to 58ecacb Compare May 1, 2026 19:21
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/session.d.ts
@@ -16,6 +16,12 @@ export interface Session {
     userId: string;
 }
 export type AccountInfo = UserAccountInfo | ServiceAccountInfo | UnknownAccountInfo;
+/**
+ * Records the user ID that should be attached to command analytics for this process.
+ *
+ * @param userId - User identifier to report on the command analytics event.
+ */
+export declare function setLastSeenUserId(userId: string): void;
 interface UserAccountInfo {
     type: 'UserAccount';
     email: string;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants