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..8eeed67954d 100644 --- a/packages/app/src/cli/hooks/public_metadata.ts +++ b/packages/app/src/cli/hooks/public_metadata.ts @@ -1,7 +1,10 @@ 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 () => { + 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..6e8c16b5da2 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,54 @@ 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 { + if (metadata.getAllPublicMetadata().api_key !== undefined) return + 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. *