app-server: stabilize detached review start on Windows (#11646)

## Why

`review_start_with_detached_delivery_returns_new_thread_id` has been
failing on Windows CI. The failure mode is a process crash
(`tokio-runtime-worker` stack overflow) during detached review setup,
which causes EOF in the test harness.

This test is intended to validate detached review thread identity, not
shell snapshot behavior. We also still want detached review to avoid
unnecessary rollout-path rediscovery when the parent thread is already
loaded.

## What Changed

- Updated detached review startup in
`codex-rs/app-server/src/codex_message_processor.rs`:
  - `start_detached_review` now receives the loaded parent thread.
  - It prefers `parent_thread.rollout_path()`.
- It falls back to `find_thread_path_by_id_str(...)` only if the
in-memory path is unavailable.
- Hardened the review test fixture in
`codex-rs/app-server/tests/suite/v2/review.rs` by setting
`shell_snapshot = false` in test config, so this test no longer depends
on unrelated Windows PowerShell snapshot initialization.

## Verification

- `cargo test -p codex-app-server`
- Verified
`suite::v2::review::review_start_with_detached_delivery_returns_new_thread_id`
passes locally.

## Notes

- Related context: rollout-path lookup behavior changed in #10532.
This commit is contained in:
Michael Bolin
2026-02-12 16:12:44 -08:00
committed by GitHub
parent aef4af1079
commit 2825ac85a8

View File

@@ -5152,10 +5152,13 @@ impl CodexMessageProcessor {
&mut self,
request_id: &ConnectionRequestId,
parent_thread_id: ThreadId,
parent_thread: Arc<CodexThread>,
review_request: ReviewRequest,
display_text: &str,
) -> std::result::Result<(), JSONRPCErrorError> {
let rollout_path =
let rollout_path = if let Some(path) = parent_thread.rollout_path() {
path
} else {
find_thread_path_by_id_str(&self.config.codex_home, &parent_thread_id.to_string())
.await
.map_err(|err| JSONRPCErrorError {
@@ -5167,7 +5170,8 @@ impl CodexMessageProcessor {
code: INVALID_REQUEST_ERROR_CODE,
message: format!("no rollout found for thread id {parent_thread_id}"),
data: None,
})?;
})?
};
let mut config = self.config.as_ref().clone();
if let Some(review_model) = &config.review_model {
@@ -5290,6 +5294,7 @@ impl CodexMessageProcessor {
.start_detached_review(
&request_id,
parent_thread_id,
parent_thread,
review_request,
display_text.as_str(),
)