fix(cli): exit non-zero from whoami and login when unauthenticated - #3654
Conversation
`veryfront whoami` printed "✗ Not logged in" and exited 0. The exit code is the machine-readable contract: CI steps, shell scripts, and agents gate on `veryfront whoami` to confirm auth, believe they are authenticated, and then fail later somewhere unrelated and much harder to diagnose. The router awaited `whoami()` and discarded its return value, so the null "no usable credential" result never reached the process exit code. `login` had the same shape — it returns null when it cannot obtain a credential and the router dropped that too. Both now exit 1. This repo's CLI reserves 2 for invalid usage (see the exit code table in the CLI output style guide), so "no usable credential" is a general failure, code 1. The human-readable output is unchanged: it still names the problem and points at `veryfront login`. `--json` still emits the `authenticated: false` envelope, now alongside a non-zero exit. Covered by subprocess tests that drive the real CLI entry point and assert on the process exit code itself, including a positive case behind a stub API so the fix cannot degenerate into always exiting 1.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe CLI now returns status 1 for failed ChangesAuthentication exit statuses
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change makes unauthenticated Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fe4ab85b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… opt-outs Two follow-ups on the same contract. `login --provider anthropic|openai` had the exact defect this PR set out to fix: both provider functions return Promise<boolean> and the router discarded the result before reaching the exit guard, so an empty key, an invalid key, or a validation network error still exited 0 — contradicting the help text this PR added. Both branches now exit 1, so every login shape reports failure the same way. The new integration suite also carried sanitizeOps/sanitizeResources: false, which pushed the sanitizer ratchet from 404 to 406 and failed lint. The suite leaks nothing — every subprocess is awaited to completion, the stub server is shut down, and the temp dirs are removed — so the opt-outs are deleted rather than the baseline raised. The suite passes with both sanitizers enabled. The provider branches are deliberately not covered by a subprocess test: promptPassword calls Deno.stdin.setRaw(), which throws ENODEV on a non-TTY stdin, so such a test would exit 1 from that crash rather than from the failure path and would pass with this fix reverted. Noted in the test file.
1 participant
The lie
veryfront whoamiprints✗ Not logged inand exits 0.Reproduced against the published npm artifact
veryfront@0.1.1232, in a sandbox outside the monorepo, with the exit code captured directly (no pipe, which would mask it):The exit code is the machine-readable contract. Any CI step, shell script, or agent that gates on
veryfront whoamibelieves it is authenticated when it is not, then fails later somewhere unrelated and much harder to diagnose. A human usedveryfront whoamiexactly this way — to confirm auth before starting a task — during the dogfood session that found this.Sibling check
Swept the neighbouring commands for the same class of defect:
whoamilogin(non-interactive, no token)logoutdoctorfailcheck throws and exits 1; warnings exit 0 unless--strictroutes(no route dirs)loginis the same shape aswhoamiand lives in the same router entry group and auth module, so it is fixed here.doctor's warning-exits-0 behaviour is deliberate and opt-in via--strict; left alone.Cause
cli/router.tsawaited the auth functions and discarded their return values:whoami()already reports "no usable credential" by returningnull. That signal simply never reached the process exit code. Same forlogin().Fix
Both handlers now exit 1 when no credential was obtained.
Exit code: 1, not 2. The CLI output style guide's exit-code table reserves
2for invalid usage (wrong args, missing required) and1for a general error;130is interrupt.veryfront whoamiwith no credential is well-formed usage that answers "no", so it is 1. This also matches the precedent of tools likegrepandgit diff --quiet, where 1 means "the answer is no" rather than "you called me wrong".Human-readable output is unchanged — it still names the problem plainly and points at
veryfront login.--jsonstill emits theauthenticated: falseenvelope, now alongside the non-zero exit. Bothwhoamiandloginhelp text now document the exit codes.Tests
cli/auth/exit-code.integration.test.tsdrives the real CLI entry point in a subprocess and asserts on the process exit code itself — the thing that was wrong. Each run gets a throwawayXDG_CONFIG_HOMEand a temp cwd so neither a developer's stored token nor a repository.envcan leak in.Red, before the fix:
Green, after:
The fourth case points the CLI at a stub
/meserver and asserts exit 0 plus the identity in the output, so the fix cannot degenerate into always exiting 1.Also run clean:
deno fmt --check,deno lint,deno checkon the changed files, anddeno test cli/router.test.ts cli/auth/ cli/help/(17 passed, 226 steps).Summary by CodeRabbit
loginreturns exit code 1 when authentication fails.whoamireturns exit code 1 when no valid credentials are available and 0 when authentication succeeds.