fix(windows): resolve npm-global coven CLI + surface codex auth failures#3747
Conversation
find_coven() only probed coven.exe under .volta/.bun/.cargo and then fell back to where.exe. npm global installs land at %APPDATA%\npm\coven.cmd, and a GUI-launched Tauri app does not inherit %APPDATA%\npm on PATH, so where.exe also missed it -> 'Coven CLI not found on PATH' despite a healthy install. Probe the npm global .cmd/.exe shim explicitly, first, with an APPDATA fallback. Reported by a user on Windows who had also historically run an older Cave build; the fresh dev box did not repro because its coven resolves via a different install path.
codex can exit 0 yet write no last-message file when it is not signed in (or refuses workspace trust). The runner already keeps a bounded stderr tail; on the empty-output path we now include that tail and, when it looks like an auth failure, return an explicit 'run `codex login`' message instead of the opaque 'assist produced no output'. Pairs with the Windows npm-CLI resolution fix; both surfaced from the same user report (Windows, historically ran an older Cave build).
There was a problem hiding this comment.
Pull request overview
Improves Windows-side reliability and debuggability for two user-facing failure modes: sidecar discovery of the Coven CLI when installed via npm globals, and Codex-based assist runs that exit successfully but produce no output due to auth/trust issues.
Changes:
- Add explicit Windows probing of
%APPDATA%\npm\coven.cmd/coven.exebefore falling back towhere.exe. - Enhance
assist produced no outputhandling to surface a bounded stderr tail and provide an explicitcodex loginhint when stderr suggests auth/trust issues.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lib/server/assist-runner.ts | Improves empty-output error reporting by including stderr context and a login hint for auth-like failures. |
| src-tauri/src/sidecar_discovery.rs | Expands Windows CLI discovery to include npm global install locations not present in GUI PATH. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL js/incomplete-sanitization (high): the sign-in suggestion escaped double quotes but not backslashes, so a Windows coven path (C:\…\coven.cmd — exactly the case this PR adds discovery for) could break out of the double-quoted command. Escape backslashes first, then quotes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uth heuristic Review hardening for the empty-output branch added in this PR: - Extract the classification into pure, exported helpers (stderrReason, describeEmptyAssistOutput) per this module's stated stance — pure logic unit-tested, spawn verified manually — and dedupe the tail formatting shared with the non-zero-exit path. - Tighten the auth heuristic: the previous pattern matched bare 'auth', 'token', 'log ?in', and 'sign ?in' substrings, so stderr like 'Unexpected token < in JSON' or 'redesign in progress' would falsely tell users to run 'codex login'. Precision over recall is safe here: a missed auth match still surfaces the stderr tail verbatim. - Cover both branches (auth phrasing, runnable quoted login command for paths with spaces, false-positive regressions, tail carry-through, empty-stderr terseness) in assist-runner.test.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BunsDev
left a comment
BunsDev
left a comment
There was a problem hiding this comment.
Review (Copilot CLI session):
Rust (find_coven) — correct and safe. Verified the consumer (sidecar_startup.rs:200-208) only prepends the found file's parent dir to the sidecar PATH, so discovering a .cmd shim (not directly spawnable by CreateProcess) is semantically right: the Node side resolves it via covenBin() → where → covenLaunchCommandForBinary() (shim → node <script>). npm-first ordering matches coven-bin.ts's documented preference of the npm-published binary over stale .cargo installs. APPDATA fallback to %USERPROFILE%\AppData\Roaming degrades harmlessly when both are unset.
TS (assist-runner) — direction right, but the auth heuristic matched bare auth/token/log ?in/sign ?in substrings, so e.g. SyntaxError: Unexpected token < in JSON or redesign in progress stderr would falsely instruct codex login. Pushed babe1a3ae:
- extracted pure, exported
stderrReason+describeEmptyAssistOutput(per this module's stance: pure logic unit-tested, spawn manual), deduping tail formatting with the exit≠0 path - tightened the regex to precise auth phrases (missed matches are benign — the tail is still surfaced verbatim; false positives are not)
- folded in
8c15e4e91's CodeQL backslash-escaping fix - added unit tests: auth phrasing, quoted login command for paths with spaces, false-positive regressions, tail carry-through, empty-stderr terseness
Verified: assist-runner.test.ts green, tsc --noEmit clean, eslint clean. Merging once required checks pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Two user-facing errors from a single Windows report (user's CLI was healthy at
C:\Users\Admin\AppData\Roaming\npm\coven.cmd, and had historically run an older Cave build). Neither reproduces on a fresh dev box.1.
Coven CLI not found on PATH—sidecar_discovery.rsfind_coven()only probedcoven.exeunder.volta/.bun/.cargo, then fell back towhere.exe. But npm global installs land at%APPDATA%\npm\coven.cmd(a.cmdshim, not.exe), and a GUI-launched Tauri app usually does not inherit%APPDATA%\npmon PATH — sowhere.exemissed it too. Result: "not found on PATH" despite a healthy install.Fix: probe
%APPDATA%\npm\coven.cmd/.exefirst (with anAPPDATAfallback), before the existing candidates.2.
assist produced no output—assist-runner.tscodex can exit
0yet write no last-message file when it is not signed in (or refuses workspace trust). The runner already keeps a bounded stderr tail, but the empty-output path discarded it and returned an opaque message.Fix: on empty output, include the stderr tail and — when it looks like auth (
not logged in / unauthorized / 401 / …) — return an explicit "run `codex login`" message.Verification
cargo check— clean (Rust)tsc --noEmit— clean (TS)Out of scope (deliberately)
The reporter had historically run an older Cave build, so stale sidecar runtimes are possible.
cleanup_stale_sidecar_runtimesonly runs after a successful start, so a wedged box never sweeps them. However, the existing code intentionally leaves complete generations to explicit-uninstall reclamation (no startup-time lease proof that another app isn't serving them); only incomplete extractions are cleared. Splitting the sweep (early incomplete-only) is a real change to a carefully-reasoned concurrency path and belongs in its own PR with sidecar-lease-owner review.