openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-13 11:47:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

cleaning up unnecessary code and exposed API

Browse Source
This commit is contained in:
mikhail-oai
2026-03-19 15:10:16 -04:00
parent 8446a7349e
commit b3b7c81d21
4 changed files with 135 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
codex-rs/keyring-store/src/lib.rs
View File

@@ -8,13 +8,9 @@ use tracing::trace;
mod split_json;
pub use split_json::JsonKeyringError;
pub use split_json::SplitJsonKeyringError;
pub use split_json::delete_json_from_keyring;
pub use split_json::delete_split_json_from_keyring;
pub use split_json::load_json_from_keyring;
pub use split_json::load_split_json_from_keyring;
pub use split_json::save_json_to_keyring;
pub use split_json::save_split_json_to_keyring;
#[derive(Debug)]
pub enum CredentialStoreError {

163
codex-rs/keyring-store/src/split_json.rs
View File

@@ -91,10 +91,7 @@ pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
service: &str,
base_key: &str,
) -> Result<Option<Value>, JsonKeyringError> {
if let Some(value) = load_split_json_from_keyring(keyring_store, service, base_key)? {
return Ok(Some(value));
}
load_full_json_from_keyring(keyring_store, service, base_key)
load_split_json_from_keyring(keyring_store, service, base_key)
}
#[cfg(not(windows))]
@@ -106,7 +103,7 @@ pub fn load_json_from_keyring<K: KeyringStore + ?Sized>(
if let Some(value) = load_full_json_from_keyring(keyring_store, service, base_key)? {
return Ok(Some(value));
}
load_split_json_from_keyring(keyring_store, service, base_key)
Ok(None)
}
#[cfg(windows)]
@@ -117,9 +114,7 @@ pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
value: &Value,
) -> Result<(), JsonKeyringError> {
save_split_json_to_keyring(keyring_store, service, base_key, value)?;
if let Err(err) = delete_full_json_from_keyring(keyring_store, service, base_key) {
warn!("failed to remove stale full JSON record from keyring: {err}");
}
Ok(())
}
@@ -130,11 +125,10 @@ pub fn save_json_to_keyring<K: KeyringStore + ?Sized>(
base_key: &str,
value: &Value,
) -> Result<(), JsonKeyringError> {
save_full_json_to_keyring(keyring_store, service, base_key, value)?;
if let Err(err) = delete_split_json_from_keyring(keyring_store, service, base_key) {
warn!("failed to remove stale split JSON record from keyring: {err}");
}
Ok(())
let bytes = serde_json::to_vec(value).map_err(|err| {
SplitJsonKeyringError::new(format!("failed to serialize JSON record: {err}"))
})?;
save_secret_to_keyring(keyring_store, service, base_key, &bytes, "JSON record")
}
#[cfg(windows)]
@@ -144,8 +138,7 @@ pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
base_key: &str,
) -> Result<bool, JsonKeyringError> {
let split_removed = delete_split_json_from_keyring(keyring_store, service, base_key)?;
let full_removed = delete_full_json_from_keyring(keyring_store, service, base_key)?;
Ok(split_removed || full_removed)
Ok(split_removed)
}
#[cfg(not(windows))]
@@ -155,11 +148,10 @@ pub fn delete_json_from_keyring<K: KeyringStore + ?Sized>(
base_key: &str,
) -> Result<bool, JsonKeyringError> {
let full_removed = delete_full_json_from_keyring(keyring_store, service, base_key)?;
let split_removed = delete_split_json_from_keyring(keyring_store, service, base_key)?;
Ok(full_removed || split_removed)
Ok(full_removed)
}
pub fn load_split_json_from_keyring<K: KeyringStore + ?Sized>(
fn load_split_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
@@ -170,7 +162,7 @@ pub fn load_split_json_from_keyring<K: KeyringStore + ?Sized>(
inflate_split_json(keyring_store, service, base_key, &manifest).map(Some)
}
pub fn save_split_json_to_keyring<K: KeyringStore + ?Sized>(
fn save_split_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
@@ -234,7 +226,7 @@ pub fn save_split_json_to_keyring<K: KeyringStore + ?Sized>(
Ok(())
}
pub fn delete_split_json_from_keyring<K: KeyringStore + ?Sized>(
fn delete_split_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
@@ -288,19 +280,6 @@ fn load_full_json_from_keyring<K: KeyringStore + ?Sized>(
}
}
#[cfg(not(windows))]
fn save_full_json_to_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
base_key: &str,
value: &Value,
) -> Result<(), SplitJsonKeyringError> {
let bytes = serde_json::to_vec(value).map_err(|err| {
SplitJsonKeyringError::new(format!("failed to serialize JSON record: {err}"))
})?;
save_secret_to_keyring(keyring_store, service, base_key, &bytes, "JSON record")
}
fn delete_full_json_from_keyring<K: KeyringStore + ?Sized>(
keyring_store: &K,
service: &str,
@@ -653,7 +632,6 @@ mod tests {
use super::save_json_to_keyring;
use super::save_split_json_to_keyring;
use super::value_key;
use crate::KeyringStore;
use crate::tests::MockKeyringStore;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -701,40 +679,109 @@ mod tests {
}
}
#[cfg(not(windows))]
#[test]
fn json_storage_loads_split_json_compatibility_on_non_windows() {
fn json_storage_does_not_load_compatibility_layout() {
let store = MockKeyringStore::default();
let expected = json!({
"token": "secret",
"nested": {"id": 9}
});
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &expected)
.expect("split JSON should save");
#[cfg(windows)]
{
store
.save(
SERVICE,
BASE_KEY,
&serde_json::to_string(&expected).expect("JSON should serialize"),
)
.expect("full JSON should save");
let loaded =
load_json_from_keyring(&store, SERVICE, BASE_KEY).expect("JSON should load");
assert_eq!(loaded, None);
}
#[cfg(not(windows))]
{
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &expected)
.expect("split JSON should save");
let loaded =
load_json_from_keyring(&store, SERVICE, BASE_KEY).expect("JSON should load");
assert_eq!(loaded, None);
}
}
#[test]
fn json_storage_save_preserves_compatibility_layout() {
let store = MockKeyringStore::default();
let current = json!({"current": true});
let compat = json!({"split": true});
#[cfg(windows)]
{
store
.save(
SERVICE,
BASE_KEY,
&serde_json::to_string(&compat).expect("JSON should serialize"),
)
.expect("full JSON should save");
}
#[cfg(not(windows))]
{
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &compat)
.expect("split JSON should save");
}
save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
let loaded = load_json_from_keyring(&store, SERVICE, BASE_KEY)
.expect("JSON should load")
.expect("JSON should exist");
assert_eq!(loaded, expected);
assert_eq!(loaded, current);
#[cfg(windows)]
{
assert_eq!(
store.saved_value(BASE_KEY),
Some(serde_json::to_string(&compat).expect("JSON should serialize"))
);
}
#[cfg(not(windows))]
{
let compat_loaded = load_split_json_from_keyring(&store, SERVICE, BASE_KEY)
.expect("split JSON should load")
.expect("split JSON should exist");
assert_eq!(compat_loaded, compat);
}
}
#[test]
fn json_storage_delete_removes_platform_and_compat_entries() {
fn json_storage_delete_removes_only_platform_entries() {
let store = MockKeyringStore::default();
let current = json!({"current": true});
let split = json!({"split": true});
let compat = json!({"split": true});
save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &split)
.expect("split JSON should save");
store
.save(
SERVICE,
BASE_KEY,
&serde_json::to_string(&current).expect("JSON should serialize"),
)
.expect("legacy JSON should save");
#[cfg(windows)]
{
store
.save(
SERVICE,
BASE_KEY,
&serde_json::to_string(&compat).expect("JSON should serialize"),
)
.expect("full JSON should save");
save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
}
#[cfg(not(windows))]
{
save_json_to_keyring(&store, SERVICE, BASE_KEY, &current).expect("JSON should save");
save_split_json_to_keyring(&store, SERVICE, BASE_KEY, &compat)
.expect("split JSON should save");
}
let removed = delete_json_from_keyring(&store, SERVICE, BASE_KEY)
.expect("JSON delete should succeed");
@@ -745,8 +792,18 @@ mod tests {
.expect("JSON load should succeed")
.is_none()
);
assert!(!store.contains(BASE_KEY));
assert!(!store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)));
#[cfg(windows)]
{
assert!(store.contains(BASE_KEY));
assert!(!store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)));
}
#[cfg(not(windows))]
{
assert!(!store.contains(BASE_KEY));
assert!(store.contains(&layout_key(BASE_KEY, MANIFEST_ENTRY)));
}
}
#[test]

