Enforce E2E process and port ownership

packages/e2e/package.json

@@ -31,6 +31,7 @@
   "devDependencies": {
     "@iarna/toml": "2.2.5",
     "@playwright/test": "^1.61.1",
+    "@shopify/cli-kit": "4.6.0",
     "@shopify/toml-patch": "0.3.0",
     "@types/node": "22.20.1",
     "dotenv": "16.6.1",

packages/e2e/playwright.config.ts

@@ -31,6 +31,7 @@ export default defineConfig({
         'tests/smoke-pty.spec.ts',
         'tests/fixture-toml.spec.ts',
         'tests/auth-diagnostics.spec.ts',
+        'tests/ownership.spec.ts',
       ],
     },
     {
@@ -45,6 +46,7 @@ export default defineConfig({
         'tests/smoke-pty.spec.ts',
         'tests/fixture-toml.spec.ts',
         'tests/auth-diagnostics.spec.ts',
+        'tests/ownership.spec.ts',
       ],
       dependencies: ['remote-auth'],
     },

packages/e2e/scripts/prime-browser-auth.ts

@@ -17,6 +17,7 @@ import {chromium} from '@playwright/test'
 import {BROWSER_TIMEOUT, CLI_TIMEOUT} from '../setup/constants.js'
 import {executables} from '../setup/env.js'
 import {isVisibleWithin} from '../setup/browser.js'
+import {observePtyExit, terminateProcessTree} from '../setup/process.js'
 import {completeLogin} from '../helpers/browser-login.js'
 import {addLoadtestHeader} from '../helpers/loadtest-header.js'
 import {stripAnsi} from '../helpers/strip-ansi.js'
@@ -160,6 +161,7 @@ async function primeCliAuth(page: Page, email: string, password: string, env: No
     rows: 30,
     env: spawnEnv,
   })
+  const exitObserver = observePtyExit(ptyProcess)
 
   let output = ''
   ptyProcess.onData((data: string) => {
@@ -178,11 +180,13 @@ async function primeCliAuth(page: Page, email: string, password: string, env: No
     await completeLogin(page, urlMatch[0], email, password)
     await waitForText(() => output, 'Logged in', BROWSER_TIMEOUT.max)
   } finally {
-    try {
-      ptyProcess.kill()
-      // eslint-disable-next-line no-catch-all/no-catch-all
-    } catch (_error) {
-      // Process may already be dead.
+    if (!exitObserver.hasExited()) {
+      await terminateProcessTree({
+        pid: ptyProcess.pid,
+        command: `node ${executables.cli} auth login`,
+        owner: 'prime-browser-auth',
+        waitForExit: exitObserver.waitForExit,
+      })
     }
   }
 }

packages/e2e/setup/app.ts

@@ -315,7 +315,7 @@ export async function configLink(
     const exitCode = await proc.waitForExit(CLI_TIMEOUT.long)
     return {exitCode, stdout: proc.getOutput(), stderr: ''}
   } finally {
-    proc.kill()
+    await proc.terminate()
   }
 }
 

packages/e2e/setup/cli.ts

@@ -1,5 +1,7 @@
 import {CLI_TIMEOUT} from './constants.js'
 import {createLogger, envFixture, executables} from './env.js'
+import {assertPortsAvailable} from './ports.js'
+import {observePtyExit, terminateProcessTree} from './process.js'
 import {stripAnsi} from '../helpers/strip-ansi.js'
 import {execa, type Options as ExecaOptions} from 'execa'
 import type {E2EEnv} from './env.js'
@@ -30,8 +32,8 @@ export interface SpawnedProcess {
   sendLine(line: string): void
   /** Wait for the process to exit */
   waitForExit(timeoutMs?: number): Promise<number>
-  /** Kill the process */
-  kill(): void
+  /** Terminate the complete process tree and wait for it to exit */
+  terminate(timeoutMs?: number): Promise<void>
   /** Get all output captured so far (ANSI stripped) */
   getOutput(): string
   /** The underlying node-pty process */
@@ -49,7 +51,7 @@ export interface CLIProcess {
 
 /**
  * Test-scoped fixture providing CLI process management.
- * Tracks all spawned processes and kills them in teardown.
+ * Tracks all spawned processes and terminates them in teardown.
  */
 export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
   cli: async ({env}, use) => {
@@ -124,6 +126,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
 
         cliLog.log(env, `spawn: node ${executables.cli}`)
         cliLog.log(env, args.join(' '))
+        const command = ['node', executables.cli, ...args].map((value) => JSON.stringify(value)).join(' ')
 
         const ptyProcess = nodePty.spawn('node', [executables.cli, ...args], {
           name: 'xterm-color',
@@ -153,14 +156,9 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
           }
         })
 
-        let exitCode: number | undefined
-        let exitResolve: ((code: number) => void) | undefined
+        const exitObserver = observePtyExit(ptyProcess)
 
         ptyProcess.onExit(({exitCode: code}) => {
-          exitCode = code
-          if (exitResolve) {
-            exitResolve(code)
-          }
           // Reject any remaining output waiters. reject() removes each waiter
           // from outputWaiters, so iterate over a snapshot to avoid skipping.
           for (const waiter of [...outputWaiters]) {
@@ -241,29 +239,21 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
           },
 
           waitForExit(timeoutMs = CLI_TIMEOUT.short) {
-            if (exitCode !== undefined) {
-              return Promise.resolve(exitCode)
-            }
-
-            return new Promise<number>((resolve, reject) => {
-              const timer = setTimeout(() => {
-                reject(new Error(`Timed out after ${timeoutMs}ms waiting for process exit`))
-              }, timeoutMs)
-
-              exitResolve = (code) => {
-                clearTimeout(timer)
-                resolve(code)
-              }
-            })
+            return exitObserver.waitForExit(timeoutMs)
           },
 
-          kill() {
-            try {
-              ptyProcess.kill()
-              // eslint-disable-next-line no-catch-all/no-catch-all
-            } catch (_error) {
-              // Process may already be dead
-            }
+          async terminate(timeoutMs) {
+            if (exitObserver.hasExited()) return
+
+            await terminateProcessTree(
+              {
+                pid: ptyProcess.pid,
+                command,
+                owner: `worker=${env.workerIndex}`,
+                waitForExit: exitObserver.waitForExit,
+              },
+              timeoutMs,
+            )
           },
 
           getOutput() {
@@ -276,11 +266,43 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
       },
     }
 
-    await use(cli)
+    let testFailed = false
+    let testFailure: unknown
+    try {
+      await use(cli)
+      // eslint-disable-next-line no-catch-all/no-catch-all
+    } catch (error) {
+      testFailed = true
+      testFailure = error
+    }
+
+    const cleanupFailures: Error[] = []
+    const ownedPorts = env.ownedPorts
 
-    // Teardown: kill all spawned processes
     for (const proc of spawnedProcesses) {
-      proc.kill()
+      try {
+        // eslint-disable-next-line no-await-in-loop
+        await proc.terminate()
+        // eslint-disable-next-line no-catch-all/no-catch-all
+      } catch (error) {
+        cleanupFailures.push(error instanceof Error ? error : new Error(String(error)))
+      }
+    }
+
+    try {
+      await assertPortsAvailable(ownedPorts, `worker=${env.workerIndex} phase=release`)
+      // eslint-disable-next-line no-catch-all/no-catch-all
+    } catch (error) {
+      cleanupFailures.push(error instanceof Error ? error : new Error(String(error)))
+    }
+    ownedPorts.splice(0, ownedPorts.length)
+
+    if (testFailed && cleanupFailures.length > 0) {
+      throw new AggregateError([testFailure, ...cleanupFailures], `[e2e][w${env.workerIndex}] test and cleanup failed`)
+    }
+    if (testFailed) throw testFailure
+    if (cleanupFailures.length > 0) {
+      throw new AggregateError(cleanupFailures, `[e2e][w${env.workerIndex}] cleanup failed`)
     }
   },
 })

packages/e2e/setup/env.ts

@@ -3,6 +3,7 @@ import {test as base} from '@playwright/test'
 import * as path from 'path'
 import * as fs from 'fs'
 import {fileURLToPath} from 'url'
+import type {OwnedPort} from './ports.js'
 
 const __filename = fileURLToPath(import.meta.url)
 const __dirname = path.dirname(__filename)
@@ -18,6 +19,8 @@ export interface E2EEnv {
   tempDir: string
   /** Playwright worker index (0-based) for debug logging */
   workerIndex: number
+  /** Ports claimed by the current test and verified during CLI fixture cleanup */
+  ownedPorts: OwnedPort[]
 }
 
 /** Worker context for logging */
@@ -196,6 +199,7 @@ export const envFixture = base.extend<{testSection: void}, {env: E2EEnv}>({
         processEnv,
         tempDir,
         workerIndex: workerInfo.parallelIndex,
+        ownedPorts: [],
       }
 
       await use(env)

packages/e2e/setup/global-auth.ts

@@ -8,6 +8,7 @@
 import {isVisibleWithin} from './browser.js'
 import {executables, globalLog} from './env.js'
 import {authStatePaths} from './auth-state.js'
+import {observePtyExit, terminateProcessTree} from './process.js'
 import {
   AuthSetupError,
   isExpectedAuthDestination,
@@ -130,6 +131,7 @@ async function authenticateOnce({
   } catch (_error) {
     throw new AuthSetupError('pty-startup', 'spawn-failed')
   }
+  const exitObserver = observePtyExit(ptyProcess)
 
   let output = ''
   ptyProcess.onData((data: string) => {
@@ -177,11 +179,13 @@ async function authenticateOnce({
       throw error
     }
   } finally {
-    try {
-      ptyProcess.kill()
-      // eslint-disable-next-line no-catch-all/no-catch-all
-    } catch (_error) {
-      // Process may already be dead
+    if (!exitObserver.hasExited()) {
+      await terminateProcessTree({
+        pid: ptyProcess.pid,
+        command: `node ${executables.cli} auth login`,
+        owner: 'global-auth',
+        waitForExit: exitObserver.waitForExit,
+      })
     }
   }
 }

packages/e2e/setup/ports.ts

@@ -0,0 +1,35 @@
+import {createServer} from 'node:net'
+
+export interface OwnedPort {
+  environmentVariable: string
+  port: number
+}
+
+export function workerPorts(workerIndex: number): OwnedPort[] {
+  const portBase = 3457 + workerIndex * 10
+  return [
+    {environmentVariable: 'SHOPIFY_FLAG_GRAPHIQL_PORT', port: portBase},
+    {environmentVariable: 'SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT', port: portBase + 2},
+  ]
+}
+
+export async function assertPortsAvailable(ports: OwnedPort[], owner: string): Promise<void> {
+  const availability = await Promise.all(
+    ports.map(async (port) => ({...port, available: await isPortAvailable(port.port)})),
+  )
+  const occupiedPorts = availability.filter(({available}) => !available)
+
+  if (occupiedPorts.length > 0) {
+    const details = occupiedPorts.map(({environmentVariable, port}) => `${environmentVariable}=${port}`).join(', ')
+    throw new Error(`[e2e][ports] owner=${owner} unavailable=${details}`)
+  }
+}
+
+function isPortAvailable(port: number): Promise<boolean> {
+  return new Promise((resolve) => {
+    const server = createServer()
+    server.unref()
+    server.once('error', () => resolve(false))
+    server.listen(port, 'localhost', () => server.close(() => resolve(true)))
+  })
+}