diff --git a/changelog.d/fixes/12645-auggie-cli-not-found-shell-exit.md b/changelog.d/fixes/12645-auggie-cli-not-found-shell-exit.md new file mode 100644 index 0000000000..f322ac43d9 --- /dev/null +++ b/changelog.d/fixes/12645-auggie-cli-not-found-shell-exit.md @@ -0,0 +1 @@ +- fix(sse): surface the actionable "Auggie CLI not found" message when the shell reports a missing `auggie` binary via exit code instead of a spawn error (#12645) diff --git a/open-sse/executors/auggie.ts b/open-sse/executors/auggie.ts index 6443f73162..076b42f683 100644 --- a/open-sse/executors/auggie.ts +++ b/open-sse/executors/auggie.ts @@ -294,6 +294,24 @@ function isEnoentLike(message: string): boolean { return message.includes("ENOENT") || message.includes("not found"); } +// Windows cmd.exe and POSIX shells never raise a Node `spawn` 'error' event for a +// missing binary when `shell: true` is used (see buildAuggieSpawnOptions) — they +// report it as a normal non-zero exit with the "not found" text on stderr instead. +// Recognize that shape too so the `close` handlers give the same actionable +// cliNotFoundMessage() as the `error` handlers already do. See #12645. +const CLI_NOT_FOUND_STDERR_PATTERNS = [ + /is not recognized as an internal or external command/i, + /command not found/i, + // dash/POSIX `sh` shells report a missing executable as `: not found` + // (no literal "command"), e.g. "sh: 1: auggie: not found". + /:\s*not found\s*$/im, + /No such file or directory/i, +]; + +function isCliNotFoundText(stderrTail: string): boolean { + return CLI_NOT_FOUND_STDERR_PATTERNS.some((pattern) => pattern.test(stderrTail)); +} + export type AuggieCliVersionCheck = { ok: boolean; version?: string; error?: string }; /** @@ -580,9 +598,11 @@ export class AuggieExecutor extends BaseExecutor { if (finished) return; if (code !== 0) { emitError( - sanitizeErrorMessage( - `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}` - ) + isCliNotFoundText(stderrTail) + ? cliNotFoundMessage(auggieBin) + : sanitizeErrorMessage( + `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}` + ) ); return; } @@ -664,9 +684,11 @@ export class AuggieExecutor extends BaseExecutor { if (code !== 0) { settle( buildAuggieErrorResponse( - sanitizeErrorMessage( - `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}` - ) + isCliNotFoundText(stderrTail) + ? cliNotFoundMessage(auggieBin) + : sanitizeErrorMessage( + `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}` + ) ) ); return; diff --git a/tests/unit/auggie-cli-not-found-shell-exit-12645.test.ts b/tests/unit/auggie-cli-not-found-shell-exit-12645.test.ts new file mode 100644 index 0000000000..4133fd198a --- /dev/null +++ b/tests/unit/auggie-cli-not-found-shell-exit-12645.test.ts @@ -0,0 +1,103 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import type { ExecuteInput } from "@omniroute/open-sse/executors/base"; + +const { AuggieExecutor, __resetAuggieModels } = await import( + "@omniroute/open-sse/executors/auggie" +); + +function makeFakeAuggieBin(dir: string, stderrLine: string): string { + const fakeBin = path.join(dir, "fake-auggie.sh"); + fs.writeFileSync(fakeBin, `#!/bin/sh\necho "${stderrLine}" 1>&2\nexit 1\n`); + fs.chmodSync(fakeBin, 0o755); + return fakeBin; +} + +async function withFakeAuggieBin( + stderrLine: string, + fn: (dir: string) => Promise +): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "auggie-probe-")); + const fakeBin = makeFakeAuggieBin(dir, stderrLine); + const prevBin = process.env.AUGGIE_BIN; + process.env.AUGGIE_BIN = fakeBin; + __resetAuggieModels(); + try { + return await fn(dir); + } finally { + if (prevBin === undefined) delete process.env.AUGGIE_BIN; + else process.env.AUGGIE_BIN = prevBin; + __resetAuggieModels(); + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +test("Auggie CLI-not-found surfaced via shell exit code (non-streaming) gets the actionable cliNotFoundMessage", async () => { + await withFakeAuggieBin( + "'auggie' is not recognized as an internal or external command,", + async () => { + const executor = new AuggieExecutor(); + const { response } = await executor.execute({ + model: "", + body: { messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: {} as never, + } satisfies ExecuteInput); + + const json = await response.json(); + const message: string = json?.error?.message ?? ""; + + // sanitizeErrorMessage() redacts the absolute bin path (and anything the + // path-redaction tokenizer folds into it) — see errorPathRedaction.ts — + // so the assertion mirrors the existing precedent in + // auggie-executor.test.ts: assert the actionable prefix routed through + // cliNotFoundMessage(), and that the raw, confusing shell text from + // #12645 is gone. + assert.match( + message, + /Auggie CLI not found/, + `expected the actionable 'Auggie CLI not found' message, but got: ${message}` + ); + assert.doesNotMatch( + message, + /is not recognized as an internal or external command/i, + `expected the raw shell text to be replaced, but got: ${message}` + ); + } + ); +}); + +test("Auggie CLI-not-found surfaced via shell exit code (streaming) gets the actionable cliNotFoundMessage", async () => { + await withFakeAuggieBin("sh: 1: auggie: not found", async () => { + const executor = new AuggieExecutor(); + const { response } = await executor.execute({ + model: "", + body: { messages: [{ role: "user", content: "hi" }] }, + stream: true, + credentials: {} as never, + } satisfies ExecuteInput); + + const text = await response.text(); + const dataLine = text + .split("\n") + .find((line) => line.startsWith("data: ") && line.includes('"error"')); + assert.ok(dataLine, `expected an SSE error frame, got body: ${text}`); + const payload = JSON.parse(dataLine!.slice("data: ".length)); + const message: string = payload?.error?.message ?? ""; + + assert.match( + message, + /Auggie CLI not found/, + `expected the actionable 'Auggie CLI not found' message, but got: ${message}` + ); + assert.doesNotMatch( + message, + /exited with code/i, + `expected the raw shell exit-code text to be replaced, but got: ${message}` + ); + }); +});