mirror of
https://github.com/openai/codex.git
synced 2026-05-03 21:01:55 +03:00
## Why Windows can represent the same canonical local path with either a normal drive path or a verbatim device path prefix. The failure pattern that motivated this PR was an assertion diff like `C:\...` versus `\\?\C:\...`: different spellings, same file. That became visible while validating the permissions stack above this PR. The stack increasingly routes paths through `AbsolutePathBuf`, which normalizes supported Windows device prefixes, while several existing tests still built expected values directly with `std::fs::canonicalize()` or compared `AbsolutePathBuf::as_path()` to a raw `PathBuf`. On Windows, that can make tests fail because the two sides choose different textual forms for an otherwise equivalent canonical path. This PR is intentionally split out as the bottom PR below #19606. The runtime permissions migration should not carry unrelated Windows test stabilization, and reviewers should be able to verify this as a test-only change before looking at the larger permissions changes. ## Failure Modes Covered - `conversation_summary` expected rollout paths were built from raw canonicalized `PathBuf`s, while app-server responses could carry `AbsolutePathBuf`-normalized paths. - `thread_resume` compared returned thread paths directly to previously stored or fixture paths, so a verbatim-prefix spelling could fail an otherwise correct resume. - `marketplace_add` compared plugin install roots through `as_path()` against raw canonicalized paths, reproducing the same `C:\...` versus `\\?\C:\...` mismatch in both app-server and core-plugin coverage. ## What Changed - In `app-server/tests/suite/conversation_summary.rs`, normalize both expected rollout paths and received `ConversationSummary.path` values through `AbsolutePathBuf` before comparing the full summary object. - In `app-server/tests/suite/v2/thread_resume.rs`, normalize both sides of thread path comparisons before asserting equality. This keeps the tests focused on whether resume returned the same existing path, not whether Windows used the same string spelling. - In `app-server/tests/suite/v2/marketplace_add.rs` and `core-plugins/src/marketplace_add.rs`, compare install roots as `AbsolutePathBuf` values instead of comparing an absolute-path wrapper to a raw canonicalized `PathBuf`. ## Behavior This PR does not change production app-server or marketplace behavior. It only changes tests to assert semantic path identity across Windows path spelling variants. It also leaves API response values untouched; the normalization happens inside assertions only. ## Verification Targeted local checks run while extracting this fix: - `cargo test -p codex-app-server get_conversation_summary_by_thread_id_reads_rollout` - `cargo test -p codex-app-server get_conversation_summary_by_relative_rollout_path_resolves_from_codex_home` - `cargo test -p codex-app-server thread_resume_prefers_path_over_thread_id` Windows-specific confidence comes from the Bazel Windows CI job for this PR, since the failure is platform-specific. ## Docs No docs update is needed because this is test-only infrastructure stabilization. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19604). * #19395 * #19394 * #19393 * #19392 * #19606 * __->__ #19604