feat(auth): headless login URL + import-codex + TUI#92
Conversation
- auth-code flow now prints the full authorization URL to stderr on the CLI path so users on headless machines (no DISPLAY, xdg-open unavailable) can copy-paste it when open::that fails. Mirrors the device-code flow's existing UX. - New `byokey import-codex` subcommand mirroring import-claude-code: reads ~/.codex/auth.json on every platform (Codex CLI doesn't use Keychain), decodes the access_token JWT for `exp`, saves under account `codex-cli`. Supports both chatgpt-mode OAuth tokens and api_key-mode raw keys. - New byokey-tui crate (ratatui + crossterm) wired in as `byokey tui`. - README install-script section + install.sh entry point.
![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: 9bb27fcf6b
ℹ️ 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".
| .to_string(); | ||
| let label = label.unwrap_or_else(|| "Codex CLI".to_string()); | ||
| self.auth | ||
| .save_token_for(&provider, &account_id, Some(label.as_str()), token) |
There was a problem hiding this comment.
When the default codex-cli import is the first Codex account, it becomes the active account, but expired active tokens are refreshed through AuthManager::refresh_token, which persists the refreshed token with store.save(provider, ...) (the default account) rather than this codex-cli account. After the first expiry, one request may get a fresh token in memory, but the active imported account remains expired, causing subsequent requests to hit the refresh cooldown/retry refresh forever instead of using the refreshed credentials.
Useful? React with 👍 / 👎.
| return Ok(Some(OAuthToken { | ||
| access_token: key, | ||
| refresh_token: None, | ||
| expires_at: None, | ||
| token_type: Some("api-key".to_string()), |
There was a problem hiding this comment.
For Codex CLI API-key mode, this stores the raw OPENAI_API_KEY in the token store, but CodexExecutor::token() treats every AuthManager token as OAuth (is_oauth = true) and sends it to the Codex Responses endpoint instead of the OpenAI Chat Completions API-key path. Users whose ~/.codex/auth.json contains only OPENAI_API_KEY will see import-codex succeed but proxied Codex requests will be authenticated/routed incorrectly; either reject this mode or teach the executor to honor token_type == "api-key".
Useful? React with 👍 / 👎.
- TokenStore::save now resolves the active account_id (from the DB for SqliteTokenStore, the in-memory map for InMemoryTokenStore) instead of hardcoding "default". The old behavior wrote refreshed tokens to a non-active "default" row that load() never read, stranding any provider whose active account had a different name (e.g. `codex-cli` from `import-codex`, or `claude-code` from `import-claude-code`) in a perpetual refresh loop. Adds regression tests in both stores. - import-codex now rejects API-key-mode auth.json with a clear error pointing the user at `byokey add-api-key codex <sk-...>`. byokey's Codex executor targets the ChatGPT-mode Responses endpoint and would misroute a raw OpenAI API key. - Bump rustls-webpki 0.103.12 → 0.103.13 (RUSTSEC-2026-0104, reachable panic in CRL parsing). - Apply cargo fmt to codex_cli.rs and import_codex.
|
TL;DR — Brings OAuth login parity to headless CLI environments, ships a Key changes
Summary ｜ 23 files ｜ 4 commits ｜ base: Headless-friendly OAuth login
The change is scoped to CLI mode; programmatic callers using the SDK form continue to receive the URL via the existing emit channel without extra stderr noise.
|
| Tab | ManagementClient call |
|---|---|
| Status | get_status → server host:port + per-provider AuthStatus |
| Accounts | list_accounts → per-provider AccountDetail with TokenState |
| Usage | get_usage → totals plus per-model rows |
crates/tui/src/lib.rs · crates/tui/src/app.rs · crates/tui/src/ui.rs · crates/tui/AGENTS.md
Reusable ManagementClient on byokey-proto
Before:
byokey-protoonly emitted generated server stubs; any client of the management API had to wire upconnectrpc::clientmachinery itself.
After: an opt-inclientCargo feature exports aManagementClientthat bundlesStatusService,AccountsService, andAmpServiceclients behind one constructor, plus convenience methods that unwrap to owned response types.
local_http builds a plaintext client at a given http::Uri with a 5s default timeout; with_config / with_transport are escape hatches for callers that need TLS, custom headers, or shared transports. The TUI is the first consumer; the same wrapper is available to any future client (e.g. a desktop app or scripting tool) without re-implementing transport setup.
crates/proto/src/client.rs · crates/proto/src/lib.rs · crates/proto/Cargo.toml
TokenStore::save() now overwrites the active account
Before: both
InMemoryTokenStore::saveandSqliteTokenStore::savehardcodedaccount_id = "default", so when the active account was named otherwise (e.g.codex-clifromimport-codex), a refresh wrote a second non-active row thatload()never returned.
After:save()resolves the currently-active account first and writes through to that row, falling back to"default"only when no active account exists yet.
Regression tests save_updates_non_default_active_account were added to both stores to lock in the behavior. This was surfaced by review feedback against the import-codex flow, where the bug would have caused refreshes to silently strand the imported token.
crates/store/src/memory.rs · crates/store/src/persistent/token.rs
install.sh and README install section
Before: binary install was Homebrew-only or
cargo install byokey.
After: a POSIX-shinstaller at the repo root fetches the latest release archive for the currentos/arch, dropsbyokeyinto$HOME/.byokey/bin(orBYOKEY_INSTALL_DIR), and prints aPATHhint when the install dir is not already onPATH. README anddocs/README_CN.mddocument the one-liner alongside the newbyokey tuientry.
The script detects platform via uname, supports BYOKEY_VERSION for pinning, and err-exits with an actionable message if the resolved release lacks a build for the detected target.
install.sh · README.md · docs/README_CN.md
Claude Opus ｜ 𝕏
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The new commit 20eccdb is labeled as a clippy/doc-comment fix but actually lands an incomplete crates/tui/src/lib.rs refactor that breaks the build. cargo check -p byokey-tui fails with three errors: byokey_proto and http are unresolved (neither is in crates/tui/Cargo.toml), and App::new is invoked with one argument while crates/tui/src/app.rs:75 still expects (Arc<SqliteTokenStore>, Arc<AuthManager>). src/main.rs:254 is also still calling byokey_tui::run(store.db).await against the old Option<PathBuf> signature.
Reviewed changes:
- Replaced direct
SqliteTokenStore+AuthManagerwiring incrates/tui/src/lib.rs::runwith abyokey_proto::client::ManagementClient(incomplete —App, the Cargo manifest, and the CLI caller were not updated). - Backticked
OpenAIin thecodex_cli.rsmodule doc comment.
Claude Opus ｜ 𝕏
| }; | ||
|
|
||
| use app::App; | ||
| use byokey_proto::client::ManagementClient; |
There was a problem hiding this comment.
This import doesn't resolve. byokey-tui's Cargo.toml lists only byokey-types, byokey-store, byokey-auth, byokey-daemon, anyhow, rquest, ratatui, crossterm — byokey-proto isn't a dependency. Even with it added, byokey_proto has no client module: crates/proto/src/lib.rs only include!s the ConnectRPC-generated code, and grepping the workspace finds no ManagementClient type anywhere except this line. cargo check -p byokey-tui fails with E0433: failed to resolve: use of unresolved module or unlinked crate \byokey_proto`. The http::Urion line 31 has the same problem —http` isn't a dependency either.
| /// Returns an error if the terminal cannot be initialized or if the underlying | ||
| /// management API URL is invalid. | ||
| pub async fn run(endpoint: Option<String>) -> Result<()> { | ||
| let mut app = App::new(management_client(endpoint)?); |
There was a problem hiding this comment.
Arity mismatch: App::new in crates/tui/src/app.rs:75 is still pub fn new(store: Arc<SqliteTokenStore>, auth: Arc<AuthManager>) -> Self and uses both fields in refresh(). cargo check reports E0061: this function takes 2 arguments but 1 argument was supplied. Likewise src/main.rs:254 still calls byokey_tui::run(store.db).await against the old Option<PathBuf> parameter, so even after fixing App the binary won't link. Either land the rest of the refactor (new App impl over ManagementClient, updated Cargo.toml, updated CLI caller) in this commit or revert the lib.rs changes back to the SQLite-direct version that was already shipped in 9bb27fc.
There was a problem hiding this comment.
Important
The TUI ConnectRPC refactor flagged in the prior review is now wired end-to-end (deps, App constructor, CLI caller all updated) and the build errors are gone. One small mismatch in crates/tui/src/lib.rs: the bail! message advertises "local http:// URLs only" but the validation only checks the scheme — --url http://anywhere.example.com:8018 is accepted and account labels/IDs and token state are sent there in plaintext.
Reviewed changes:
- Refactored
crates/tuito fetch state throughbyokey_proto::client::ManagementClientinstead of touchingSqliteTokenStore/AuthManager/byokey-daemondirectly. - Added a new
clientcargo feature onbyokey-protoexposingManagementClientover the generatedStatusServiceClient/AccountsServiceClient/AmpServiceClient. - Replaced the old
byokey tui --dbflag with--url <URL>(defaults tohttp://127.0.0.1:8018) insrc/main.rsandbyokey_tui::run. - Documented the new TUI boundary and subcommand in
AGENTS.md,crates/tui/AGENTS.md,README.md, anddocs/README_CN.md.
Prior review feedback:
-
byokey_proto::client::ManagementClientimport unresolvable — addressed via newclientfeature onbyokey-protoplusbyokey-proto/httpdeps incrates/tui/Cargo.toml -
App::newarity mismatch andsrc/main.rscalling oldOption<PathBuf>signature — addressed (App::new(ManagementClient),byokey_tui::run(url))
Claude Opus ｜ 𝕏
| if uri.scheme_str() != Some("http") { | ||
| bail!("TUI management client currently supports local http:// URLs only"); | ||
| } | ||
| if uri.authority().is_none() { | ||
| bail!("management API URL must include host and port"); | ||
| } |
There was a problem hiding this comment.
The error message claims "local http:// URLs only" but only the scheme is checked — --url http://anywhere.example.com:8018 passes validation and the TUI then sends GetStatus / ListAccounts / GetUsage to that host in plaintext, leaking account IDs, labels, token state and usage counters. Either drop "local" from the message or enforce a loopback authority (127.0.0.1 / ::1 / localhost) here.
1 participant
Summary
DISPLAY,xdg-openfailing) can copy-paste it. Device-code flow already did this; this brings parity.byokey import-codex— new subcommand mirroringimport-claude-code. Reads~/.codex/auth.json, decodes theaccess_tokenJWT'sexpclaim, and stores the token under accountcodex-cli. Supports both ChatGPT-mode OAuth and raw-OPENAI_API_KEYmodes. Codex CLI uses the same path on macOS (no Keychain), so a single non-Keychain code path suffices.byokey tui— newbyokey-tuicrate (ratatui + crossterm) wired up as a top-level subcommand.install.sh.Test plan
cargo build --release --bin byokeycleancargo test -p byokey-auth codex_cli— 3 new unit tests pass (chatgpt-mode JSON parse, api_key-mode parse, JWT decode bails on non-JWT)byokey login codex --account dryrunprints the fullhttps://auth.openai.com/oauth/authorize?...URL to stderr before blocking on the callback listenerbyokey import-codexfollowed bybyokey statusreportscodex: authenticated; account list showscodex-cli [Codex CLI] (active)byokey tuiin an interactive terminal