mirror of
https://github.com/openai/codex.git
synced 2026-11-09 16:41:37 +00:00
[Codex Thread 019edd6d-6f14-74e2-853c-345d1803d4a6](https://codex-thread-link.openai.chatgpt-team.site/thread/019edd6d-6f14-74e2-853c-345d1803d4a6) ## Stack Review and merge in order. Every layer is independently correct and documents its safe stopping point. 1. [openai/codex#30292](https://github.com/openai/codex/pull/30292) — aggregate File/Secrets store locking 2. [openai/codex#30293](https://github.com/openai/codex/pull/30293) — resolve and lifecycle-pin the exact OAuth store 3. [openai/codex#30416](https://github.com/openai/codex/pull/30416) — serialized authoritative refresh transaction 4. [openai/codex#30294](https://github.com/openai/codex/pull/30294) — Codex-owned transport refresh and one-shot 401 recovery 5. [openai/codex#30295](https://github.com/openai/codex/pull/30295) — login/logout transaction serialization 6. [openai/codex#30296](https://github.com/openai/codex/pull/30296) — diagnostic-only Auto store drift reporting **This PR is layer 1.** ## Why MCP OAuth credentials stored in File or Secrets share one aggregate map. Concurrent read-modify-write operations for different MCP servers can both read the same snapshot and let the later write discard the earlier update. That is a correctness problem independent of refresh-token rotation. ## What this PR does - Adds a bounded cross-process lock around aggregate File and Secrets loads, saves, and deletes. - Distinguishes aggregate-lock failures from Secrets backend unavailability, so Auto can fall back only for the latter and cannot bypass serialization by reading or writing File. - Keeps Direct keyring operations outside this lock because they are already per credential. - Releases the Secrets aggregate lock before legacy File cleanup so cross-store cleanup cannot create nested aggregate-lock ordering. - Tests actual contention by waiting for an observed `WouldBlock`, rather than assuming a sleeping worker reached the lock. - Tests load and save with only the Secrets lock path broken while fallback File remains readable and writable. ## Decisions and non-goals - This lock protects aggregate-store read-modify-write integrity only. It does not choose a credential authority or serialize an OAuth refresh transaction. - The lock is scoped to the active `CODEX_HOME`, matching the aggregate files it protects. - Lock waits are bounded, and coordination failures are surfaced rather than treated as evidence that Secrets is unavailable. ## Safe stopping point This PR can merge alone. It prevents lost updates and partial aggregate reads. Auto can still resolve again during a client lifecycle until layer 2, and concurrent refreshes remain possible until layer 3. ## Validation - `just test -p codex-rmcp-client` (96 passed; expected environment skips) - Focused aggregate File/Secrets lock contention and Auto fallback tests
47 lines
1.2 KiB
Rust
47 lines
1.2 KiB
Rust
use std::path::Path;
|
|
use std::sync::Mutex;
|
|
use std::sync::MutexGuard;
|
|
use std::sync::OnceLock;
|
|
use std::sync::PoisonError;
|
|
|
|
use tempfile::tempdir;
|
|
|
|
/// Serializes tests that mutate process-wide CODEX_HOME.
|
|
///
|
|
/// Keep OAuth tests on this one guard instead of defining per-module helpers; otherwise
|
|
/// concurrently running test modules can point File/Secrets storage at different homes.
|
|
pub(super) struct TempCodexHome {
|
|
_guard: MutexGuard<'static, ()>,
|
|
_dir: tempfile::TempDir,
|
|
}
|
|
|
|
impl TempCodexHome {
|
|
pub(super) fn new() -> Self {
|
|
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
|
|
let guard = LOCK
|
|
.get_or_init(Mutex::default)
|
|
.lock()
|
|
.unwrap_or_else(PoisonError::into_inner);
|
|
let dir = tempdir().expect("create CODEX_HOME temp dir");
|
|
unsafe {
|
|
std::env::set_var("CODEX_HOME", dir.path());
|
|
}
|
|
Self {
|
|
_guard: guard,
|
|
_dir: dir,
|
|
}
|
|
}
|
|
|
|
pub(super) fn path(&self) -> &Path {
|
|
self._dir.path()
|
|
}
|
|
}
|
|
|
|
impl Drop for TempCodexHome {
|
|
fn drop(&mut self) {
|
|
unsafe {
|
|
std::env::remove_var("CODEX_HOME");
|
|
}
|
|
}
|
|
}
|