Make teardown app resolution and browser delete resilient

- appByKey now sends organizationId: without it a deleted app cannot
  resolve an organization and 404s instead of returning null.
- Client IDs are only taken from /apps/{segment} URL parts that are
  non-numeric; deploy output yields /apps/{numericAppId} URLs, which
  now fall back to a name search.
- The settings-page navigation clicks through the accounts.shopify.com
  account picker, which cold browser contexts bounce to — the main
  reason direct-URL app deletion has been failing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: isaacroldan

packages/e2e/scripts/probe-api.ts

@@ -0,0 +1,49 @@
+/* eslint-disable no-console */
+// Throwaway diagnostic: exercise the new app-management-api helper with
+// read-only queries against the real API. Not committed — local debugging only.
+import {config} from 'dotenv'
+import * as path from 'path'
+import {fileURLToPath} from 'url'
+import {findAppByClientId, findAppByName, appInstallCount} from '../setup/app-management-api.js'
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url))
+config({path: path.resolve(__dirname, '../.env')})
+
+const orgId = (process.env.E2E_ORG_ID ?? '').trim()
+
+async function main() {
+  console.log('[probe-api] org:', orgId)
+
+  // 1. appByKey with a bogus key — proves auth + query validity; expects undefined
+  const bogus = await findAppByClientId(process.env, '0'.repeat(32))
+  console.log('[probe-api] findAppByClientId(bogus):', bogus === undefined ? 'undefined (expected)' : bogus)
+
+  // 2. name search for a name that should not exist — proves appsConnection + org routing
+  const missing = await findAppByName(process.env, 'E2E-probe-does-not-exist', orgId)
+  console.log('[probe-api] findAppByName(missing):', missing === undefined ? 'undefined (expected)' : missing)
+
+  // 3. name search with a broad E2E prefix — if any leaked app exists, exercise installCount on it
+  const anyApp = await findAppByName(process.env, 'E2E-', orgId)
+  if (anyApp) {
+    const count = await appInstallCount(process.env, anyApp.id)
+    console.log(`[probe-api] installCount(${anyApp.key.slice(0, 8)}…):`, count)
+  } else {
+    console.log('[probe-api] no app named exactly "E2E-" (expected) — resolving one via secondary client id instead')
+    const secondary = (process.env.E2E_SECONDARY_CLIENT_ID ?? '').trim()
+    if (secondary) {
+      const app = await findAppByClientId(process.env, secondary)
+      console.log('[probe-api] findAppByClientId(secondary):', app ? `id=${app.id}` : 'undefined')
+      if (app) {
+        const count = await appInstallCount(process.env, app.id)
+        console.log('[probe-api] installCount(secondary):', count)
+      }
+    }
+  }
+
+  console.log('[probe-api] all queries executed without errors')
+}
+
+main().catch((err) => {
+  console.error('[probe-api] fatal:', err instanceof Error ? err.message : err)
+  process.exitCode = 1
+})

packages/e2e/setup/app-management-api.ts

