fix(cli): exit cleanly with a re-login hint on an expired/invalid token (#3231)#3240
Open
abhay-codes07 wants to merge 1 commit into
Open
Conversation
…en (omnigent-ai#3231) An expired or invalid access token surfaced as an uncaught OmnigentError (e.g. `{'error_code': 403, 'message': 'Invalid access token...'}`) from `sdk.sessions.create` on the daemon-chat path. It propagated to `main`'s catch-all, which handed it to the friendly crash handler -- so the user got the calm crash screen and a prompt to FILE A GITHUB ISSUE for what is really just their own lapsed credentials. It was also auto-reported by the crash handler. A 401/403 from the server against our own credentials is user-actionable (sign in again), not a bug to report. `main` now recognizes that case and prints a re-login hint pointing at `omnigent login`, then exits 1 without the crash screen or the bug-filing prompt. Every other error still routes to the crash handler unchanged. Detection is a small pure helper `credential_rejection_hint` keyed on the error's HTTP status (the client SDK sets `status_code`; server-side errors set `http_status`), so it does not rely on substring matching a message. The real omnigent-ai#3231 body shape -- a proxy 403 that is not the `{"error": {...}}` envelope, which makes the SDK stringify the whole body as the message -- is covered by reconstructing it through the SDK's own `raise_for_status`. Signed-off-by: abhay-codes07 <abhaysingh0293@gmail.com>
Contributor
|
@abhay-codes07 This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder. These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:
Use |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
Closes #3231
Summary
An expired or invalid access token crashed the CLI. On the daemon-chat path
sdk.sessions.createraised an uncaughtOmnigentError(the auto-reported crash was{'error_code': 403, 'message': 'Invalid access token. [ReqId: ...]'}). It propagated tomain's catch-allexcept Exception, which handed it to the friendly crash handler — so the user got the calm crash screen and a prompt to file a GitHub issue for what is really just their own lapsed credentials, and the crash handler auto-reported it.A 401/403 from the server against our own credentials is user-actionable (sign in again), not a bug to report.
mainnow recognizes that case, prints a re-login hint pointing atomnigent login, and exits 1 — without the crash screen or the bug-filing prompt. Every other error still routes to the crash handler unchanged.Detection is a small pure helper,
credential_rejection_hint, keyed on the error's HTTP status (the client SDK'sOmnigentErrorsetsstatus_code; server-side errors sethttp_status) rather than substring-matching a message. That helper fully captures the routing decision; the glue inmainis three lines that print the hint and exit when it returns non-None.Test Plan
tests/cli/test_credential_rejection_hint.py(new):omnigent loginhttp_status(notstatus_code) → hintNone(falls through to the crash handler)Noneraise_for_status, → recognized as a credential rejectionWithout the change the suite errors at import (the helper does not exist); with it, all 10 pass.
ruff check+ruff format --checkclean.Demo
N/A. CLI error-handling change with no visual surface of its own. Before: an expired token renders the crash screen and prompts the user to file a GitHub issue (and auto-reports it). After:
Authentication failed: the server rejected your credentials (HTTP 403). Your login may have expired — run \omnigent login ` to sign in again.` and a clean exit 1.Type of change
Test coverage
Coverage notes
The helper is pure and covers the full routing decision (which statuses are treated as credential rejections vs. handed to the crash handler), including the real-world #3231 body shape reconstructed via the SDK. The three-line glue in
main(print hint +SystemExit(1)when the helper returns a message) is verified by reading; a fullmain()invocation is intentionally not driven since it installs the crash handler, migrates state, and sets up logging.Changelog
An expired or invalid login now exits with a clear "run
omnigent login" hint instead of showing the crash screen and prompting to file a bug report.