diff --git a/src-tauri/src/sidecar_discovery.rs b/src-tauri/src/sidecar_discovery.rs index 6dca01b44..0d9503933 100644 --- a/src-tauri/src/sidecar_discovery.rs +++ b/src-tauri/src/sidecar_discovery.rs @@ -154,7 +154,16 @@ pub(super) fn find_coven() -> Option { #[cfg(target_os = "windows")] { let home = std::env::var("USERPROFILE").unwrap_or_default(); + // npm global installs land in %APPDATA%\\npm as a .cmd shim (not .exe), + // which is the most common way users install the Coven CLI. A GUI-launched + // Tauri app frequently does not inherit that dir on PATH, so we must probe + // it explicitly or `where.exe` below will miss it. Fall back to + // %USERPROFILE%\\AppData\\Roaming\\npm when APPDATA is unset. + let appdata = std::env::var("APPDATA") + .unwrap_or_else(|_| format!("{}\\AppData\\Roaming", home)); let candidates = [ + PathBuf::from(format!("{}\\npm\\coven.cmd", appdata)), + PathBuf::from(format!("{}\\npm\\coven.exe", appdata)), PathBuf::from(format!("{}\\.volta\\bin\\coven.exe", home)), PathBuf::from(format!("{}\\.bun\\bin\\coven.exe", home)), PathBuf::from(format!("{}\\.cargo\\bin\\coven.exe", home)), diff --git a/src/lib/server/assist-runner.test.ts b/src/lib/server/assist-runner.test.ts index 778128da4..6a6f944e0 100644 --- a/src/lib/server/assist-runner.test.ts +++ b/src/lib/server/assist-runner.test.ts @@ -1,6 +1,11 @@ // @ts-nocheck import assert from "node:assert/strict"; -import { buildAssistInvocation, ASSIST_TIMEOUT_MS } from "./assist-runner.ts"; +import { + buildAssistInvocation, + describeEmptyAssistOutput, + stderrReason, + ASSIST_TIMEOUT_MS, +} from "./assist-runner.ts"; const prevBin = process.env.COVEN_CODEX_BIN; @@ -36,6 +41,50 @@ try { // The default budget matches the sew's historical bound. assert.equal(ASSIST_TIMEOUT_MS, 180_000); + // ── stderr reason collapsing ─────────────────────────────────────────────── + assert.equal(stderrReason(""), ""); + assert.equal(stderrReason(" \n\t\n"), "", "whitespace-only tail collapses to nothing"); + assert.equal( + stderrReason("line1\r\nline2\nline3\n\nline4\n"), + "line2 · line3 · line4", + "keeps the last 3 non-empty lines across CRLF", + ); + assert.equal(stderrReason("x".repeat(400)), "x".repeat(300), "caps at the final 300 chars"); + + // ── empty-output diagnostics ─────────────────────────────────────────────── + // Auth-shaped stderr → explicit sign-in guidance with a runnable command. + let msg = describeEmptyAssistOutput( + "codex", + "ERROR: Not logged in. Run `codex login` to authenticate.", + ); + assert.ok(msg.includes("isn't signed in"), msg); + assert.ok(msg.includes("`codex login`"), msg); + assert.ok(msg.includes("Not logged in"), "carries the stderr tail"); + + msg = describeEmptyAssistOutput("codex", "request failed: 401 Unauthorized"); + assert.ok(msg.includes("`codex login`"), msg); + + // A COVEN_CODEX_BIN path with spaces still yields a runnable login command, + // with backslashes escaped ahead of quotes (js/incomplete-sanitization). + msg = describeEmptyAssistOutput("C:\\Program Files\\codex.exe", "stream error: unauthorized"); + assert.ok(msg.includes('`"C:\\\\Program Files\\\\codex.exe" login`'), msg); + + // Non-auth stderr must NOT claim a sign-in problem — loose `token` / `auth` + // / `sign ?in` substring matching used to misfire on lines like these. + for (const stderr of [ + "SyntaxError: Unexpected token < in JSON at position 0", + "error: not inside a trusted directory; pass --skip-git-repo-check", + "warning: redesign in progress for the author page catalog index", + ]) { + msg = describeEmptyAssistOutput("codex", stderr); + assert.ok(!msg.includes("signed in"), `false auth positive for: ${stderr}`); + assert.ok(msg.startsWith("assist produced no output"), msg); + assert.ok(msg.includes(stderrReason(stderr)), "still surfaces the stderr tail"); + } + + // No stderr at all → the original terse message, no dangling separator. + assert.equal(describeEmptyAssistOutput("codex", ""), "assist produced no output"); + console.log("assist-runner.test.ts OK"); } finally { if (prevBin === undefined) delete process.env.COVEN_CODEX_BIN; diff --git a/src/lib/server/assist-runner.ts b/src/lib/server/assist-runner.ts index abda387a6..96f0b667c 100644 --- a/src/lib/server/assist-runner.ts +++ b/src/lib/server/assist-runner.ts @@ -57,11 +57,52 @@ export type AssistRunResult = | { ok: true; lastMessage: string } | { ok: false; error: string }; +/** + * Pure: collapse a bounded stderr capture into a one-line reason suitable + * for appending to an error message — last 3 non-empty lines, joined, capped + * at the final 300 chars. Unit-tested. + */ +export function stderrReason(stderrTail: string): string { + return stderrTail.trim().split(/\r?\n/).filter(Boolean).slice(-3).join(" · ").slice(-300); +} + +// Signals that codex exited without output because it isn't signed in. Kept +// deliberately precise: loose substrings like `auth`, `token`, or `sign ?in` +// would misread "unexpected token" / "author" / "design in…" stderr as a +// sign-in problem and send the user to `codex login` for an unrelated +// failure. A missed match is benign — the generic path below still surfaces +// the stderr tail verbatim. +const AUTH_STDERR_RE = + /not (?:logged|signed) in|codex login|please (?:log|sign) ?in|unauthorized|authentication (?:required|failed|error)|(?:invalid|missing|expired) (?:credentials?|api key)|credentials? (?:invalid|missing|expired)|token (?:expired|revoked)|\b401\b/i; + +/** + * Pure: error message for the "exited 0 but wrote no last message" case. + * codex does this when it is not signed in, leaving the real reason on + * stderr — so surface the stderr tail, plus an explicit sign-in hint (with a + * runnable login command) when that tail looks like an auth failure. + * Unit-tested. + */ +export function describeEmptyAssistOutput(command: string, stderrTail: string): string { + const tail = stderrReason(stderrTail); + if (AUTH_STDERR_RE.test(stderrTail)) { + const loginCommand = /\s/.test(command) + ? // Escape backslashes before quotes so a Windows path (C:\…\coven.cmd) + // can't break out of the double-quoted suggestion (js/incomplete-sanitization). + `"${command.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}" login` + : `${command} login`; + return `${command} isn't signed in, so this assist produced no output. Run \`${loginCommand}\` in a terminal, then try again${ + tail ? ` (${tail})` : "" + }.`; + } + return `assist produced no output${tail ? ` — ${tail}` : ""}`; +} + /** * Run one bounded assist end-to-end: spawn `codex exec`, wait (bounded), and * return the final message for the caller to parse against its own output * contract. Never throws. Spawn behavior is exercised manually — CI covers - * `buildAssistInvocation` only. + * the pure pieces only (`buildAssistInvocation`, `stderrReason`, + * `describeEmptyAssistOutput`). */ export async function runBoundedAssist(opts: { prompt: string; @@ -140,7 +181,7 @@ export async function runBoundedAssist(opts: { }); if (spawned.error) return { ok: false, error: spawned.error }; if (spawned.code !== 0) { - const reason = stderrTail.trim().split(/\r?\n/).filter(Boolean).slice(-3).join(" · ").slice(-300); + const reason = stderrReason(stderrTail); return { ok: false, error: `codex exec exited with ${spawned.code}${reason ? ` — ${reason}` : ""}`, @@ -150,7 +191,11 @@ export async function runBoundedAssist(opts: { try { lastMessage = await readFile(/* turbopackIgnore: true */ lastMessagePath, "utf8"); } catch { - return { ok: false, error: "assist produced no output" }; + // codex can exit 0 yet write no last-message file when it is not signed + // in. The real reason lands on stderr, so surface that (and an explicit + // sign-in hint for the auth case) instead of an opaque "produced no + // output". + return { ok: false, error: describeEmptyAssistOutput(inv.command, stderrTail) }; } return { ok: true, lastMessage }; } catch (err) {