@@ -89,20 +89,24 @@ async function appManagementQuery(
  * Look up an app by its API key (client_id). Returns undefined when the app
  * does not exist — for teardown that means it was already deleted.
  *
- * The variable must be named `apiKey`: the API resolves the request's
- * organization from specifically-named variables (`organizationId`, `apiKey`,
- * `appId`) and rejects the request with "Cannot find a valid organization"
- * otherwise.
+ * Variable names matter: the API resolves the request's organization from
+ * specifically-named variables (`organizationId`, `apiKey`, `appId`) and
+ * rejects the request with "Cannot find a valid organization" otherwise.
+ * `organizationId` must be sent even though `apiKey` alone can resolve it:
+ * a deleted app resolves no organization, which would surface as that same
+ * rejection instead of a null `appByKey`.
  */
 export async function findAppByClientId(
   sessionEnv: NodeJS.ProcessEnv,
   clientId: string,
+  orgId: string,
 ): Promise<AppManagementApp | undefined> {
   const data = (await appManagementQuery(
     sessionEnv,
     'query appByKey($apiKey: String!) { appByKey(key: $apiKey) { id key } }',
     {
       apiKey: clientId,
+      organizationId: orgId,
     },
   )) as {appByKey?: AppManagementApp | null}
   return data.appByKey ?? undefined

packages/e2e/setup/app.ts

@@ -323,6 +323,29 @@ export async function configLink(
 // Dev dashboard browser actions — delete apps
 // ---------------------------------------------------------------------------
 
+/**
+ * Load the app's settings page, clicking through the accounts.shopify.com
+ * account picker if the session bounces there — which is common when the
+ * browser context has not visited the Dev Dashboard yet.
+ */
+async function gotoAppSettings(page: Page, appUrl: string): Promise<void> {
+  await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
+  await page.waitForTimeout(BROWSER_TIMEOUT.medium)
+
+  if (!page.url().startsWith('https://accounts.shopify.com')) return
+
+  const email = process.env.E2E_ACCOUNT_EMAIL
+  if (email) {
+    const accountButton = page.locator(`text=${email}`).first()
+    if (await isVisibleWithin(accountButton, BROWSER_TIMEOUT.long)) {
+      await accountButton.click()
+      await page.waitForTimeout(BROWSER_TIMEOUT.medium)
+    }
+  }
+  await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
+  await page.waitForTimeout(BROWSER_TIMEOUT.medium)
+}
+
 /**
  * Delete an app from its dev dashboard settings page. Returns true if deleted.
  *
@@ -333,8 +356,7 @@ export async function configLink(
  */
 export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Promise<boolean> {
   // Step 1: Navigate to the app's settings page. 404 → already deleted. 5xx → throw for retry.
-  await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
-  await page.waitForTimeout(BROWSER_TIMEOUT.medium)
+  await gotoAppSettings(page, appUrl)
   const gotoStatus = getLastPageStatus(page)
   if (gotoStatus === 404) return true
   if (gotoStatus !== undefined && gotoStatus >= 500) {
@@ -345,6 +367,11 @@ export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Pro
   // Button can be below the fold, and takes ~1-2s to enable after uninstall (one reload covers propagation lag).
   // If it stays disabled after reload, installs remain — fail fast for caller.
   const deleteBtn = page.locator('button:has-text("Delete app")').first()
+  if (!(await isVisibleWithin(deleteBtn, BROWSER_TIMEOUT.long))) {
+    // Include the landed URL: the usual cause is a session bounce that the
+    // account-picker handling above did not cover.
+    throw new Error(`Delete app button not found (page: ${page.url()})`)
+  }
   await deleteBtn.scrollIntoViewIfNeeded({timeout: BROWSER_TIMEOUT.long})
   if (!(await deleteBtn.isEnabled())) {
     await page.reload({waitUntil: 'domcontentloaded'})

packages/e2e/setup/teardown.ts

@@ -52,9 +52,10 @@ export async function teardownAll(ctx: TeardownCtx): Promise<void> {
   const clientId = resolveClientId(ctx)
   for (let attempt = 1; attempt <= 3; attempt++) {
     try {
-      if (clientId) {
-        app = await findAppByClientId(sessionEnv, clientId)
-      } else if (ctx.env.orgId) {
+      if (clientId && ctx.env.orgId) {
+        app = await findAppByClientId(sessionEnv, clientId, ctx.env.orgId)
+      }
+      if (!app && ctx.env.orgId) {
         app = await findAppByName(sessionEnv, ctx.appName, ctx.env.orgId)
       }
       appResolved = true
@@ -170,7 +171,10 @@ export async function teardownAll(ctx: TeardownCtx): Promise<void> {
 
 /**
  * The app's client_id: read from the local TOML when the app dir is known,
- * otherwise take the last segment of the Dev Dashboard app URL.
+ * otherwise from the Dev Dashboard app URL. Dashboard URLs come in two
+ * shapes — apps/[clientId] (built by devDashboardAppUrl) and
+ * apps/[numericAppId] (parsed from deploy output) — and only the former is
+ * usable as an API key, so numeric segments resolve via name search instead.
  */
 function resolveClientId(ctx: TeardownCtx): string | undefined {
   if (ctx.appDir) {
@@ -181,5 +185,6 @@ function resolveClientId(ctx: TeardownCtx): string | undefined {
       // TOML may be missing when the test failed before app creation.
     }
   }
-  return ctx.appUrl?.split('/').filter(Boolean).at(-1)
+  const urlSegment = ctx.appUrl?.match(/\/apps\/([^/?#]+)/)?.[1]
+  return urlSegment && !/^\d+$/.test(urlSegment) ? urlSegment : undefined
 }