Lazy-load app context in the public_command_metadata analytics hook

packages/app/src/cli/constants.ts

@@ -8,6 +8,9 @@ export const environmentVariableNames = {
   disableMinificationOnDev: 'SHOPIFY_CLI_DISABLE_MINIFICATION_ON_DEV',
 }
 
+// Matches the default config file (shopify.app.toml) and named configs (shopify.app.<name>.toml)
+export const appConfigurationFileGlob = 'shopify.app*.toml'
+
 export const configurationFileNames = {
   app: 'shopify.app.toml',
   web: 'shopify.web.toml',

packages/app/src/cli/hooks/public_metadata.test.ts

@@ -2,51 +2,85 @@ import gatherPublicMetadata from './public_metadata.js'
 import {localAppContext} 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'
+import {cwd, joinPath} from '@shopify/cli-kit/node/path'
+import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'
 
 vi.mock('../services/app-context.js')
-vi.mock('@shopify/cli-kit/node/path')
+vi.mock('@shopify/cli-kit/node/path', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@shopify/cli-kit/node/path')>()
+  return {...actual, cwd: vi.fn()}
+})
+
+async function inTemporaryAppProject(runTest: (appDirectory: string) => Promise<void>): Promise<void> {
+  await inTemporaryDirectory(async (tmpDir) => {
+    await writeFile(joinPath(tmpDir, 'shopify.app.toml'), '')
+    await runTest(tmpDir)
+  })
+}
 
 describe('gatherPublicMetadata', () => {
   beforeEach(() => {
-    vi.mocked(cwd).mockReturnValue('/some/app/dir')
     vi.mocked(localAppContext).mockResolvedValue({} as Awaited<ReturnType<typeof localAppContext>>)
   })
 
   test('opportunistically enriches metadata from the current directory and returns the public metadata', async () => {
-    // Given
-    vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue({api_key: 'from-loader'})
+    await inTemporaryAppProject(async (appDirectory) => {
+      // Given
+      vi.mocked(cwd).mockReturnValue(appDirectory)
+      vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue({api_key: 'from-loader'})
 
-    // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+      // When
+      const result = await (gatherPublicMetadata as () => Promise<unknown>)()
 
-    // Then
-    expect(localAppContext).toHaveBeenCalledWith({directory: '/some/app/dir', skipPrompts: true})
-    expect(result).toEqual(metadata.getAllPublicMetadata())
+      // Then
+      expect(localAppContext).toHaveBeenCalledWith({directory: appDirectory, skipPrompts: true})
+      expect(result).toEqual(metadata.getAllPublicMetadata())
+    })
   })
 
   test('skips local app loading when api_key is already set', async () => {
-    // Given
-    vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({api_key: 'already-set'})
+    await inTemporaryAppProject(async (appDirectory) => {
+      // Given
+      vi.mocked(cwd).mockReturnValue(appDirectory)
+      vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({api_key: 'already-set'})
+
+      // When
+      const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+
+      // Then
+      expect(localAppContext).not.toHaveBeenCalled()
+      expect(result).toEqual(metadata.getAllPublicMetadata())
+    })
+  })
+
+  test('skips local app loading when the directory is not inside an app project', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      vi.mocked(cwd).mockReturnValue(tmpDir)
+      vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({})
 
-    // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+      // When
+      const result = await (gatherPublicMetadata as () => Promise<unknown>)()
 
-    // Then
-    expect(localAppContext).not.toHaveBeenCalled()
-    expect(result).toEqual(metadata.getAllPublicMetadata())
+      // Then
+      expect(localAppContext).not.toHaveBeenCalled()
+      expect(result).toEqual(metadata.getAllPublicMetadata())
+    })
   })
 
   test('still returns metadata when best-effort app loading fails', async () => {
-    // Given
-    vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({})
-    vi.mocked(localAppContext).mockRejectedValue(new Error('not an app'))
+    await inTemporaryAppProject(async (appDirectory) => {
+      // Given
+      vi.mocked(cwd).mockReturnValue(appDirectory)
+      vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({})
+      vi.mocked(localAppContext).mockRejectedValue(new Error('not an app'))
 
-    // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+      // When
+      const result = await (gatherPublicMetadata as () => Promise<unknown>)()
 
-    // Then
-    expect(localAppContext).toHaveBeenCalledOnce()
-    expect(result).toEqual(metadata.getAllPublicMetadata())
+      // Then
+      expect(localAppContext).toHaveBeenCalledOnce()
+      expect(result).toEqual(metadata.getAllPublicMetadata())
+    })
   })
 })

packages/app/src/cli/hooks/public_metadata.ts

@@ -1,15 +1,31 @@
 import metadata from '../metadata.js'
