Fix delayed development store provisioning and E2E cleanup

packages/e2e/README.md

@@ -44,7 +44,7 @@ pnpm --filter e2e exec playwright test --project remote --shard 1/2
 
 `tests/store-lifecycle.spec.ts` is the one exception to store reuse. It creates one temporary development store with `store create dev --json`, checks it with `store list --json`, and deletes it with `store delete --force --json`. It requires confirmed deletion and checks that the store is absent from the active store list. It runs once across the two remote shards and needs one free store slot in the E2E organization.
 
-The lifecycle commands do not use a browser. They use the CLI session from shared authentication. The temporary store has an `e2e-store-` name with the run marker and a random suffix. Cleanup runs even when the test fails or `E2E_SKIP_TEARDOWN=1`, with a separate timeout. It finds only the exact test name and refuses to delete protected stores. The test report includes the temporary name for recovery if the runner stops before cleanup or provisioning finishes late.
+The lifecycle commands do not use a browser. They use the CLI session from shared authentication. The temporary store has an `e2e-store-` name with the run marker and a random suffix. Cleanup runs even when the test fails or `E2E_SKIP_TEARDOWN=1`, with a separate timeout. After a failed test, cleanup waits up to one minute for the store to appear. If the store remains absent, cleanup reports an unresolved failure. It finds only the exact test name and refuses to delete protected stores. The test report includes the temporary name for recovery if the runner stops before cleanup or provisioning finishes late.
 
 Run only this test with:
 

packages/e2e/setup/temporary-store.ts

@@ -4,6 +4,7 @@ import {e2eRunSegment, requireEnv} from './env.js'
 import {isProtectedStore, readPersistentStores} from './persistent-stores.js'
 import {deleteDevStoreWithCli} from './store.js'
 import {randomUUID} from 'node:crypto'
+import {setTimeout as delay} from 'node:timers/promises'
 import type {CLIProcess} from './cli.js'
 
 interface TemporaryStore {
@@ -54,9 +55,23 @@ export async function cleanupTemporaryStore(options: {
   cli: Pick<CLIProcess, 'exec'>
   orgId: string
   name: string
+  /** Wait for provisioning after a failed test before treating an absent store as unresolved cleanup. */
+  creationTimeoutMs?: number
 }): Promise<void> {
-  const store = await findTemporaryStore(options)
-  if (!store) return
+  const deadline = Date.now() + (options.creationTimeoutMs ?? 0)
+  let store = await findTemporaryStore(options)
+  while (!store && Date.now() < deadline) {
+    // eslint-disable-next-line no-await-in-loop
+    await delay(Math.min(5000, Math.max(0, deadline - Date.now())))
+    // eslint-disable-next-line no-await-in-loop
+    store = await findTemporaryStore(options)
+  }
+  if (!store) {
+    if (options.creationTimeoutMs) {
+      throw new Error(`Temporary store ${options.name} did not appear before cleanup timed out; cleanup is unresolved.`)
+    }
+    return
+  }
   const confirmed = await deleteDevStoreWithCli({...options, storeFqdn: store.store})
   if (!confirmed) throw new Error(`Deletion of temporary store ${store.store} was not confirmed.`)
 }
@@ -72,9 +87,14 @@ export const temporaryStoreFixture = authFixture.extend<{temporaryStoreName: str
         await use(name)
       } finally {
         // Creation can succeed remotely before the CLI returns its domain.
-        await cleanupTemporaryStore({cli, orgId: env.orgId, name})
+        await cleanupTemporaryStore({
+          cli,
+          orgId: env.orgId,
+          name,
+          creationTimeoutMs: testInfo.status === 'passed' ? 0 : CLI_TIMEOUT.short,
+        })
       }
     },
-    {timeout: CLI_TIMEOUT.store + CLI_TIMEOUT.short},
+    {timeout: CLI_TIMEOUT.store + 2 * CLI_TIMEOUT.short},
   ],
 })

packages/e2e/tests/persistent-stores.spec.ts

