fix(dlx): don't mislabel ambiguous Coana failures as launcher failures

mtorp · mtorp · commit 6ac680ff573b · 2026-06-05T08:37:10.000+02:00
Bugbot flagged that with stdio:'inherit' the spawn rejection carries no captured output, so coanaBannerSeen is always false and a Coana process that started then died by signal / exit >= 128 (e.g. OOM, exit 137) was wrongly reported as "the launcher exited before Coana started". Empirically, `coana manifest gradle` writes all output to stdout and never prints the "Coana CLI version" banner, so banner-based detection never worked for the manifest / reachability (inherit) paths anyway. Only a spawn-level error (a string `code` like ENOENT) definitively proves the launcher never started. - buildDlxErrorResult now claims a launch failure ONLY for a spawn-level error; signals and non-zero exits get neutral wording ("Coana failed to run via the package manager (exit code N)") since we cannot tell launcher-vs-Coana apart without captured output. - Soften the fallback warning to drop the "before Coana started" claim (it fires on the ambiguous >= 128 / signal cases too). The npm-install + node fallback gating is unchanged (pre-existing #1327 behavior): it still retries on spawn errors / signals / exit >= 128. Fully suppressing a retry when Coana actually ran would require capturing the launcher output, which conflicts with live streaming (and the reachability spinner); left as a possible follow-up. Update tests for the definitive (ENOENT) vs ambiguous (signal / >= 128) split.
diff --git a/src/utils/dlx.mts b/src/utils/dlx.mts
@@ -502,7 +502,7 @@ export async function spawnCoanaDlx(
     }
 
     logger.warn(
-      'Coana dlx invocation failed before Coana started; falling back to `npm install` + `node`.',
+      'Coana dlx invocation failed; retrying via `npm install` + `node`.',
     )
 
     const fallbackResult = await spawnCoanaViaNpmInstall(
@@ -618,14 +618,18 @@ function buildDlxErrorResult(e: unknown): CResult<string> {
   // Be honest about WHERE the failure happened. On the dlx path the spawned
   // process is the package-manager launcher (npx / pnpm dlx / yarn dlx), which
   // downloads @coana-tech/cli and only then runs it — so a failure may be the
-  // launcher dying before Coana ever started (e.g. the package failed to
-  // download), not Coana itself. Asserting "Coana command failed" in that case
-  // is misleading.
+  // launcher dying before Coana ever started, not Coana itself. We can only be
+  // CERTAIN of that for a spawn-level error (a string `code` like ENOENT: the
+  // launcher binary could not start, so Coana provably never ran). A non-zero
+  // exit or signal is genuinely ambiguous — Coana may have started, streamed
+  // output, and then died (e.g. OOM), or the launcher may have failed to fetch
+  // the package — and with inherited stdio there is no captured output to tell
+  // them apart, so we must not assert either way.
   let message: string
   if (coanaBannerSeen(e)) {
     message = `Coana command failed${detailSuffix}: ${detail}`
-  } else if (dlxLauncherFailedBeforeCoana(e)) {
-    message = `Failed to launch Coana via the package manager${detailSuffix} — the npx/pnpm-dlx/yarn-dlx launcher exited before Coana started (the package may have failed to download): ${detail}`
+  } else if (typeof (e as any)?.code === 'string') {
+    message = `Failed to launch Coana via the package manager${detailSuffix} — the npx/pnpm-dlx/yarn-dlx launcher could not start (e.g. it is missing from PATH): ${detail}`
   } else {
     message = `Coana failed to run via the package manager${detailSuffix}: ${detail}`
   }
diff --git a/src/utils/dlx.test.mts b/src/utils/dlx.test.mts
@@ -355,7 +355,10 @@ describe('utils/dlx', () => {
       })
 
       expect(result.ok).toBe(false)
-      expect(result.message).toContain('Failed to launch Coana')
+      // exit 249 is ambiguous, so the message stays neutral about launcher-vs-Coana.
+      expect(result.message).toContain(
+        'Coana failed to run via the package manager',
+      )
       // No npm install was attempted.
       const npmInstallCalls = mockSpawn.mock.calls.filter(
         ([cmd, args]) => cmd === 'npm' && (args as string[])[0] === 'install',
@@ -396,7 +399,9 @@ describe('utils/dlx', () => {
       })
 
       expect(result.ok).toBe(false)
-      expect(result.message).toContain('Failed to launch Coana')
+      expect(result.message).toContain(
+        'Coana failed to run via the package manager',
+      )
       expect(result.message).toContain('npx aborted')
       expect(result.message).toContain('npm-install fallback also failed')
       expect(result.message).toContain('registry unreachable')
@@ -426,22 +431,42 @@ describe('utils/dlx', () => {
       expect(npmInstallCalls).toHaveLength(0)
     })
 
-    it('reports a launch failure (not a Coana failure) when the launcher dies before Coana starts', async () => {
-      // npx aborts (exit 249) before Coana boots and there is no Coana banner.
-      // Disable the fallback so the dlx error itself is surfaced.
+    it('reports a definitive launch failure for a spawn-level error (the launcher could not start)', async () => {
+      // ENOENT: the launcher binary (npx) is missing from PATH, so Coana
+      // provably never ran. Disable the fallback so the dlx error is surfaced.
       process.env['SOCKET_CLI_COANA_DISABLE_NPM_FALLBACK'] = '1'
-      setDlxRejection({ code: 249, stderr: 'npx aborted' })
+      setDlxRejection({ code: 'ENOENT' })
 
       const result = await spawnCoanaDlx(['manifest', 'gradle', '.'], 'acme', {
         coanaVersion: nextVersion(),
       })
 
       expect(result.ok).toBe(false)
       expect(result.message).toContain('Failed to launch Coana')
-      expect(result.message).toContain('before Coana started')
+      expect(result.message).toContain('could not start')
       expect(result.message).not.toContain('Coana command failed')
     })
 
+    it('does NOT claim a launch failure for an ambiguous signal/high exit code (Coana may have started)', async () => {
+      // exit 137 (OOM-style) is ambiguous: Coana may have started, streamed
+      // output, and been killed — or the launcher may have failed. The message
+      // must not assert either way. Disable the fallback so it is surfaced.
+      process.env['SOCKET_CLI_COANA_DISABLE_NPM_FALLBACK'] = '1'
+      setDlxRejection({ code: 137 })
+
+      const result = await spawnCoanaDlx(['manifest', 'gradle', '.'], 'acme', {
+        coanaVersion: nextVersion(),
+      })
+
+      expect(result.ok).toBe(false)
+      expect(result.message).toContain('exit code 137')
+      expect(result.message).toContain(
+        'Coana failed to run via the package manager',
+      )
+      expect(result.message).not.toContain('Failed to launch Coana')
+      expect(result.message).not.toContain('before Coana started')
+    })
+
     it('does NOT fall back when captured stderr shows Coana booted', async () => {
       // Coana banner present in stderr → Coana clearly ran, so any subsequent
       // failure is a real Coana issue, not a launcher problem.