-import {localAppContext} from '../services/app-context.js'
+import {appConfigurationFileGlob} from '../constants.js'
 import {FanoutHookFunction} from '@shopify/cli-kit/node/plugins'
-import {cwd} from '@shopify/cli-kit/node/path'
+import {cwd, joinPath} from '@shopify/cli-kit/node/path'
+import {findPathUp, glob} from '@shopify/cli-kit/node/fs'
 
 const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000
 
+async function insideAppProject(directory: string): Promise<boolean> {
+  const found = await findPathUp(
+    async (candidateDirectory) => {
+      const matches = await glob(joinPath(candidateDirectory, appConfigurationFileGlob))
+      if (matches.length > 0) return candidateDirectory
+    },
+    {cwd: directory, type: 'directory'},
+  )
+  return found !== undefined
+}
+
 async function logAppContextMetadata(directory: string): Promise<void> {
   let timer: ReturnType<typeof setTimeout> | undefined
   try {
     if (metadata.getAllPublicMetadata().api_key !== undefined) return
+    if (!(await insideAppProject(directory))) return
 
+    // Loading the app context pulls in a large module graph, so only import it
+    // once we know the command ran inside an app project.
+    const {localAppContext} = await import('../services/app-context.js')
     await Promise.race([
       localAppContext({directory, skipPrompts: true}),
       new Promise<void>((resolve) => {

packages/app/src/cli/models/project/project.ts

@@ -1,4 +1,4 @@
-import {configurationFileNames} from '../../constants.js'
+import {appConfigurationFileGlob, configurationFileNames} from '../../constants.js'
 import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file'
 import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
 import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs'
@@ -12,7 +12,7 @@ import {joinPath, basename} from '@shopify/cli-kit/node/path'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {JsonMapType} from '@shopify/cli-kit/node/toml'
 
-const APP_CONFIG_GLOB = 'shopify.app*.toml'
+const APP_CONFIG_GLOB = appConfigurationFileGlob
 const APP_CONFIG_REGEX = /^shopify\.app(\.[-\w]+)?\.toml$/
 const EXTENSION_TOML = '*.extension.toml'
 const WEB_TOML = 'shopify.web.toml'

packages/cli-kit/src/private/node/otel-metrics.test.ts

@@ -26,11 +26,13 @@ describe('otel-metrics', () => {
 
   test('logs metrics when activated', async () => {
     const mockOtelRecorder = vi.fn()
+    const mockForceFlush = vi.fn().mockResolvedValue(undefined)
     const mockOtelCreator = vi.fn()
     mockOtelCreator.mockReturnValue({
       type: 'otel',
       otel: {
         record: mockOtelRecorder,
+        getMeterProvider: () => ({forceFlush: mockForceFlush}),
       },
     })
 
@@ -52,5 +54,42 @@ describe('otel-metrics', () => {
 
     expect(mockOtelCreator).toHaveBeenCalledOnce()
     expect(mockOtelRecorder.mock.calls).toMatchSnapshot()
+    expect(mockForceFlush).toHaveBeenCalledOnce()
+  })
+
+  test('waits for metrics to flush', async () => {
+    let resolveFlush: () => void = () => {}
+    const flush = new Promise<void>((resolve) => {
+      resolveFlush = resolve
+    })
+    const recorderFactory = vi.fn().mockReturnValue({
+      type: 'otel',
+      otel: {
+        record: vi.fn(),
+        getMeterProvider: () => ({forceFlush: () => flush}),
+      },
+    })
+
+    let metricsRecorded = false
+    const recording = recordMetrics(
+      {
+        skipMetricAnalytics: false,
+        cliVersion: '4.6.0',
+        owningPlugin: '@shopify/app',
+        command: 'app dev',
+        exitMode: 'ok',
+      },
+      {active: 10, network: 20, prompt: 30},
+      recorderFactory,
+    ).then(() => {
+      metricsRecorded = true
+    })
+
+    await vi.waitFor(() => expect(recorderFactory).toHaveBeenCalledOnce())
+    expect(metricsRecorded).toBe(false)
+
+    resolveFlush()
+    await recording
+    expect(metricsRecorded).toBe(true)
   })
 })

packages/cli-kit/src/private/node/otel-metrics.ts

@@ -11,7 +11,7 @@ type MetricRecorder =
   | 'console'
   | {
       type: 'otel'
-      otel: Pick<OtelService, 'record'>
+      otel: Pick<OtelService, 'getMeterProvider' | 'record'>
     }
 
 // this should be type, not interface
@@ -80,6 +80,9 @@ export async function recordMetrics(
 
   recordCommandCounter(recorder, labels)
   recordCommandTiming(recorder, labels, timing)
+  if (recorder !== 'console') {
+    await recorder.otel.getMeterProvider().forceFlush({})
+  }
 }
 
 const COMMAND_DURATION_BOUNDARIES_MS = [