From e97be11a0c6922a3dd8ca39142246cf970b200ef Mon Sep 17 00:00:00 2001
From: Chris Wanstrath <chris@ozmm.org>
Date: Tue, 10 Mar 2026 10:03:17 -0700
Subject: [PATCH] Fix setup env precedence and validate directives

---
 src/cli/index.ts | 61 +++++++++++++++++++++++++++++++-----------------
 src/parse.ts     |  5 ++--
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/src/cli/index.ts b/src/cli/index.ts
index 292dbee..d25b178 100755
--- a/src/cli/index.ts
+++ b/src/cli/index.ts
@@ -90,37 +90,46 @@ $ true
     const results: TestResult[] = []
     const cwd = process.cwd()
     const portFrom = opts.portFrom ? parseInt(opts.portFrom, 10) : undefined
+    if (portFrom !== undefined && Number.isNaN(portFrom)) {
+      console.error("--port-from must be an integer")
+      process.exit(1)
+    }
     let nextPort = portFrom ?? 0
 
-    const runOne = async (filePath: string, port?: number) => {
+    const runOne = async (filePath: string) => {
       const content = await readFile(filePath, "utf-8")
       const parsed = parse(relative(cwd, filePath), content)
 
-      // Collect env vars: --port-from, then @env directives
+      // Resolve directives in a single pass. Setup @env is collected separately
+      // so that the user file's @env always takes precedence.
       const envVars: Record<string, string> = {}
-      if (port !== undefined) envVars["PORT"] = String(port)
-      for (const d of parsed.directives) {
-        if (d.type === "env") envVars[d.key] = d.value
-      }
-
-      // Resolve @setup directives: prepend setup commands, collect their @env
+      if (portFrom !== undefined) envVars["PORT"] = String(nextPort++)
+      const setupEnvVars: Record<string, string> = {}
+      const userEnvVars: Record<string, string> = {}
       const setupCommands: Command[] = []
       for (const d of parsed.directives) {
-        if (d.type !== "setup") continue
-        const setupPath = resolve(dirname(filePath), d.path)
-        const setupContent = await readFile(setupPath, "utf-8")
-        const setupParsed = parse(relative(cwd, setupPath), setupContent)
-        for (const sd of setupParsed.directives) {
-          if (sd.type === "env") envVars[sd.key] = sd.value
+        if (d.type === "setup") {
+          const setupPath = resolve(dirname(filePath), d.path)
+          const setupContent = await readFile(setupPath, "utf-8")
+          const setupParsed = parse(relative(cwd, setupPath), setupContent)
+          for (const sd of setupParsed.directives) {
+            if (sd.type === "setup") {
+              throw new Error(`${relative(cwd, setupPath)}: @setup not allowed in setup files`)
+            }
+            if (sd.type === "env") setupEnvVars[sd.key] = sd.value
+          }
+          setupCommands.push(...setupParsed.commands)
+        } else if (d.type === "env") {
+          userEnvVars[d.key] = d.value
         }
-        setupCommands.push(...setupParsed.commands)
       }
+      Object.assign(envVars, setupEnvVars, userEnvVars)
       const merged: ShoutFile = { ...parsed, commands: [...setupCommands, ...parsed.commands] }
 
       const fileResult = await runFile(merged, {
         cleanEnv: opts.cleanEnv ?? false,
         pathDirs: opts.path,
-        envVars: Object.keys(envVars).length > 0 ? envVars : undefined,
+        envVars,
         timeout: timeoutMs,
         verbose: opts.verbose ?? false,
         onCommand: opts.verbose
@@ -128,6 +137,18 @@ $ true
           : undefined,
       })
 
+      // Check setup commands for failures
+      for (let i = 0; i < setupCommands.length; i++) {
+        const r = fileResult.results[i]
+        if (r && r.exitCode !== 0) {
+          return evaluateFile(
+            parsed.path,
+            [],
+            `setup command failed (exit ${r.exitCode}): $ ${setupCommands[i]!.command}`,
+          )
+        }
+      }
+
       const fileOwnResults = fileResult.results.slice(setupCommands.length)
 
       const testResult = evaluateFile(
@@ -167,10 +188,7 @@ $ true
     }
 
     if (opts.parallel) {
-      const tasks = files.map(f => {
-        const port = portFrom !== undefined ? nextPort++ : undefined
-        return runOne(f, port)
-      })
+      const tasks = files.map(f => runOne(f))
       const all = await Promise.all(tasks)
       for (const r of all) {
         printDots(r)
@@ -179,8 +197,7 @@ $ true
       process.stdout.write("\n")
     } else {
       for (const filePath of files) {
-        const port = portFrom !== undefined ? nextPort++ : undefined
-        const r = await runOne(filePath, port)
+        const r = await runOne(filePath)
         printDots(r)
         results.push(r)
       }
diff --git a/src/parse.ts b/src/parse.ts
index 4bd07a4..615204a 100644
--- a/src/parse.ts
+++ b/src/parse.ts
@@ -76,9 +76,10 @@ export function parse(path: string, content: string): ShoutFile {
       } else if (line.startsWith("@env ")) {
         const rest = line.slice(5).trim()
         const eq = rest.indexOf("=")
-        if (eq > 0) {
-          directives.push({ type: "env", key: rest.slice(0, eq), value: rest.slice(eq + 1), line: i + 1 })
+        if (eq <= 0) {
+          throw new Error(`${path}:${i + 1}: malformed @env directive (expected KEY=VALUE): ${line}`)
         }
+        directives.push({ type: "env", key: rest.slice(0, eq), value: rest.slice(eq + 1), line: i + 1 })
       } else {
         throw new Error(`${path}:${i + 1}: unknown directive: ${line}`)
       }