Skip to content

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Jan 30, 2026

Previously, CodexAuth was defined as follows:

pub struct CodexAuth {
pub mode: AuthMode,
pub(crate) api_key: Option<String>,
pub(crate) auth_dot_json: Arc<Mutex<Option<AuthDotJson>>>,
storage: Arc<dyn AuthStorageBackend>,
pub(crate) client: CodexHttpClient,
}

But if you looked at its constructors, we had creation for AuthMode::ApiKey where storage was built using a nonsensical path (PathBuf::new()) and auth_dot_json was None:

fn from_api_key_with_client(api_key: &str, client: CodexHttpClient) -> Self {
Self {
api_key: Some(api_key.to_owned()),
mode: AuthMode::ApiKey,
storage: create_auth_storage(PathBuf::new(), AuthCredentialsStoreMode::File),
auth_dot_json: Arc::new(Mutex::new(None)),
client,
}
}

By comparison, when AuthMode::ChatGPT was used, api_key was always None:

Ok(Self {
api_key: None,
mode: auth_mode,
storage,
auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))),
client,
})

#10012 took things further because it introduced a new ChatgptAuthTokens variant to AuthMode, which is important in when invoking account/login/start via the app server, but most logic internal to the app server should just reason about two AuthMode variants: ApiKey and ChatGPT.

This PR tries to clean things up as follows:

  • LoginAccountParams and AuthMode in codex-rs/app-server-protocol/ both continue to have the ChatgptAuthTokens variant, though it is used exclusively for the on-the-wire messaging.
  • codex-rs/core/src/auth.rs now has its own AuthMode enum, which only has two variants: ApiKey and ChatGPT.
  • CodexAuth has been changed from a struct to an enum. It is a disjoint union where each variant (ApiKey, ChatGpt, and ChatGptAuthTokens) have only the associated fields that make sense for that variant.

Stack created with Sapling. Best reviewed with ReviewStack.

pub enum CodexAuth {
ApiKey(ApiKeyAuth),
ChatGpt(ChatGptAuth),
ChatGptAuthTokens(ChatGptAuthTokens),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally a nit: I'd prefer Chatgpt / ChatgptAuthTokens (lowercase g)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do it for consistency with the surrounding code, but I think we should be consistent throughout.

Based on https://rust-lang.github.io/api-guidelines/naming.html and looking at other things in the standard library (like std::net::TcpStream), using ChatGpt seems like the correct casing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline: will stick with ChatGpt.

@bolinfest bolinfest merged commit 377ab0c into main Jan 30, 2026
56 of 64 checks passed
@bolinfest bolinfest deleted the pr10208 branch January 30, 2026 17:33
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants