From ffcd9421f71c93f8124558282eaa0c2d35596715 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Tue, 9 Jun 2026 15:27:12 -0400 Subject: [PATCH 1/4] 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/4] 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 From 578082a0c9ea308495447dd7081f63de1f631fd0 Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Mon, 15 Jun 2026 09:13:37 -0400 Subject: [PATCH 3/4] Replay full app context for commands run outside an app directory The public_command_metadata hook now captures the complete app-context analytics block (api_key, project_type, app_*, cmd_app_linked_config_*, cmd_app_all_configs_*, and the sensitive app_name) when an app resolves, persists it as the most-recently-used app, and replays the whole block for commands run outside any app directory. A new public field, cmd_app_context_source, records how the context was resolved: "current_directory" for a real on-disk load, "last_used" when it was replayed from the most-recently-used app. Note: the Monorail backend schema registry must register this field for it to be retained/queryable. The snapshot keeps app identity/shape fields and excludes per-run flags (e.g. cmd_app_warning_*). Enrichment remains best-effort: authenticated users only, no prompts, no network, bounded by a short timeout, all errors swallowed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app-context-telemetry-everywhere.md | 4 +- .../app/src/cli/services/app-context.test.ts | 94 ++++++++++++-- packages/app/src/cli/services/app-context.ts | 121 +++++++++++++++--- .../src/cli/services/local-storage.test.ts | 53 ++++++++ .../app/src/cli/services/local-storage.ts | 39 ++++++ packages/cli-kit/src/public/node/monorail.ts | 5 + 6 files changed, 286 insertions(+), 30 deletions(-) diff --git a/.changeset/app-context-telemetry-everywhere.md b/.changeset/app-context-telemetry-everywhere.md index 87c38b898a1..6bd6fe4aff1 100644 --- a/.changeset/app-context-telemetry-everywhere.md +++ b/.changeset/app-context-telemetry-everywhere.md @@ -3,6 +3,8 @@ '@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. +Command analytics now opportunistically capture app context (`api_key`, `project_type`, and the `app_*` / `cmd_app_*` fields) 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. Commands run **outside** any app directory replay the full context of the most-recently-used app, so app context is still captured even when the working directory isn't an app root (e.g. running from a parent folder). + +A new `cmd_app_context_source` field records how the context was resolved: `"current_directory"` when loaded from the command's working directory, or `"last_used"` when replayed from the most-recently-used app. 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/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index db31af8f921..57764468481 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -517,25 +517,53 @@ describe('logAppContextMetadataIfAuthenticated', () => { api_version = "2024-01" ` + let getMruSpy: ReturnType + let setMruSpy: ReturnType + beforeEach(() => { vi.mocked(loadLocalExtensionsSpecifications).mockResolvedValue([]) vi.mocked(sessionExists).mockResolvedValue(true) + // Stub the MRU store so tests never touch the real on-disk LocalStorage. + getMruSpy = vi.spyOn(localStorage, 'getMostRecentlyUsedAppContext').mockReturnValue(undefined) + setMruSpy = vi.spyOn(localStorage, 'setMostRecentlyUsedAppContext').mockImplementation(() => {}) }) - test('attaches api_key for an authenticated user inside an app project', async () => { + test('tags current_directory and records the full snapshot when loading from the app directory', async () => { await inTemporaryDirectory(async (tmp) => { - // Given - vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) + // Given — the loader (driven by localAppContext) populates the app_* block; we model + // its end state. The first read (the identity short-circuit) must see no api_key. + const loadedPublic = { + api_key: 'test-client-id', + project_type: 'node', + app_scopes: '["read_products"]', + cmd_app_linked_config_name: 'shopify.app.toml', + cmd_app_warning_api_key_deprecation_displayed: false, + } + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue(loadedPublic) + vi.spyOn(metadata, 'getAllSensitiveMetadata').mockReturnValue({app_name: 'test-app'}) 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). + // Then — emits api_key tagged as a real directory load (the local load adds its own + // metadata too, so find the call carrying api_key + the source tag). const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) - expect(payloads).toContainEqual(expect.objectContaining({api_key: 'test-client-id'})) + expect(payloads).toContainEqual( + expect.objectContaining({api_key: 'test-client-id', cmd_app_context_source: 'current_directory'}), + ) + // And snapshots the full app-context block — keeping app identity/shape fields, + // dropping per-run flags like cmd_app_warning_*, plus the sensitive app_name. + expect(setMruSpy).toHaveBeenCalledWith({ + public: { + api_key: 'test-client-id', + project_type: 'node', + app_scopes: '["read_products"]', + cmd_app_linked_config_name: 'shopify.app.toml', + }, + sensitive: {app_name: 'test-app'}, + }) }) }) @@ -555,24 +583,64 @@ describe('logAppContextMetadataIfAuthenticated', () => { }) }) - 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'}) + test('records the snapshot and short-circuits without checking the session when api_key is already set', async () => { + // Given — a command like `app dev` already populated the full app context + const loadedPublic = { + api_key: 'already-set', + project_type: 'node', + app_scopes: '["x"]', + cmd_app_warning_api_key_deprecation_displayed: false, + } + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue(loadedPublic) + vi.spyOn(metadata, 'getAllSensitiveMetadata').mockReturnValue({app_name: 'already-app'}) const addSpy = vi.spyOn(metadata, 'addPublicMetadata') // When await logAppContextMetadataIfAuthenticated('/does/not/matter') - // Then + // Then — only tags the source, does no load, and records the full snapshot. + const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) + expect(payloads).toEqual([{cmd_app_context_source: 'current_directory'}]) expect(sessionExists).not.toHaveBeenCalled() - expect(addSpy).not.toHaveBeenCalled() + expect(setMruSpy).toHaveBeenCalledWith({ + public: {api_key: 'already-set', project_type: 'node', app_scopes: '["x"]'}, + sensitive: {app_name: 'already-app'}, + }) + }) + + test('replays the full most-recently-used context tagged last_used when the directory is not an app project', async () => { + await inTemporaryDirectory(async (tmp) => { + // Given — no shopify.app.toml on disk, but we have a remembered app + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) + const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + const addSensitiveSpy = vi.spyOn(metadata, 'addSensitiveMetadata') + getMruSpy.mockReturnValue({ + public: {api_key: 'mru-key', project_type: 'node', app_scopes: '["read_products"]'}, + sensitive: {app_name: 'mru-app'}, + }) + + // When + await logAppContextMetadataIfAuthenticated(tmp) + + // Then — replays the whole public block tagged as deduced, plus the sensitive app_name. + const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) + expect(payloads).toContainEqual({ + api_key: 'mru-key', + project_type: 'node', + app_scopes: '["read_products"]', + cmd_app_context_source: 'last_used', + }) + const sensitivePayloads = await Promise.all(addSensitiveSpy.mock.calls.map((call) => call[0]())) + expect(sensitivePayloads).toContainEqual({app_name: 'mru-app'}) + }) }) - test('never throws and adds nothing when the directory is not an app project', async () => { + test('never throws and adds nothing when the directory is not an app project and there is no MRU', async () => { await inTemporaryDirectory(async (tmp) => { - // Given — no shopify.app.toml on disk + // Given — no shopify.app.toml on disk and no remembered app vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) const addSpy = vi.spyOn(metadata, 'addPublicMetadata') + getMruSpy.mockReturnValue(undefined) // When / Then await expect(logAppContextMetadataIfAuthenticated(tmp)).resolves.toBeUndefined() diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 6e8c16b5da2..793e074b529 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -1,5 +1,10 @@ import {appFromIdentifiers} from './context.js' -import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' +import { + getCachedAppInfo, + setCachedAppInfo, + getMostRecentlyUsedAppContext, + setMostRecentlyUsedAppContext, +} from './local-storage.js' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' import link from './app/config/link.js' import {fetchOrgFromId} from './dev/fetch.js' @@ -9,7 +14,6 @@ import {Organization, OrganizationApp, OrganizationSource} from '../models/organ import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' import { getAppConfigurationContext, - getProjectType, loadAppFromContext, formatConfigurationError, type ConfigurationError, @@ -214,16 +218,75 @@ export async function localAppContext({ // every command, so it must never delay process exit, even on a pathological project. const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000 +type AppMetadataPublic = ReturnType +type AppMetadataSensitive = ReturnType + +// Sensitive metadata fields that are part of "app context" and worth replaying for +// out-of-app commands. Kept tiny and explicit so we never persist more than intended. +const APP_CONTEXT_SENSITIVE_KEYS = ['app_name'] + +/** + * Whether a public metadata key is part of the app-context block emitted when an app loads + * (see `logMetadataForLoadedAppUsingRawValues` in the loader). These are the fields we + * snapshot and replay so a command run outside an app directory looks like one run inside. + * Deliberately excludes per-run `cmd_app_*` flags (e.g. `cmd_app_warning_*`, + * `cmd_app_reset_used`) that describe the command rather than the app's identity/shape. + */ +function isAppContextPublicKey(key: string): boolean { + return ( + key === 'api_key' || + key === 'project_type' || + key.startsWith('app_') || + key.startsWith('cmd_app_all_configs_') || + key.startsWith('cmd_app_linked_config_') + ) +} + +/** + * Snapshots the app-context fields currently in the metadata container and records them as + * the most-recently-used app, so later commands run outside an app directory can replay + * them. No-op unless a non-empty `api_key` is present (i.e. an app actually resolved). + */ +function recordMostRecentlyUsedAppContext(): void { + const allPublic = metadata.getAllPublicMetadata() + if (typeof allPublic.api_key !== 'string' || allPublic.api_key.length === 0) return + + const publicContext: {[key: string]: unknown} = {} + for (const [key, value] of Object.entries(allPublic)) { + if (value !== undefined && isAppContextPublicKey(key)) publicContext[key] = value + } + + const allSensitive = metadata.getAllSensitiveMetadata() as {[key: string]: unknown} + const sensitiveContext: {[key: string]: unknown} = {} + for (const key of APP_CONTEXT_SENSITIVE_KEYS) { + if (allSensitive[key] !== undefined) sensitiveContext[key] = allSensitive[key] + } + + setMostRecentlyUsedAppContext({public: publicContext, sensitive: sensitiveContext}) +} + /** * 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: + * Attaches the full app-context block (`api_key`, `project_type`, and the `app_*` / + * `cmd_app_*` fields the loader emits) to the Monorail metadata so that every command — not + * just app commands — is attributed to an app for authenticated users. It resolves the app + * from one of two sources, recorded in the `cmd_app_context_source` field: + * 1. `"current_directory"` — `directory` is an app project, loaded from disk (no network, + * no prompts). This is the authoritative source; its full context is also snapshotted + * as the "most recently used" app for later. + * 2. `"last_used"` — `directory` is not an app project, so the previously snapshotted + * most-recently-used app context is replayed in full (public fields plus the sensitive + * `app_name`). This lets commands run *outside* any app directory (e.g. from a parent + * folder) still be attributed to the app the user was last working with. + * + * `cmd_app_context_source` is the explicit discriminator between the two; the absence of the + * loader-only `cmd_app_linked_config_*` fields on a `"last_used"` event is a secondary tell. + * + * 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), + * - when `api_key` is already set (a command like `app dev` already loaded the app), tags + * the source and snapshots that app as most-recently-used, but does no further 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.). @@ -232,7 +295,14 @@ const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000 */ export async function logAppContextMetadataIfAuthenticated(directory: string): Promise { try { - if (metadata.getAllPublicMetadata().api_key !== undefined) return + const existingApiKey = metadata.getAllPublicMetadata().api_key + if (typeof existingApiKey === 'string' && existingApiKey.length > 0) { + // A command (e.g. `app dev`) already resolved and emitted the full app context. Tag it + // as a real load and remember it so later out-of-app commands can replay it. + await metadata.addPublicMetadata(() => ({cmd_app_context_source: 'current_directory'})) + recordMostRecentlyUsedAppContext() + return + } if (!(await sessionExists())) return let timer: ReturnType | undefined @@ -241,13 +311,32 @@ export async function logAppContextMetadataIfAuthenticated(directory: string): P }) 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 { + const {app} = await localAppContext({directory, skipPrompts: true}) + const clientId = app.configuration.client_id + if (typeof clientId === 'string' && clientId.length > 0) { + // localAppContext drove the loader, which already populated project_type and the + // app_* block. Add api_key + the source tag, then snapshot the whole thing. + await metadata.addPublicMetadata(() => ({api_key: clientId, cmd_app_context_source: 'current_directory'})) + recordMostRecentlyUsedAppContext() + return + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // `directory` isn't an app project (or it's unreadable) — fall back to the most + // recently used app below. + } + + const recent = getMostRecentlyUsedAppContext() + const recentApiKey = recent?.public?.api_key + if (typeof recentApiKey === 'string' && recentApiKey.length > 0) { + await metadata.addPublicMetadata( + () => ({...recent!.public, cmd_app_context_source: 'last_used'}) as unknown as AppMetadataPublic, + ) + if (recent!.sensitive && Object.keys(recent!.sensitive).length > 0) { + await metadata.addSensitiveMetadata(() => ({...recent!.sensitive}) as unknown as AppMetadataSensitive) + } + } })() try { diff --git a/packages/app/src/cli/services/local-storage.test.ts b/packages/app/src/cli/services/local-storage.test.ts index a6c161d77f6..16b7055a0c1 100644 --- a/packages/app/src/cli/services/local-storage.test.ts +++ b/packages/app/src/cli/services/local-storage.test.ts @@ -1,9 +1,12 @@ import { + AppContextLocalStorageSchema, AppLocalStorageSchema, clearCachedAppInfo, clearCurrentConfigFile, getCachedAppInfo, + getMostRecentlyUsedAppContext, setCachedAppInfo, + setMostRecentlyUsedAppContext, } from './local-storage.js' import {describe, expect, test} from 'vitest' import {LocalStorage} from '@shopify/cli-kit/node/local-storage' @@ -140,3 +143,53 @@ describe('clearCurrentConfigFile', async () => { }) }) }) + +describe('mostRecentlyUsedAppContext', async () => { + const snapshot = { + public: {api_key: 'mru-client-id', project_type: 'node', app_scopes: '["read_products"]'}, + sensitive: {app_name: 'my-app'}, + } + + test('returns undefined before anything is stored', async () => { + await inTemporaryDirectory(async (cwd) => { + // Given + const storage = new LocalStorage({cwd}) + + // When + const got = getMostRecentlyUsedAppContext(storage) + + // Then + expect(got).toBeUndefined() + }) + }) + + test('round-trips the full app context snapshot', async () => { + await inTemporaryDirectory(async (cwd) => { + // Given + const storage = new LocalStorage({cwd}) + + // When + setMostRecentlyUsedAppContext(snapshot, storage) + const got = getMostRecentlyUsedAppContext(storage) + + // Then + expect(got).toEqual(snapshot) + }) + }) + + test('overwrites the previously stored app context', async () => { + await inTemporaryDirectory(async (cwd) => { + // Given + const storage = new LocalStorage({cwd}) + setMostRecentlyUsedAppContext(snapshot, storage) + + // When + const next = {public: {api_key: 'new-client-id'}, sensitive: {}} + setMostRecentlyUsedAppContext(next, storage) + const got = getMostRecentlyUsedAppContext(storage) + + // Then + expect(got).toEqual(next) + }) + }) +}) diff --git a/packages/app/src/cli/services/local-storage.ts b/packages/app/src/cli/services/local-storage.ts index bb0f6f4aedf..63a8247b3e3 100644 --- a/packages/app/src/cli/services/local-storage.ts +++ b/packages/app/src/cli/services/local-storage.ts @@ -128,3 +128,42 @@ export function setCachedCommandTomlMap(tomls: {[clientId: string]: AppConfigura export function setCachedCommandTomlPreference(selectedToml: AppConfigurationFileName) { setCachedCommandInfo({selectedToml}) } + +// A snapshot of the app-context analytics metadata (api_key, project_type, the app_* and +// cmd_app_* fields, and the sensitive app_name) for the last app the CLI resolved. Unlike +// `getCachedAppInfo`, which is keyed by directory, this is a single global value so that +// commands run *outside* any app directory can still be attributed to the app the user was +// most recently working with — the whole snapshot is replayed onto the event. Used only for +// best-effort telemetry enrichment. +export interface MostRecentlyUsedAppContext { + // Public metadata fields, keyed by their Monorail field name (e.g. `api_key`, `app_scopes`). + public?: {[key: string]: unknown} + // Sensitive metadata fields, keyed by their Monorail field name (currently just `app_name`). + sensitive?: {[key: string]: unknown} +} + +export interface AppContextLocalStorageSchema { + mostRecentlyUsedApp?: MostRecentlyUsedAppContext +} + +let _appContextLocalStorageInstance: LocalStorage | undefined + +function appContextLocalStorage() { + _appContextLocalStorageInstance ??= new LocalStorage({ + projectName: 'shopify-cli-app-context', + }) + return _appContextLocalStorageInstance +} + +export function getMostRecentlyUsedAppContext( + config: LocalStorage = appContextLocalStorage(), +): MostRecentlyUsedAppContext | undefined { + return config.get('mostRecentlyUsedApp') +} + +export function setMostRecentlyUsedAppContext( + context: MostRecentlyUsedAppContext, + config: LocalStorage = appContextLocalStorage(), +): void { + config.set('mostRecentlyUsedApp', context) +} diff --git a/packages/cli-kit/src/public/node/monorail.ts b/packages/cli-kit/src/public/node/monorail.ts index bb8d1e9dcd2..c3a14838a91 100644 --- a/packages/cli-kit/src/public/node/monorail.ts +++ b/packages/cli-kit/src/public/node/monorail.ts @@ -95,6 +95,11 @@ export interface Schemas { cmd_app_linked_config_source?: Optional cmd_app_linked_config_uses_cli_managed_urls?: Optional cmd_app_warning_api_key_deprecation_displayed?: Optional + // How the app context (api_key, project_type, app_*, …) on this event was resolved: + // "current_directory" when the app was loaded from the command's working directory, + // "last_used" when it was replayed from the most-recently-used app because the command + // ran outside any app directory. + cmd_app_context_source?: Optional cmd_app_deployment_mode?: Optional cmd_app_validate_json?: Optional cmd_app_validate_valid?: Optional From 7631e02a4cad5229f60896621263e7d52b2d9d7e Mon Sep 17 00:00:00 2001 From: Nelson Wittwer Date: Mon, 15 Jun 2026 13:13:58 -0400 Subject: [PATCH 4/4] Drop MRU fallback: broadcast app context only when run inside an app Reverts the most-recently-used app replay. We no longer deduce an app for commands run outside an app directory. Instead we trust in-app context: any command run within an app path broadcasts the same app-context block a `shopify app *` command would (api_key, project_type, app_*, cmd_app_linked_config_*, and the sensitive app_name), driven by loading the app from the current directory in the public_command_metadata hook. Removes the dedicated MRU LocalStorage store, the cmd_app_context_source Monorail discriminator, and the snapshot/replay helpers. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app-context-telemetry-everywhere.md | 4 +- packages/app/src/cli/models/app/loader.ts | 2 +- .../app/src/cli/services/app-context.test.ts | 93 ++------------ packages/app/src/cli/services/app-context.ts | 116 +++--------------- .../src/cli/services/local-storage.test.ts | 53 -------- .../app/src/cli/services/local-storage.ts | 39 ------ packages/cli-kit/src/public/node/monorail.ts | 5 - 7 files changed, 29 insertions(+), 283 deletions(-) diff --git a/.changeset/app-context-telemetry-everywhere.md b/.changeset/app-context-telemetry-everywhere.md index 6bd6fe4aff1..55450bf8e9e 100644 --- a/.changeset/app-context-telemetry-everywhere.md +++ b/.changeset/app-context-telemetry-everywhere.md @@ -3,8 +3,8 @@ '@shopify/app': patch --- -Command analytics now opportunistically capture app context (`api_key`, `project_type`, and the `app_*` / `cmd_app_*` fields) 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. Commands run **outside** any app directory replay the full context of the most-recently-used app, so app context is still captured even when the working directory isn't an app root (e.g. running from a parent folder). +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. -A new `cmd_app_context_source` field records how the context was resolved: `"current_directory"` when loaded from the command's working directory, or `"last_used"` when replayed from the most-recently-used app. 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. +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/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 44994542578..206aa1d9471 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) } -export async function getProjectType(webs: Web[]): Promise<'node' | 'php' | 'ruby' | 'frontend' | undefined> { +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 57764468481..1b6b89fb1eb 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -517,53 +517,24 @@ describe('logAppContextMetadataIfAuthenticated', () => { api_version = "2024-01" ` - let getMruSpy: ReturnType - let setMruSpy: ReturnType - beforeEach(() => { vi.mocked(loadLocalExtensionsSpecifications).mockResolvedValue([]) vi.mocked(sessionExists).mockResolvedValue(true) - // Stub the MRU store so tests never touch the real on-disk LocalStorage. - getMruSpy = vi.spyOn(localStorage, 'getMostRecentlyUsedAppContext').mockReturnValue(undefined) - setMruSpy = vi.spyOn(localStorage, 'setMostRecentlyUsedAppContext').mockImplementation(() => {}) }) - test('tags current_directory and records the full snapshot when loading from the app directory', async () => { + test('attaches api_key for an authenticated user inside an app project', async () => { await inTemporaryDirectory(async (tmp) => { - // Given — the loader (driven by localAppContext) populates the app_* block; we model - // its end state. The first read (the identity short-circuit) must see no api_key. - const loadedPublic = { - api_key: 'test-client-id', - project_type: 'node', - app_scopes: '["read_products"]', - cmd_app_linked_config_name: 'shopify.app.toml', - cmd_app_warning_api_key_deprecation_displayed: false, - } - vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue(loadedPublic) - vi.spyOn(metadata, 'getAllSensitiveMetadata').mockReturnValue({app_name: 'test-app'}) + // Given + vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) const addSpy = vi.spyOn(metadata, 'addPublicMetadata') await writeAppConfig(tmp, linkedAppToml) // When await logAppContextMetadataIfAuthenticated(tmp) - // Then — emits api_key tagged as a real directory load (the local load adds its own - // metadata too, so find the call carrying api_key + the source tag). + // 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', cmd_app_context_source: 'current_directory'}), - ) - // And snapshots the full app-context block — keeping app identity/shape fields, - // dropping per-run flags like cmd_app_warning_*, plus the sensitive app_name. - expect(setMruSpy).toHaveBeenCalledWith({ - public: { - api_key: 'test-client-id', - project_type: 'node', - app_scopes: '["read_products"]', - cmd_app_linked_config_name: 'shopify.app.toml', - }, - sensitive: {app_name: 'test-app'}, - }) + expect(payloads).toContainEqual(expect.objectContaining({api_key: 'test-client-id'})) }) }) @@ -583,64 +554,24 @@ describe('logAppContextMetadataIfAuthenticated', () => { }) }) - test('records the snapshot and short-circuits without checking the session when api_key is already set', async () => { - // Given — a command like `app dev` already populated the full app context - const loadedPublic = { - api_key: 'already-set', - project_type: 'node', - app_scopes: '["x"]', - cmd_app_warning_api_key_deprecation_displayed: false, - } - vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue(loadedPublic) - vi.spyOn(metadata, 'getAllSensitiveMetadata').mockReturnValue({app_name: 'already-app'}) + 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 — only tags the source, does no load, and records the full snapshot. - const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) - expect(payloads).toEqual([{cmd_app_context_source: 'current_directory'}]) + // Then expect(sessionExists).not.toHaveBeenCalled() - expect(setMruSpy).toHaveBeenCalledWith({ - public: {api_key: 'already-set', project_type: 'node', app_scopes: '["x"]'}, - sensitive: {app_name: 'already-app'}, - }) - }) - - test('replays the full most-recently-used context tagged last_used when the directory is not an app project', async () => { - await inTemporaryDirectory(async (tmp) => { - // Given — no shopify.app.toml on disk, but we have a remembered app - vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) - const addSpy = vi.spyOn(metadata, 'addPublicMetadata') - const addSensitiveSpy = vi.spyOn(metadata, 'addSensitiveMetadata') - getMruSpy.mockReturnValue({ - public: {api_key: 'mru-key', project_type: 'node', app_scopes: '["read_products"]'}, - sensitive: {app_name: 'mru-app'}, - }) - - // When - await logAppContextMetadataIfAuthenticated(tmp) - - // Then — replays the whole public block tagged as deduced, plus the sensitive app_name. - const payloads = await Promise.all(addSpy.mock.calls.map((call) => call[0]())) - expect(payloads).toContainEqual({ - api_key: 'mru-key', - project_type: 'node', - app_scopes: '["read_products"]', - cmd_app_context_source: 'last_used', - }) - const sensitivePayloads = await Promise.all(addSensitiveSpy.mock.calls.map((call) => call[0]())) - expect(sensitivePayloads).toContainEqual({app_name: 'mru-app'}) - }) + expect(addSpy).not.toHaveBeenCalled() }) - test('never throws and adds nothing when the directory is not an app project and there is no MRU', async () => { + 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 and no remembered app + // Given — no shopify.app.toml on disk vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({}) const addSpy = vi.spyOn(metadata, 'addPublicMetadata') - getMruSpy.mockReturnValue(undefined) // When / Then await expect(logAppContextMetadataIfAuthenticated(tmp)).resolves.toBeUndefined() diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 793e074b529..c29d1edf7d2 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -1,10 +1,5 @@ import {appFromIdentifiers} from './context.js' -import { - getCachedAppInfo, - setCachedAppInfo, - getMostRecentlyUsedAppContext, - setMostRecentlyUsedAppContext, -} from './local-storage.js' +import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' import link from './app/config/link.js' import {fetchOrgFromId} from './dev/fetch.js' @@ -218,75 +213,18 @@ export async function localAppContext({ // every command, so it must never delay process exit, even on a pathological project. const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000 -type AppMetadataPublic = ReturnType -type AppMetadataSensitive = ReturnType - -// Sensitive metadata fields that are part of "app context" and worth replaying for -// out-of-app commands. Kept tiny and explicit so we never persist more than intended. -const APP_CONTEXT_SENSITIVE_KEYS = ['app_name'] - -/** - * Whether a public metadata key is part of the app-context block emitted when an app loads - * (see `logMetadataForLoadedAppUsingRawValues` in the loader). These are the fields we - * snapshot and replay so a command run outside an app directory looks like one run inside. - * Deliberately excludes per-run `cmd_app_*` flags (e.g. `cmd_app_warning_*`, - * `cmd_app_reset_used`) that describe the command rather than the app's identity/shape. - */ -function isAppContextPublicKey(key: string): boolean { - return ( - key === 'api_key' || - key === 'project_type' || - key.startsWith('app_') || - key.startsWith('cmd_app_all_configs_') || - key.startsWith('cmd_app_linked_config_') - ) -} - -/** - * Snapshots the app-context fields currently in the metadata container and records them as - * the most-recently-used app, so later commands run outside an app directory can replay - * them. No-op unless a non-empty `api_key` is present (i.e. an app actually resolved). - */ -function recordMostRecentlyUsedAppContext(): void { - const allPublic = metadata.getAllPublicMetadata() - if (typeof allPublic.api_key !== 'string' || allPublic.api_key.length === 0) return - - const publicContext: {[key: string]: unknown} = {} - for (const [key, value] of Object.entries(allPublic)) { - if (value !== undefined && isAppContextPublicKey(key)) publicContext[key] = value - } - - const allSensitive = metadata.getAllSensitiveMetadata() as {[key: string]: unknown} - const sensitiveContext: {[key: string]: unknown} = {} - for (const key of APP_CONTEXT_SENSITIVE_KEYS) { - if (allSensitive[key] !== undefined) sensitiveContext[key] = allSensitive[key] - } - - setMostRecentlyUsedAppContext({public: publicContext, sensitive: sensitiveContext}) -} - /** * Best-effort, non-interactive enrichment of command analytics with app context. * - * Attaches the full app-context block (`api_key`, `project_type`, and the `app_*` / - * `cmd_app_*` fields the loader emits) to the Monorail metadata so that every command — not - * just app commands — is attributed to an app for authenticated users. It resolves the app - * from one of two sources, recorded in the `cmd_app_context_source` field: - * 1. `"current_directory"` — `directory` is an app project, loaded from disk (no network, - * no prompts). This is the authoritative source; its full context is also snapshotted - * as the "most recently used" app for later. - * 2. `"last_used"` — `directory` is not an app project, so the previously snapshotted - * most-recently-used app context is replayed in full (public fields plus the sensitive - * `app_name`). This lets commands run *outside* any app directory (e.g. from a parent - * folder) still be attributed to the app the user was last working with. - * - * `cmd_app_context_source` is the explicit discriminator between the two; the absence of the - * loader-only `cmd_app_linked_config_*` fields on a `"last_used"` event is a secondary tell. + * 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), - * - when `api_key` is already set (a command like `app dev` already loaded the app), tags - * the source and snapshots that app as most-recently-used, but does no further work, + * - 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.). @@ -295,14 +233,7 @@ function recordMostRecentlyUsedAppContext(): void { */ export async function logAppContextMetadataIfAuthenticated(directory: string): Promise { try { - const existingApiKey = metadata.getAllPublicMetadata().api_key - if (typeof existingApiKey === 'string' && existingApiKey.length > 0) { - // A command (e.g. `app dev`) already resolved and emitted the full app context. Tag it - // as a real load and remember it so later out-of-app commands can replay it. - await metadata.addPublicMetadata(() => ({cmd_app_context_source: 'current_directory'})) - recordMostRecentlyUsedAppContext() - return - } + if (metadata.getAllPublicMetadata().api_key !== undefined) return if (!(await sessionExists())) return let timer: ReturnType | undefined @@ -311,31 +242,12 @@ export async function logAppContextMetadataIfAuthenticated(directory: string): P }) const load = (async () => { - try { - const {app} = await localAppContext({directory, skipPrompts: true}) - const clientId = app.configuration.client_id - if (typeof clientId === 'string' && clientId.length > 0) { - // localAppContext drove the loader, which already populated project_type and the - // app_* block. Add api_key + the source tag, then snapshot the whole thing. - await metadata.addPublicMetadata(() => ({api_key: clientId, cmd_app_context_source: 'current_directory'})) - recordMostRecentlyUsedAppContext() - return - } - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // `directory` isn't an app project (or it's unreadable) — fall back to the most - // recently used app below. - } - - const recent = getMostRecentlyUsedAppContext() - const recentApiKey = recent?.public?.api_key - if (typeof recentApiKey === 'string' && recentApiKey.length > 0) { - await metadata.addPublicMetadata( - () => ({...recent!.public, cmd_app_context_source: 'last_used'}) as unknown as AppMetadataPublic, - ) - if (recent!.sensitive && Object.keys(recent!.sensitive).length > 0) { - await metadata.addSensitiveMetadata(() => ({...recent!.sensitive}) as unknown as AppMetadataSensitive) - } + // 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})) } })() diff --git a/packages/app/src/cli/services/local-storage.test.ts b/packages/app/src/cli/services/local-storage.test.ts index 16b7055a0c1..a6c161d77f6 100644 --- a/packages/app/src/cli/services/local-storage.test.ts +++ b/packages/app/src/cli/services/local-storage.test.ts @@ -1,12 +1,9 @@ import { - AppContextLocalStorageSchema, AppLocalStorageSchema, clearCachedAppInfo, clearCurrentConfigFile, getCachedAppInfo, - getMostRecentlyUsedAppContext, setCachedAppInfo, - setMostRecentlyUsedAppContext, } from './local-storage.js' import {describe, expect, test} from 'vitest' import {LocalStorage} from '@shopify/cli-kit/node/local-storage' @@ -143,53 +140,3 @@ describe('clearCurrentConfigFile', async () => { }) }) }) - -describe('mostRecentlyUsedAppContext', async () => { - const snapshot = { - public: {api_key: 'mru-client-id', project_type: 'node', app_scopes: '["read_products"]'}, - sensitive: {app_name: 'my-app'}, - } - - test('returns undefined before anything is stored', async () => { - await inTemporaryDirectory(async (cwd) => { - // Given - const storage = new LocalStorage({cwd}) - - // When - const got = getMostRecentlyUsedAppContext(storage) - - // Then - expect(got).toBeUndefined() - }) - }) - - test('round-trips the full app context snapshot', async () => { - await inTemporaryDirectory(async (cwd) => { - // Given - const storage = new LocalStorage({cwd}) - - // When - setMostRecentlyUsedAppContext(snapshot, storage) - const got = getMostRecentlyUsedAppContext(storage) - - // Then - expect(got).toEqual(snapshot) - }) - }) - - test('overwrites the previously stored app context', async () => { - await inTemporaryDirectory(async (cwd) => { - // Given - const storage = new LocalStorage({cwd}) - setMostRecentlyUsedAppContext(snapshot, storage) - - // When - const next = {public: {api_key: 'new-client-id'}, sensitive: {}} - setMostRecentlyUsedAppContext(next, storage) - const got = getMostRecentlyUsedAppContext(storage) - - // Then - expect(got).toEqual(next) - }) - }) -}) diff --git a/packages/app/src/cli/services/local-storage.ts b/packages/app/src/cli/services/local-storage.ts index 63a8247b3e3..bb0f6f4aedf 100644 --- a/packages/app/src/cli/services/local-storage.ts +++ b/packages/app/src/cli/services/local-storage.ts @@ -128,42 +128,3 @@ export function setCachedCommandTomlMap(tomls: {[clientId: string]: AppConfigura export function setCachedCommandTomlPreference(selectedToml: AppConfigurationFileName) { setCachedCommandInfo({selectedToml}) } - -// A snapshot of the app-context analytics metadata (api_key, project_type, the app_* and -// cmd_app_* fields, and the sensitive app_name) for the last app the CLI resolved. Unlike -// `getCachedAppInfo`, which is keyed by directory, this is a single global value so that -// commands run *outside* any app directory can still be attributed to the app the user was -// most recently working with — the whole snapshot is replayed onto the event. Used only for -// best-effort telemetry enrichment. -export interface MostRecentlyUsedAppContext { - // Public metadata fields, keyed by their Monorail field name (e.g. `api_key`, `app_scopes`). - public?: {[key: string]: unknown} - // Sensitive metadata fields, keyed by their Monorail field name (currently just `app_name`). - sensitive?: {[key: string]: unknown} -} - -export interface AppContextLocalStorageSchema { - mostRecentlyUsedApp?: MostRecentlyUsedAppContext -} - -let _appContextLocalStorageInstance: LocalStorage | undefined - -function appContextLocalStorage() { - _appContextLocalStorageInstance ??= new LocalStorage({ - projectName: 'shopify-cli-app-context', - }) - return _appContextLocalStorageInstance -} - -export function getMostRecentlyUsedAppContext( - config: LocalStorage = appContextLocalStorage(), -): MostRecentlyUsedAppContext | undefined { - return config.get('mostRecentlyUsedApp') -} - -export function setMostRecentlyUsedAppContext( - context: MostRecentlyUsedAppContext, - config: LocalStorage = appContextLocalStorage(), -): void { - config.set('mostRecentlyUsedApp', context) -} diff --git a/packages/cli-kit/src/public/node/monorail.ts b/packages/cli-kit/src/public/node/monorail.ts index c3a14838a91..bb8d1e9dcd2 100644 --- a/packages/cli-kit/src/public/node/monorail.ts +++ b/packages/cli-kit/src/public/node/monorail.ts @@ -95,11 +95,6 @@ export interface Schemas { cmd_app_linked_config_source?: Optional cmd_app_linked_config_uses_cli_managed_urls?: Optional cmd_app_warning_api_key_deprecation_displayed?: Optional - // How the app context (api_key, project_type, app_*, …) on this event was resolved: - // "current_directory" when the app was loaded from the command's working directory, - // "last_used" when it was replayed from the most-recently-used app because the command - // ran outside any app directory. - cmd_app_context_source?: Optional cmd_app_deployment_mode?: Optional cmd_app_validate_json?: Optional cmd_app_validate_valid?: Optional