@@ -141,6 +141,7 @@ test('protects named persistent stores and configured stores from cleanup', asyn
 
 for (const scenario of [
   'owned',
+  'delayed',
   'absent',
   'protected',
   'duplicate',
@@ -153,6 +154,7 @@ for (const scenario of [
     const ownedStore = {name, store: 'e2e-store-local-test.myshopify.com', type: 'dev'}
     const candidates = {
       owned: [ownedStore],
+      delayed: [ownedStore],
       absent: [{...ownedStore, name: 'another-run'}],
       protected: [{...ownedStore, store: stores[0]}],
       duplicate: [ownedStore, {...ownedStore, store: 'another.myshopify.com'}],
@@ -161,15 +163,17 @@ for (const scenario of [
       unconfirmed: [ownedStore],
     }
     const deleted: string[] = []
+    let listings = 0
     const cli = {
       async exec(args: string[]) {
         if (args[1] === 'list') {
+          listings++
           return {
             exitCode: 0,
             stderr: '',
             stdout: JSON.stringify({
               organization: {id: scenario === 'wrong-org' ? '456' : '123'},
-              stores: candidates[scenario],
+              stores: scenario === 'delayed' && listings === 1 ? [] : candidates[scenario],
               truncated: scenario === 'truncated',
             }),
           }
@@ -183,8 +187,13 @@ for (const scenario of [
         }
       },
     }
-    const cleanup = cleanupTemporaryStore({cli, orgId: '123', name})
-    if (scenario === 'owned' || scenario === 'absent') {
+    const cleanup = cleanupTemporaryStore({
+      cli,
+      orgId: '123',
+      name,
+      creationTimeoutMs: scenario === 'absent' ? 0 : 1000,
+    })
+    if (scenario === 'owned' || scenario === 'absent' || scenario === 'delayed') {
       await cleanup
     } else {
       const errors = {
@@ -196,10 +205,44 @@ for (const scenario of [
       }
       await expect(cleanup).rejects.toThrow(errors[scenario])
     }
-    expect(deleted).toEqual(scenario === 'owned' || scenario === 'unconfirmed' ? [ownedStore.store] : [])
+    expect(deleted).toEqual(
+      scenario === 'owned' || scenario === 'delayed' || scenario === 'unconfirmed' ? [ownedStore.store] : [],
+    )
+    expect(listings).toBe(scenario === 'delayed' ? 2 : 1)
   })
 }
 
+test('reports unresolved cleanup when a store does not finish provisioning within the wait', async () => {
+  const cli = {
+    async exec(args: string[]) {
+      expect(args.slice(0, 2)).toEqual(['store', 'list'])
+      return {exitCode: 0, stdout: JSON.stringify({organization: {id: '123'}, stores: []}), stderr: ''}
+    },
+  }
+
+  await expect(
+    cleanupTemporaryStore({cli, orgId: '123', name: 'e2e-store-delayed', creationTimeoutMs: 1000}),
+  ).rejects.toThrow('Temporary store e2e-store-delayed did not appear before cleanup timed out; cleanup is unresolved.')
+})
+
+test('preserves listing errors while waiting for a store to finish provisioning', async () => {
+  let listings = 0
+  const cli = {
+    async exec(args: string[]) {
+      expect(args.slice(0, 2)).toEqual(['store', 'list'])
+      listings++
+      return listings === 1
+        ? {exitCode: 0, stdout: JSON.stringify({organization: {id: '123'}, stores: []}), stderr: ''}
+        : {exitCode: 1, stdout: '', stderr: 'Authentication failed'}
+    },
+  }
+
+  await expect(
+    cleanupTemporaryStore({cli, orgId: '123', name: 'e2e-store-delayed', creationTimeoutMs: 1000}),
+  ).rejects.toThrow('Authentication failed')
+  expect(listings).toBe(2)
+})
+
 test('legacy store cleanup requires an explicit empty state', async () => {
   const page = (empty: boolean) =>
     ({

packages/organizations/src/cli/services/dev/create-dev-store.test.ts

@@ -1,5 +1,6 @@
 import {createDevStoreJsonOutputSchema} from './types.js'
 import {createDevStore} from './create-dev-store.js'
+import {PollStoreCreation} from '../../api/graphql/business-platform-organizations/generated/poll_store_creation.js'
 import {describe, expect, test, vi, beforeEach} from 'vitest'
 
 import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform'
@@ -51,6 +52,81 @@ beforeEach(() => {
 })
 
 describe('createDevStore', () => {
+  test.each([undefined, null])('waits for an initially missing creation record (%s)', async (storeCreation) => {
+    vi.mocked(businessPlatformOrganizationsRequestDoc)
+      .mockResolvedValueOnce(defaultMutationResult)
+      .mockResolvedValueOnce({organization: {id: '123', storeCreation}})
+      .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status: 'AWAITING_CORE_STORE_READY'}}})
+      .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status: 'COMPLETE'}}})
+    vi.mocked(sleep).mockImplementation(async () => {
+      expect(outputResult).not.toHaveBeenCalled()
+    })
+
+    const domain = await createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic', json: true})
+
+    expect(domain).toBe('test-store.myshopify.com')
+    expect(sleep).toHaveBeenCalledTimes(2)
+    expect(businessPlatformOrganizationsRequestDoc).toHaveBeenCalledTimes(4)
+    for (const [request] of vi.mocked(businessPlatformOrganizationsRequestDoc).mock.calls.slice(1)) {
+      expect(request).toMatchObject({query: PollStoreCreation, variables: {shopDomain: domain}})
+    }
+    expect(JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string).store.domain).toBe(domain)
+  })
+
+  test('stops waiting for a missing creation record at the existing timeout', async () => {
+    let now = 0
+    const clock = vi.spyOn(Date, 'now').mockImplementation(() => now)
+    vi.mocked(businessPlatformOrganizationsRequestDoc)
+      .mockResolvedValueOnce(defaultMutationResult)
+      .mockResolvedValue({organization: {id: '123', storeCreation: null}})
+    vi.mocked(sleep).mockImplementation(async (seconds) => {
+      now += seconds * 1000
+    })
+
+    try {
+      await expect(
+        createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic', json: true}),
+      ).rejects.toThrow('Store creation timed out after 5 minutes.')
+      expect(now).toBeGreaterThanOrEqual(5 * 60_000)
+      expect(now).toBeLessThanOrEqual(5 * 60_000 + 2000)
+      expect(outputResult).not.toHaveBeenCalled()
+    } finally {
+      clock.mockRestore()
+    }
+  })
+
+  test.each([undefined, null])('does not retry a missing organization (%s)', async (organization) => {
+    vi.mocked(businessPlatformOrganizationsRequestDoc)
+      .mockResolvedValueOnce(defaultMutationResult)
+      .mockResolvedValueOnce({organization})
+
+    await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toThrow(
+      'Unable to determine store creation status.',
+    )
+    expect(sleep).not.toHaveBeenCalled()
+  })
+
+  test.each(['FAILED', 'TIMED_OUT', 'USER_ERROR'])('does not retry terminal creation status %s', async (status) => {
+    vi.mocked(businessPlatformOrganizationsRequestDoc)
+      .mockResolvedValueOnce(defaultMutationResult)
+      .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status}}})
+
+    await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toThrow(
+      `Store creation failed with status: ${status}`,
+    )
+    expect(sleep).not.toHaveBeenCalled()
+  })
+
+  test('preserves unrelated polling errors', async () => {
+    const error = new Error('Authentication failed')
+    vi.mocked(businessPlatformOrganizationsRequestDoc)
+      .mockResolvedValueOnce(defaultMutationResult)
+      .mockRejectedValueOnce(error)
+
+    await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toBe(error)
+    expect(sleep).not.toHaveBeenCalled()
+  })
+
   test('returns the polled shop domain without rendering output when summary is false', async () => {
     vi.mocked(businessPlatformOrganizationsRequestDoc)
       .mockResolvedValueOnce(defaultMutationResult)

packages/organizations/src/cli/services/dev/create-dev-store.ts

@@ -107,10 +107,10 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise<st
           unauthorizedHandler,
         })
 
-        const status = pollResult.organization?.storeCreation?.status
-        if (!status) {
+        if (!pollResult.organization) {
           throw new AbortError('Unable to determine store creation status.')
         }
+        const status = pollResult.organization.storeCreation?.status
 
         if (status === 'COMPLETE') {
           return
@@ -119,7 +119,8 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise<st
           throw new AbortError(`Store creation failed with status: ${status}`)
         }
 
-        updateStatus(outputContent`${friendlyStatus(status)}`)
+        // The creation record can take time to become available after the mutation succeeds.
+        if (status) updateStatus(outputContent`${friendlyStatus(status)}`)
 
         // eslint-disable-next-line no-await-in-loop
         await sleep(POLL_INTERVAL_SECONDS)