diff --git a/.changeset/app-context-telemetry-everywhere.md b/.changeset/app-context-telemetry-everywhere.md new file mode 100644 index 00000000000..55450bf8e9e --- /dev/null +++ b/.changeset/app-context-telemetry-everywhere.md @@ -0,0 +1,10 @@ +--- +'@shopify/cli-kit': patch +'@shopify/app': patch +--- + +Command analytics now capture app context (`api_key`, `project_type`, and the `app_*` / `cmd_app_*` fields) for **every** command run inside an app project — not just commands that load an app. When the user is already authenticated and the working directory is at or below an app root, the public metadata hook reads the app from disk (no network, no prompts) and attaches those fields, so the command is attributed to that app just as a `shopify app …` command would be. + +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/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index 95fe5c3ea0b..1b6b89fb1eb 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,80 @@ 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 load adds its own metadata too, so find the call carrying api_key. + 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 loaded the app and set api_key + 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..c29d1edf7d2 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -18,6 +18,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 +62,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 +191,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 +208,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 `directory` is inside an app project, this loads the app from disk and attaches + * `api_key` to the public Monorail metadata — the load also drives the app loader, which + * emits the same `project_type` / `app_*` / `cmd_app_linked_config_*` block that running a + * `shopify app …` command would. The effect: any command run within an app path is + * attributed to that app, not just app commands, for authenticated users. + * + * 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 (an app command already loaded the app), + * - 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 () => { + // localAppContext drives the app loader, which populates project_type and the app_* + // block. It does not emit api_key (that comes from the remote app), so add it here. + const {app} = await localAppContext({directory, skipPrompts: true}) + const clientId = app.configuration.client_id + if (typeof clientId === 'string' && clientId.length > 0) { + await metadata.addPublicMetadata(() => ({api_key: clientId})) + } + })() + + 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. *