Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/app-context-telemetry-everywhere.md
Original file line number Diff line number Diff line change
@@ -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).
38 changes: 38 additions & 0 deletions packages/app/src/cli/hooks/public_metadata.test.ts
Original file line number Diff line number Diff line change
@@ -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<unknown>)()

// 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<unknown>)()

// Then
expect(logAppContextMetadataIfAuthenticated).toHaveBeenCalledOnce()
expect(result).toBeTypeOf('object')
})
})
3 changes: 3 additions & 0 deletions packages/app/src/cli/hooks/public_metadata.ts
Original file line number Diff line number Diff line change
@@ -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()
}

Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
82 changes: 81 additions & 1 deletion packages/app/src/cli/services/app-context.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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()
})
})
})
58 changes: 57 additions & 1 deletion packages/app/src/cli/services/app-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'
Expand Down Expand Up @@ -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
Comment thread
nelsonwittwer marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -188,8 +192,9 @@ interface LocalAppContextOutput {
export async function localAppContext({
directory,
userProvidedConfigName,
skipPrompts = false,
}: LocalAppContextOptions): Promise<LocalAppContextOutput> {
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'))
Expand All @@ -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<void> {
try {
if (metadata.getAllPublicMetadata().api_key !== undefined) return
if (!(await sessionExists())) return

let timer: ReturnType<typeof setTimeout> | undefined
const deadline = new Promise<void>((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.
}
}
23 changes: 23 additions & 0 deletions packages/cli-kit/src/public/node/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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')
Expand Down
20 changes: 20 additions & 0 deletions packages/cli-kit/src/public/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
const sessions = await sessionStore.fetch()
return sessions !== undefined && Object.keys(sessions).length > 0
}

/**
* Ensure that we have a valid session with no particular scopes.
*
Expand Down
Loading