Skip to content

Add public store_domain to CLI command analytics - #7578

Merged
dmerand merged 3 commits into
mainfrom
donald/monorail-shop-domain-metrics
Jun 3, 2026
Merged

Add public store_domain to CLI command analytics#7578
dmerand merged 3 commits into
mainfrom
donald/monorail-shop-domain-metrics

Conversation

@dmerand

@dmerand dmerand commented May 18, 2026

Copy link
Copy Markdown
Contributor

Requires https://app.graphite.com/github/pr/Shopify/monorail/23826 to include store_domain in the Monorail schema.

What

Add a public store_domain field to CLI command analytics and bump the Monorail command topic to app_cli3_command/1.26.

This records the normalized shop domain alongside existing store attribution fields in app, store, and theme analytics paths.

Why

The CLI needs a non-sensitive store-domain signal after the sensitive store_fqdn field is redacted. Keeping the field under the store_ prefix makes it consistent with the existing store attribution fields and lets metadata containers include it through existing store_ prefix typing.

How

Populate store_domain wherever CLI command analytics already record store attribution, and keep store_fqdn, store_fqdn_hash, and store_fqdn_validated unchanged.

The app store context path normalizes the selected store domain before logging metadata so the new public field and existing store attribution fields are consistent.

@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 May 18, 2026
@dmerand
dmerand marked this pull request as ready for review May 18, 2026 19:14
@dmerand
dmerand requested review from a team as code owners May 18, 2026 19:14
Copilot AI review requested due to automatic review settings May 18, 2026 19:14

Copilot AI 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.

Pull request overview

Adds a public shop_domain field to CLI command analytics and bumps the Monorail topic to app_cli3_command/1.25. The field is populated alongside existing store attribution in app, store, and theme analytics paths, and a small bug in storeContext is fixed so the store FQDN is normalized before logging.

Changes:

  • Add shop_domain to public metadata in app dev, store attribution, store-context, and theme command analytics.
  • Bump Monorail command topic to 1.25 and add shop_domain to the schema and metadata type picks.
  • Normalize selectedStore.shopDomain before logMetadata in storeContext and add a test for it.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/cli-kit/src/public/node/monorail.ts Bump topic to 1.25 and add optional shop_domain to schema.
packages/cli-kit/src/public/node/monorail.test.ts Updated expectations to include shop_domain.
packages/cli-kit/src/public/node/metadata.ts Include shop_domain in CmdFieldsFromMonorail pick.
packages/app/src/cli/metadata.ts Include shop_domain in app metadata public field pick.
packages/app/src/cli/services/dev.ts Record shop_domain in dev metadata.
packages/app/src/cli/services/store-context.ts Normalize store FQDN before logging; record shop_domain.
packages/app/src/cli/services/store-context.test.ts New test for normalization before metadata logging.
packages/store/src/cli/services/store/attribution.ts Add shop_domain to public metadata.
packages/store/src/cli/services/store/attribution.test.ts Updated test to cover shop_domain.
packages/theme/src/cli/utilities/theme-command.ts Record shop_domain in theme metadata.

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

Comment thread packages/app/src/cli/metadata.ts Outdated
PickByPrefix<MonorailEventPublic, 'env_'> &
PickByPrefix<MonorailEventPublic, 'store_'>
PickByPrefix<MonorailEventPublic, 'store_'> &
Pick<MonorailEventPublic, 'shop_domain'>

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.

feels like we could be consistent here and continue using store_prefix to refence shop stuff instead of spliting it, what do you think? I'm aware it'd mean updating the monorail schema

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR to update the topic to have store_domain here https://app.graphite.com/github/pr/Shopify/monorail/23826

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've made the update here as well.

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, thank you!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@isaacroldan this needs a team approval to merge, are you good with the changes?

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.

Sorry, i was sure i approved this, but i obviously didn't 🙈

dmerand commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

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

@dmerand dmerand changed the title Add public shop_domain to CLI command analytics Add public store_shop_domain to CLI command analytics May 26, 2026
@dmerand
dmerand marked this pull request as draft May 26, 2026 20:42
@dmerand
dmerand force-pushed the donald/monorail-shop-domain-metrics branch 2 times, most recently from 8dfcb6f to a53b14a Compare May 28, 2026 18:03
@dmerand dmerand changed the title Add public store_shop_domain to CLI command analytics Add public store_domain to CLI command analytics May 28, 2026
@dmerand
dmerand marked this pull request as ready for review May 28, 2026 20:15
@dmerand
dmerand requested a review from isaacroldan May 28, 2026 20:24
@dmerand
dmerand force-pushed the donald/monorail-shop-domain-metrics branch from a53b14a to a1fe5ec Compare May 28, 2026 20:34
@github-actions

Copy link
Copy Markdown
Contributor

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/monorail.d.ts
@@ -2,7 +2,7 @@ import { JsonMap } from '../../private/common/json.js';
 import { DeepRequired } from '../common/ts/deep-required.js';
 export { DeepRequired };
 type Optional<T> = T | null;
-export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.24";
+export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.26";
 export interface Schemas {
     [MONORAIL_COMMAND_TOPIC]: {
         sensitive: {
@@ -33,6 +33,7 @@ export interface Schemas {
             is_employee: boolean;
             store_fqdn_hash?: Optional<string>;
             store_fqdn_validated?: Optional<boolean>;
+            store_domain?: Optional<string>;
             user_id: string;
             cmd_all_alias_used?: Optional<string>;
             cmd_all_launcher?: Optional<string>;

@dmerand
dmerand added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 42ec1ff Jun 3, 2026
28 checks passed
@dmerand
dmerand deleted the donald/monorail-shop-domain-metrics branch June 3, 2026 07:43
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.

4 participants