30
codex-rs/login/src/auth/storage_tests.rs
View File

@@ -292,7 +292,16 @@ fn keyring_auth_storage_load_supports_legacy_single_entry() -> anyhow::Result<()
)?;
let loaded = storage.load()?;
assert_eq!(Some(expected), loaded);
#[cfg(not(windows))]
{
assert_eq!(Some(expected), loaded);
}
#[cfg(windows)]
{
assert_eq!(None, loaded);
}
Ok(())
}
@@ -310,25 +319,6 @@ fn keyring_auth_storage_load_returns_deserialized_keyring_auth() -> anyhow::Resu
Ok(())
}
#[test]
fn keyring_auth_storage_load_supports_split_json_compatibility() -> anyhow::Result<()> {
let codex_home = tempdir()?;
let mock_keyring = MockKeyringStore::default();
let storage = KeyringAuthStorage::new(
codex_home.path().to_path_buf(),
Arc::new(mock_keyring.clone()),
);
let expected = auth_with_prefix("split-compat");
let key = compute_store_key(codex_home.path())?;
let value = serde_json::to_value(&expected)?;
codex_keyring_store::save_split_json_to_keyring(&mock_keyring, KEYRING_SERVICE, &key, &value)?;
let loaded = storage.load()?;
assert_eq!(Some(expected), loaded);
Ok(())
}
#[test]
fn keyring_auth_storage_compute_store_key_for_home_directory() -> anyhow::Result<()> {
let codex_home = PathBuf::from("~/.codex");

40
codex-rs/rmcp-client/src/oauth.rs
View File

@@ -603,7 +603,6 @@ mod tests {
use codex_keyring_store::CredentialStoreError;
use codex_keyring_store::load_json_from_keyring;
use codex_keyring_store::save_json_to_keyring;
use codex_keyring_store::save_split_json_to_keyring;
use keyring::Error as KeyringError;
use pretty_assertions::assert_eq;
use std::sync::Mutex;
@@ -757,22 +756,6 @@ mod tests {
Ok(())
}
#[test]
fn load_oauth_tokens_supports_split_json_compatibility() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
let loaded =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("tokens should load from split-json compatibility format");
assert_tokens_match_without_expiry(&loaded, &tokens);
Ok(())
}
#[test]
fn load_oauth_tokens_supports_legacy_single_entry() -> Result<()> {
let _env = TempCodexHome::new();
@@ -783,9 +766,18 @@ mod tests {
store.save(KEYRING_SERVICE, &key, &serialized)?;
let loaded =
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.expect("tokens should load from keyring");
assert_tokens_match_without_expiry(&loaded, &tokens);
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?;
#[cfg(not(windows))]
{
let loaded = loaded.expect("tokens should load from keyring");
assert_tokens_match_without_expiry(&loaded, &tokens);
}
#[cfg(windows)]
{
assert!(loaded.is_none());
}
Ok(())
}
@@ -895,15 +887,13 @@ mod tests {
}
#[test]
fn delete_oauth_tokens_removes_all_storage() -> Result<()> {
fn delete_oauth_tokens_removes_active_storage() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
let serialized = serde_json::to_string(&tokens)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
save_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
super::save_oauth_tokens_to_file(&tokens)?;
let removed = super::delete_oauth_tokens_from_keyring_and_file(
@@ -928,7 +918,7 @@ mod tests {
let tokens = sample_tokens();
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
let value = serde_json::to_value(&tokens)?;
save_split_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
save_json_to_keyring(&store, KEYRING_SERVICE, &key, &value)?;
assert!(
super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)?
.is_some()