From ffcd9421f71c93f8124558282eaa0c2d35596715 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Tue, 9 Jun 2026 15:27:12 -0400 Subject: [PATCH 1/2] Capture app context (api_key/project_type) in analytics for every command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Today only commands that load an app (app dev/deploy/...) populate api_key and project_type in Monorail; lightweight commands (search, fetch-doc, version, ...) emit nothing, so there's no way to know what app context they ran in. Enrich the global public_command_metadata hook so that — for users who are ALREADY authenticated and inside an app project — it reads the app from disk and attaches api_key + project_type. It is strictly best-effort: - gated on an existing local session; never triggers a login or any network call - short-circuits when api_key is already set (an app command already loaded the app) - threads skipPrompts through localAppContext so it can never prompt - bounded by a 3s timeout so it can't delay command exit - swallows all errors (not an app dir, invalid config, etc.) Adds a passive, non-interactive `sessionExists()` to @shopify/cli-kit/node/session. No Monorail schema change: api_key/project_type already exist on the event. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app-context-telemetry-everywhere.md | 8 ++ .../app/src/cli/hooks/public_metadata.test.ts | 38 +++++++++ packages/app/src/cli/hooks/public_metadata.ts | 6 ++ packages/app/src/cli/models/app/loader.ts | 2 +- .../app/src/cli/services/app-context.test.ts | 82 ++++++++++++++++++- packages/app/src/cli/services/app-context.ts | 60 +++++++++++++- .../cli-kit/src/public/node/session.test.ts | 23 ++++++ packages/cli-kit/src/public/node/session.ts | 20 +++++ 8 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 .changeset/app-context-telemetry-everywhere.md create mode 100644 packages/app/src/cli/hooks/public_metadata.test.ts diff --git a/.changeset/app-context-telemetry-everywhere.md b/.changeset/app-context-telemetry-everywhere.md new file mode 100644 index 00000000000..87c38b898a1 --- /dev/null +++ b/.changeset/app-context-telemetry-everywhere.md @@ -0,0 +1,8 @@ +--- +'@shopify/cli-kit': patch +'@shopify/app': patch +--- + +Command analytics now opportunistically capture app context (`api_key` and `project_type`) for **every** command — not just commands that load an app. When the user is already authenticated and runs a command inside an app project, the public metadata hook reads the app from disk (no network, no prompts) and attaches those fields. It does nothing for users who aren't logged in, never triggers a login, and is bounded by a short timeout so it can't delay command exit. + +Adds `sessionExists()` to `@shopify/cli-kit/node/session`: a passive, non-interactive check for whether local credentials exist (no prompt, no network). diff --git a/packages/app/src/cli/hooks/public_metadata.test.ts b/packages/app/src/cli/hooks/public_metadata.test.ts new file mode 100644 index 00000000000..26d11e7c7eb --- /dev/null +++ b/packages/app/src/cli/hooks/public_metadata.test.ts @@ -0,0 +1,38 @@ +import gatherPublicMetadata from './public_metadata.js' +import {logAppContextMetadataIfAuthenticated} from '../services/app-context.js' +import metadata from '../metadata.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' +import {cwd} from '@shopify/cli-kit/node/path' + +vi.mock('../services/app-context.js') +vi.mock('@shopify/cli-kit/node/path') + +describe('gatherPublicMetadata', () => { + beforeEach(() => { + vi.mocked(cwd).mockReturnValue('/some/app/dir') + }) + + test('opportunistically enriches metadata from the current directory and returns the public metadata', async () => { + // Given + await metadata.addPublicMetadata(() => ({api_key: 'from-helper'})) + + // When + const result = await (gatherPublicMetadata as () => Promise)() + + // Then + expect(logAppContextMetadataIfAuthenticated).toHaveBeenCalledWith('/some/app/dir') + expect(result).toEqual(metadata.getAllPublicMetadata()) + }) + + test('still returns metadata when the best-effort enrichment is a no-op', async () => { + // Given the helper does nothing (e.g. not authenticated) + vi.mocked(logAppContextMetadataIfAuthenticated).mockResolvedValue() + + // When + const result = await (gatherPublicMetadata as () => Promise)() + + // Then + expect(logAppContextMetadataIfAuthenticated).toHaveBeenCalledOnce() + expect(result).toBeTypeOf('object') + }) +}) diff --git a/packages/app/src/cli/hooks/public_metadata.ts b/packages/app/src/cli/hooks/public_metadata.ts index 5448a5fb40b..02245a55516 100644 --- a/packages/app/src/cli/hooks/public_metadata.ts +++ b/packages/app/src/cli/hooks/public_metadata.ts @@ -1,7 +1,13 @@ import metadata from '../metadata.js' +import {logAppContextMetadataIfAuthenticated} from '../services/app-context.js' import {FanoutHookFunction} from '@shopify/cli-kit/node/plugins' +import {cwd} from '@shopify/cli-kit/node/path' const gatherPublicMetadata: FanoutHookFunction<'public_command_metadata', '@shopify/app'> = async () => { + // Runs for every CLI command. Best-effort: if the user is logged in and is inside an + // app project, attach api_key/project_type so commands that don't otherwise load an + // app (search, fetch-doc, version, …) still report the app context they ran in. + await logAppContextMetadataIfAuthenticated(cwd()) return metadata.getAllPublicMetadata() } diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 206aa1d9471..44994542578 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -969,7 +969,7 @@ function getAllLinkedConfigClientIds( return Object.fromEntries(entries) } -async function getProjectType(webs: Web[]): Promise<'node' | 'php' | 'ruby' | 'frontend' | undefined> { +export async function getProjectType(webs: Web[]): Promise<'node' | 'php' | 'ruby' | 'frontend' | undefined> { const backendWebs = webs.filter((web) => isWebType(web, WebType.Backend)) const frontendWebs = webs.filter((web) => isWebType(web, WebType.Frontend)) if (backendWebs.length > 1) { diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index 95fe5c3ea0b..db31af8f921 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -1,4 +1,4 @@ -import {linkedAppContext, localAppContext} from './app-context.js' +import {linkedAppContext, localAppContext, logAppContextMetadataIfAuthenticated} from './app-context.js' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' import {addUidToTomlsIfNecessary} from './app/add-uid-to-extension-toml.js' import link from './app/config/link.js' @@ -14,6 +14,7 @@ import {beforeEach, describe, expect, test, vi} from 'vitest' import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs' import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' import {tryParseInt} from '@shopify/cli-kit/common/string' +import {sessionExists} from '@shopify/cli-kit/node/session' vi.mock('../models/app/validation/multi-cli-warning.js') vi.mock('./generate/fetch-extension-specifications.js') @@ -22,6 +23,7 @@ vi.mock('./context.js') vi.mock('./dev/fetch.js') vi.mock('./app/add-uid-to-extension-toml.js') vi.mock('../models/extensions/load-specifications.js') +vi.mock('@shopify/cli-kit/node/session') async function writeAppConfig(tmp: string, content: string, configName?: string) { const appConfigPath = joinPath(tmp, configName ?? 'shopify.app.toml') @@ -500,3 +502,81 @@ describe('localAppContext', () => { }) }) }) + +describe('logAppContextMetadataIfAuthenticated', () => { + const linkedAppToml = ` + name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" + ` + + beforeEach(() => { + vi.mocked(loadLocalExtensionsSpecifications).mockResolvedValue([]) + vi.mocked(sessionExists).mockResolvedValue(true) + }) + + test('attaches api_key for an authenticated user inside an app project', async () => { + await inTemporaryDirectory(async (tmp) => { + // Given + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) + const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + await writeAppConfig(tmp, linkedAppToml) + + // When + await logAppContextMetadataIfAuthenticated(tmp) + + // Then — the local app load emits its own metadata too, so find the call + // carrying the api_key (only this helper emits it in the local-load path). + const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) + expect(payloads).toContainEqual(expect.objectContaining({api_key: 'test-client-id'})) + }) + }) + + test('does nothing when the user is not authenticated', async () => { + await inTemporaryDirectory(async (tmp) => { + // Given + vi.mocked(sessionExists).mockResolvedValue(false) + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) + const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + await writeAppConfig(tmp, linkedAppToml) + + // When + await logAppContextMetadataIfAuthenticated(tmp) + + // Then + expect(addSpy).not.toHaveBeenCalled() + }) + }) + + test('short-circuits without checking the session when api_key is already set', async () => { + // Given — a command like `app dev` already populated identity + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({api_key: 'already-set'}) + const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + + // When + await logAppContextMetadataIfAuthenticated('/does/not/matter') + + // Then + expect(sessionExists).not.toHaveBeenCalled() + expect(addSpy).not.toHaveBeenCalled() + }) + + test('never throws and adds nothing when the directory is not an app project', async () => { + await inTemporaryDirectory(async (tmp) => { + // Given — no shopify.app.toml on disk + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) + const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + + // When / Then + await expect(logAppContextMetadataIfAuthenticated(tmp)).resolves.toBeUndefined() + expect(addSpy).not.toHaveBeenCalled() + }) + }) +}) diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 8c980f1f309..5d351aab525 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -9,6 +9,7 @@ import {Organization, OrganizationApp, OrganizationSource} from '../models/organ import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' import { getAppConfigurationContext, + getProjectType, loadAppFromContext, formatConfigurationError, type ConfigurationError, @@ -18,6 +19,7 @@ import {AppLinkedInterface, AppInterface} from '../models/app/app.js' import {Project} from '../models/project/project.js' import metadata from '../metadata.js' import {tryParseInt} from '@shopify/cli-kit/common/string' +import {sessionExists} from '@shopify/cli-kit/node/session' import {AbortError, BugError} from '@shopify/cli-kit/node/error' import {outputContent, outputToken} from '@shopify/cli-kit/node/output' import {basename} from '@shopify/cli-kit/node/path' @@ -61,10 +63,12 @@ interface LoadedAppContextOptions { * * @param directory - The directory containing the app. * @param userProvidedConfigName - The name of an existing config file in the app, if not provided, the cached/default one will be used. + * @param skipPrompts - When true, never prompts the user (e.g. to re-select a config). Required for non-interactive callers such as telemetry. */ interface LocalAppContextOptions { directory: string userProvidedConfigName?: string + skipPrompts?: boolean } /** @@ -188,8 +192,9 @@ interface LocalAppContextOutput { export async function localAppContext({ directory, userProvidedConfigName, + skipPrompts = false, }: LocalAppContextOptions): Promise { - const {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName) + const {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName, {skipPrompts}) if (activeConfig.file.errors.length > 0) { throw new AbortError(activeConfig.file.errors.map((err) => err.message).join('\n')) @@ -204,3 +209,56 @@ export async function localAppContext({ return {app, project} } + +// Upper bound on the best-effort app-context load below. It runs on the postrun of +// every command, so it must never delay process exit, even on a pathological project. +const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000 + +/** + * Best-effort, non-interactive enrichment of command analytics with app context. + * + * When the user is already authenticated and `directory` is an app project, this reads + * the app from disk (no network, no prompts) and attaches `api_key` and `project_type` + * to the public Monorail metadata. It is designed to run on the postrun of every CLI + * command, so it: + * - does nothing unless the user is already logged in (so we never enrich anonymous usage), + * - short-circuits when `api_key` is already set (a command like `app dev` already loaded + * the app — no need to redo the work), + * - never prompts, never makes a network request, + * - is bounded by a short timeout so it can't delay command exit, and + * - swallows all errors (a directory that isn't an app, an invalid config, etc.). + * + * @param directory - The working directory to inspect for an app. + */ +export async function logAppContextMetadataIfAuthenticated(directory: string): Promise { + try { + // A command that loads the app (e.g. `app dev`) already populated identity. + if (metadata.getAllPublicMetadata().api_key !== undefined) return + // Only enrich for users who are already authenticated; never trigger a login. + if (!(await sessionExists())) return + + let timer: ReturnType | undefined + const deadline = new Promise((resolve) => { + timer = setTimeout(resolve, APP_CONTEXT_METADATA_TIMEOUT_MS) + }) + + const load = (async () => { + const {app} = await localAppContext({directory, skipPrompts: true}) + const clientId = app.configuration.client_id + const projectType = await getProjectType(app.webs) + await metadata.addPublicMetadata(() => ({ + ...(typeof clientId === 'string' && clientId.length > 0 ? {api_key: clientId} : {}), + ...(projectType ? {project_type: projectType} : {}), + })) + })() + + try { + await Promise.race([load, deadline]) + } finally { + if (timer) clearTimeout(timer) + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // Telemetry is strictly best-effort: never surface errors or affect the command. + } +} diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 810f9acc1c1..8f232729972 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/packages/cli-kit/src/public/node/session.test.ts @@ -6,11 +6,13 @@ import { ensureAuthenticatedPartners, ensureAuthenticatedStorefront, ensureAuthenticatedThemes, + sessionExists, setLastSeenUserId, } from './session.js' import {getAppAutomationToken} from './environment.js' import {shopifyFetch} from './http.js' +import * as sessionStore from '../../private/node/session/store.js' import {ensureAuthenticated, setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../../private/node/session.js' import {ApplicationToken} from '../../private/node/session/schema.js' import { @@ -31,9 +33,30 @@ const partnersToken: ApplicationToken = { vi.mock('../../private/node/session.js') vi.mock('../../private/node/session/exchange.js') +vi.mock('../../private/node/session/store.js') vi.mock('./environment.js') vi.mock('./http.js') +describe('sessionExists', () => { + test('returns true when the local session store has at least one session', async () => { + vi.mocked(sessionStore.fetch).mockResolvedValue({'accounts.shopify.com': {}} as any) + + await expect(sessionExists()).resolves.toBe(true) + }) + + test('returns false when the local session store is empty', async () => { + vi.mocked(sessionStore.fetch).mockResolvedValue({} as any) + + await expect(sessionExists()).resolves.toBe(false) + }) + + test('returns false when there is no stored session', async () => { + vi.mocked(sessionStore.fetch).mockResolvedValue(undefined) + + await expect(sessionExists()).resolves.toBe(false) + }) +}) + describe('store command analytics session helpers', () => { test('sets last seen user id through the public session helper', () => { setLastSeenUserId('store-user-id') diff --git a/packages/cli-kit/src/public/node/session.ts b/packages/cli-kit/src/public/node/session.ts index 1ebffdeaf47..0bddf5bcebe 100644 --- a/packages/cli-kit/src/public/node/session.ts +++ b/packages/cli-kit/src/public/node/session.ts @@ -85,6 +85,26 @@ export function isServiceAccount(account: AccountInfo): account is ServiceAccoun return account.type === 'ServiceAccount' } +/** + * Reports whether the CLI already has stored credentials, without prompting the + * user, opening a browser, or making any network request. + * + * This is a passive, side-effect-free check: it reads the local session store and + * returns `true` when at least one valid session is present. Unlike the + * `ensureAuthenticated*` functions, it never triggers a login flow and never logs + * the user out. Because it does not contact the network, it cannot tell whether the + * stored token is currently valid/unexpired — only that credentials exist locally. + * + * Intended for best-effort, opportunistic behaviour (for example, enriching + * telemetry only for users who are already logged in). + * + * @returns True if local credentials exist, false otherwise. + */ +export async function sessionExists(): Promise { + const sessions = await sessionStore.fetch() + return sessions !== undefined && Object.keys(sessions).length > 0 +} + /** * Ensure that we have a valid session with no particular scopes. * From 698c6349c722e97e19c877f63578870ce6639246 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Tue, 9 Jun 2026 15:42:09 -0400 Subject: [PATCH 2/2] Address review: drop redundant comments Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/app/src/cli/hooks/public_metadata.ts | 3 --- packages/app/src/cli/services/app-context.ts | 2 -- 2 files changed, 5 deletions(-) diff --git a/packages/app/src/cli/hooks/public_metadata.ts b/packages/app/src/cli/hooks/public_metadata.ts index 02245a55516..8eeed67954d 100644 --- a/packages/app/src/cli/hooks/public_metadata.ts +++ b/packages/app/src/cli/hooks/public_metadata.ts @@ -4,9 +4,6 @@ import {FanoutHookFunction} from '@shopify/cli-kit/node/plugins' import {cwd} from '@shopify/cli-kit/node/path' const gatherPublicMetadata: FanoutHookFunction<'public_command_metadata', '@shopify/app'> = async () => { - // Runs for every CLI command. Best-effort: if the user is logged in and is inside an - // app project, attach api_key/project_type so commands that don't otherwise load an - // app (search, fetch-doc, version, …) still report the app context they ran in. await logAppContextMetadataIfAuthenticated(cwd()) return metadata.getAllPublicMetadata() } diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 5d351aab525..6e8c16b5da2 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -232,9 +232,7 @@ const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000 */ export async function logAppContextMetadataIfAuthenticated(directory: string): Promise { try { - // A command that loads the app (e.g. `app dev`) already populated identity. if (metadata.getAllPublicMetadata().api_key !== undefined) return - // Only enrich for users who are already authenticated; never trigger a login. if (!(await sessionExists())) return let timer: ReturnType | undefined