Skip to content

Report store lifecycle command telemetry - #8389

Open
dmerand wants to merge 2 commits into
mainfrom
donald/store-lifecycle-telemetry
Open

Report store lifecycle command telemetry#8389
dmerand wants to merge 2 commits into
mainfrom
donald/store-lifecycle-telemetry

Conversation

@dmerand

@dmerand dmerand commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Context: https://github.com/shop/issues-develop/issues/22725

store create dev and store delete print exact JSON for expected errors and exit directly. This path skipped the async command analytics report, so JSON expected failures were invisible in app_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.

  • Both commands set requiresSyncAnalytics and await expected-error reporting for their JSON AbortError paths before the exact JSON output and direct exit.
  • store delete records the parsed store FQDN as unvalidated; the existing accessible-shop lookup upgrades it to validated with the shop ID when available.
  • store create dev records the returned domain as validated before polling.
  • Interactive delete decline stays unreported.

How to test your changes?

No behavior or output change is intended. Run store create dev --json or store delete --json with 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 --json expected failure and disabled analytics delivery. Both returned byte-identical JSON and exit code 1. Only the PR head built the awaited expected_error payload before exit, including the unvalidated store domain and matching hash. No analytics event was sent and no real store was created or deleted. The store create dev error path was verified through focused command tests because a live create would mutate state.

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
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

No changeset: internal telemetry only, no user-visible behavior or output change.

Assisted-By: devx/9c08eef3-3e18-4b5f-8ed8-bef380040c5d
@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 Aug 24, 2026
@dmerand
dmerand marked this pull request as ready for review August 25, 2026 13:34
@dmerand
dmerand requested a review from a team as a code owner August 25, 2026 13:34
Copilot AI lite review requested due to automatic review settings August 25, 2026 13:34
@dmerand
dmerand marked this pull request as draft August 25, 2026 13:34

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

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 + AbortError paths 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 for store 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.

Comment thread packages/store/src/cli/commands/store/delete.test.ts Outdated
Comment thread packages/store/src/cli/commands/store/create/dev.test.ts
Assisted-By: devx/65d0e15e-19bf-4af6-8cb3-53772dd9d107
@dmerand
dmerand marked this pull request as ready for review August 25, 2026 13:57
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