Report store lifecycle command telemetry - #8389
Open
dmerand wants to merge 2 commits into
Open
Conversation
Assisted-By: devx/9c08eef3-3e18-4b5f-8ed8-bef380040c5d
dmerand
marked this pull request as ready for review
August 25, 2026 13:34
dmerand
marked this pull request as draft
August 25, 2026 13:34
Contributor
There was a problem hiding this comment.
Pull request overview
Improves command analytics coverage for store create dev and store delete so that expected failures emitted as exact JSON (and exiting early) still get reported, and lifecycle commands gain store attribution metadata.
Changes:
- Added synchronous analytics requirement and explicit
reportAnalyticsEvent(...)reporting for--json+AbortErrorpaths before printing JSON and exiting. - Recorded store attribution metadata for
store delete(unvalidated from flag, upgraded to validated when shop lookup yields an ID) and forstore create dev(validated once a domain is returned). - Updated unit tests to cover the new attribution + expected-error analytics behavior and expanded mocking to support multiple lookup responses.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/store/src/cli/services/store/delete/dev.ts | Records validated store attribution when shop lookup succeeds (including shop ID when available). |
| packages/store/src/cli/services/store/delete/dev.test.ts | Adds coverage for attribution recording across lookup scenarios. |
| packages/store/src/cli/services/store/create/dev.ts | Records validated store attribution as soon as the created store domain is available. |
| packages/store/src/cli/services/store/create/dev.test.ts | Verifies attribution is recorded before later polling failures and not recorded for early mutation failures. |
| packages/store/src/cli/commands/store/delete.ts | Forces synchronous analytics and explicitly reports expected JSON aborts; records initial unvalidated store metadata. |
| packages/store/src/cli/commands/store/delete.test.ts | Validates synchronous analytics, JSON abort reporting, and attribution recording in command-level tests. |
| packages/store/src/cli/commands/store/create/dev.ts | Forces synchronous analytics and explicitly reports expected JSON aborts; expands try/catch to include org selection and prompts. |
| packages/store/src/cli/commands/store/create/dev.test.ts | Validates synchronous analytics and JSON abort reporting including org-selection aborts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Assisted-By: devx/65d0e15e-19bf-4af6-8cb3-53772dd9d107
dmerand
marked this pull request as ready for review
August 25, 2026 13:57
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?
Context: https://github.com/shop/issues-develop/issues/22725
store create devandstore deleteprint exact JSON for expected errors and exit directly. This path skipped the async command analytics report, so JSON expected failures were invisible inapp_cli3_command. Command analytics also had no store attribution for these lifecycle commands.WHAT is this pull request doing?
Existing command analytics now covers JSON expected failures and gains known store attribution. No new events, no schema changes, and no telemetry-only network requests.
requiresSyncAnalyticsand await expected-error reporting for their JSONAbortErrorpaths before the exact JSON output and direct exit.store deleterecords the parsed store FQDN as unvalidated; the existing accessible-shop lookup upgrades it to validated with the shop ID when available.store create devrecords the returned domain as validated before polling.How to test your changes?
No behavior or output change is intended. Run
store create dev --jsonorstore delete --jsonwith an expected failure (for example, an inaccessible store) and confirm the JSON error output and exit code are unchanged.Smoke test: Compared the PR head with the base using a non-interactive
store delete --jsonexpected failure and disabled analytics delivery. Both returned byte-identical JSON and exit code 1. Only the PR head built the awaitedexpected_errorpayload before exit, including the unvalidated store domain and matching hash. No analytics event was sent and no real store was created or deleted. Thestore create deverror path was verified through focused command tests because a live create would mutate state.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset addNo changeset: internal telemetry only, no user-visible behavior or output change.