Deprecate detached review delivery (#42602)

## What changed

- Emit a connection-scoped `deprecationNotice` when `review/start` uses
  `delivery: "detached"`, including when validation later rejects the request.
- Preserve detached review behavior while directing callers to use `thread/start`
  followed by an inline review for a separate review thread.
- Document the deprecation and migration options in the app-server protocol and
  README.

## Testing

- Cover the notice for accepted and rejected detached reviews.
- Verify that omitted, null, and inline delivery do not emit the notice.

GitOrigin-RevId: 4234a09d2bb7ef8053091580175dc8cbe8d7512c

codex-rs/app-server-protocol/schema/json/ClientRequest.json

@@ -3687,7 +3687,7 @@
             }
           ],
           "default": null,
-          "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`)."
+          "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`). Detached delivery is deprecated and emits `deprecationNotice`. Use `thread/start` followed by an inline review for a separate review thread."
         },
         "target": {
           "$ref": "#/definitions/ReviewTarget"

codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json

@@ -19041,7 +19041,7 @@
               }
             ],
             "default": null,
-            "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`)."
+            "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`). Detached delivery is deprecated and emits `deprecationNotice`. Use `thread/start` followed by an inline review for a separate review thread."
           },
           "target": {
             "$ref": "#/definitions/v2/ReviewTarget"

codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json

@@ -15113,7 +15113,7 @@
             }
           ],
           "default": null,
-          "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`)."
+          "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`). Detached delivery is deprecated and emits `deprecationNotice`. Use `thread/start` followed by an inline review for a separate review thread."
         },
         "target": {
           "$ref": "#/definitions/ReviewTarget"

codex-rs/app-server-protocol/schema/json/v2/ReviewStartParams.json

@@ -111,7 +111,7 @@
         }
       ],
       "default": null,
-      "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`)."
+      "description": "Where to run the review: inline (default) on the current thread or detached on a new thread (returned in `reviewThreadId`). Detached delivery is deprecated and emits `deprecationNotice`. Use `thread/start` followed by an inline review for a separate review thread."
     },
     "target": {
       "$ref": "#/definitions/ReviewTarget"

codex-rs/app-server-protocol/schema/typescript/v2/ReviewStartParams.ts

@@ -8,5 +8,7 @@ export type ReviewStartParams = { threadId: string, target: ReviewTarget,
 /**
  * Where to run the review: inline (default) on the current thread or
  * detached on a new thread (returned in `reviewThreadId`).
+ * Detached delivery is deprecated and emits `deprecationNotice`.
+ * Use `thread/start` followed by an inline review for a separate review thread.
  */
 delivery?: ReviewDelivery | null, };

codex-rs/app-server-protocol/src/protocol/v2/review.rs

@@ -20,6 +20,8 @@ pub struct ReviewStartParams {
 
     /// Where to run the review: inline (default) on the current thread or
     /// detached on a new thread (returned in `reviewThreadId`).
+    /// Detached delivery is deprecated and emits `deprecationNotice`.
+    /// Use `thread/start` followed by an inline review for a separate review thread.
     #[serde(default)]
     #[ts(optional = nullable)]
     pub delivery: Option<ReviewDelivery>,

codex-rs/app-server/README.md

@@ -239,7 +239,7 @@ Example with notification opt-out:
 - `thread/realtime/appendSpeech` — append text that the realtime model should speak to the user (experimental); returns `{}`. Parent-owned Multi-Agent V2 subagents reject this request.
 - `thread/realtime/stop` — stop the active realtime session for the thread (experimental); returns `{}`. Parent-owned Multi-Agent V2 subagents reject this request.
 - `thread/timeline/list` — page ordinary turn items, durable realtime facts, and turn boundaries together in rollout order (experimental). Entries are tagged `item`, `realtime`, `turnStarted`, or `turnCompleted`. Turn boundaries carry lifecycle metadata without duplicating the turn's items; completed boundaries also cover interrupted and failed turns. Each response contains an opaque continuation cursor and `activeRealtimeSessionAtPageStart`, allowing clients to render any bounded page without loading earlier thread history. Entries at the same rollout position have stable ordering and can span pages. Existing `thread/items/list` remains unchanged.
-- `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start`. Inline reviews emit `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. Detached reviews stream ordinary turn items on the new review thread. Parent-owned Multi-Agent V2 subagents reject both inline and detached reviews.
+- `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start`. Inline reviews emit `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. Detached delivery is deprecated and emits `deprecationNotice`; supported detached reviews still stream ordinary turn items on the new review thread. Parent-owned Multi-Agent V2 subagents reject both inline and detached reviews.
 - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation).
 - `command/exec/write` — write base64-decoded stdin bytes to a running `command/exec` session or close stdin; returns `{}`.
 - `command/exec/resize` — resize a running PTY-backed `command/exec` session by `processId`; returns `{}`.
@@ -1488,7 +1488,11 @@ Use `review/start` to run Codex’s reviewer on the currently checked-out projec
 - `{"type":"custom","instructions":"Free-form reviewer instructions"}` — fallback prompt equivalent to the legacy manual review request.
 - `delivery` (`"inline"` or `"detached"`, default `"inline"`) — where the review runs:
   - `"inline"`: run the review as a new turn on the existing thread. The response’s `reviewThreadId` equals the original `threadId`, and no new `thread/started` notification is emitted.
-  - `"detached"`: fork a new review thread from the parent conversation and run the review there. The response’s `reviewThreadId` is the id of this new review thread, and the server emits a `thread/started` notification for it before streaming review items.
+  - `"detached"` (deprecated): fork a new review thread from the parent conversation and run the review there. The response’s `reviewThreadId` is the id of this new review thread, and the server emits a `thread/started` notification for it before streaming review items. Paginated parent threads do not support detached delivery.
+
+Detached delivery will be removed in a future release. Requests with `"delivery": "detached"` emit a `deprecationNotice` to the requesting connection before being processed; existing behavior and validation remain in place. No removal date is set yet. Omitted, null, and `"inline"` delivery remain supported without this warning.
+
+To migrate, create a separate thread with `thread/start`, then call `review/start` with `"delivery": "inline"` on that thread. This runs the built-in reviewer without copying the parent conversation. If your reviewer needs the parent conversation's history, use `thread/fork` followed by `turn/start` with your own review instructions.
 
 Example request/response:
 
@@ -1511,7 +1515,7 @@ Example request/response:
 } }
 ```
 
-For a detached review, use `"delivery": "detached"`. The response is the same shape, but `reviewThreadId` will be the id of the new review thread (different from the original `threadId`). The server also emits a `thread/started` notification for that new thread before streaming the review turn. Internally, this is a normal forked thread and turn whose prompt mentions the bundled `$review-agent` skill, so normal turn steering, tool, permission, and item-stream behavior applies.
+Existing callers using the deprecated `"delivery": "detached"` receive the same response shape, but `reviewThreadId` is the id of the new review thread (different from the original `threadId`). The server also emits a `thread/started` notification for that new thread before streaming the review turn. Internally, this is a normal forked thread and turn whose prompt mentions the bundled `$review-agent` skill, so normal turn steering, tool, permission, and item-stream behavior applies.
 
 Detached review is unsupported when the parent thread is paginated.
 

codex-rs/app-server/src/request_processors/turn_processor.rs

@@ -333,6 +333,17 @@ impl TurnRequestProcessor {
         request_id: &ConnectionRequestId,
         params: ReviewStartParams,
     ) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
+        if matches!(params.delivery, Some(ApiReviewDelivery::Detached)) {
+            self.outgoing
+                .send_server_notification_to_connections(
+                    &[request_id.connection_id],
+                    ServerNotification::DeprecationNotice(DeprecationNoticeNotification {
+                        summary: "review/start with delivery \"detached\" is deprecated and will be removed in a future release.".to_string(),
+                        details: Some("Use thread/start followed by review/start with delivery \"inline\" for a separate review thread, or thread/fork followed by turn/start with your own review instructions.".to_string()),
+                    }),
+                )
+                .await;
+        }
         self.review_start_inner(request_id, params)
             .await
             .map(|()| None)

codex-rs/app-server/tests/suite/v2/review.rs

@@ -6,6 +6,7 @@ use app_test_support::create_final_assistant_message_sse_response;
 use app_test_support::create_mock_responses_server_repeating_assistant;
 use app_test_support::create_mock_responses_server_sequence;
 use codex_app_server_protocol::ClientRequest;
+use codex_app_server_protocol::DeprecationNoticeNotification;
 use codex_app_server_protocol::ItemCompletedNotification;
 use codex_app_server_protocol::ItemStartedNotification;
 use codex_app_server_protocol::JSONRPCError;
@@ -33,6 +34,7 @@ use core_test_support::responses;
 use pretty_assertions::assert_eq;
 use serde_json::json;
 use tempfile::TempDir;
+use test_case::test_case;
 use tokio::time::timeout;
 
 const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
@@ -79,12 +81,23 @@ async fn review_start_rejects_detached_delivery_for_paginated_parent() -> Result
         review_err.error.message,
         "paginated threads do not support detached review"
     );
+    assert!(
+        mcp.pending_notification_methods()
+            .iter()
+            .any(|method| method == "deprecationNotice"),
+        "rejected detached requests should still emit a deprecation notice"
+    );
 
     Ok(())
 }
 
+#[test_case(None; "omitted_delivery")]
+#[test_case(Some(json!(null)); "null_delivery")]
+#[test_case(Some(json!("inline")); "inline_delivery")]
 #[tokio::test]
-async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()> {
+async fn review_start_runs_review_turn_and_emits_code_review_item(
+    delivery: Option<serde_json::Value>,
+) -> Result<()> {
     let review_payload = json!({
         "findings": [
             {
@@ -113,22 +126,22 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
         .build_initialized()
         .await?;
     let thread_id = start_default_thread(&mut mcp).await?;
+    let mut params = json!({
+        "threadId": thread_id,
+        "target": {
+            "type": "commit",
+            "sha": "1234567deadbeef",
+            "title": "Tidy UI colors",
+        },
+    });
+    if let Some(delivery) = delivery {
+        params["delivery"] = delivery;
+    }
+    let request_id = mcp.send_raw_request("review/start", Some(params)).await?;
     let ReviewStartResponse {
         turn,
         review_thread_id,
-    } = mcp
-        .request(|request_id| ClientRequest::ReviewStart {
-            request_id,
-            params: ReviewStartParams {
-                thread_id: thread_id.clone(),
-                delivery: Some(ReviewDelivery::Inline),
-                target: ReviewTarget::Commit {
-                    sha: "1234567deadbeef".to_string(),
-                    title: Some("Tidy UI colors".to_string()),
-                },
-            },
-        })
-        .await?;
+    } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(request_id)).await??;
     assert_eq!(review_thread_id, thread_id.clone());
     let turn_id = turn.id.clone();
     assert_eq!(turn.status, TurnStatus::InProgress);
@@ -187,6 +200,12 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
     let review = review_body.expect("did not observe a code review item");
     assert!(review.contains("Prefer Stylize helpers"));
     assert!(review.contains("/tmp/file.rs:10-20"));
+    assert!(
+        !mcp.pending_notification_methods()
+            .iter()
+            .any(|method| method == "deprecationNotice"),
+        "inline reviews should not emit a deprecation notice"
+    );
 
     Ok(())
 }
@@ -379,21 +398,31 @@ async fn review_start_with_detached_delivery_returns_new_thread_id() -> Result<(
     .await??;
     let thread_id = thread.id;
     materialize_thread_rollout(&mut mcp, &thread_id).await?;
-    let ReviewStartResponse {
-        turn,
-        review_thread_id,
-    } = mcp
-        .request(|request_id| ClientRequest::ReviewStart {
-            request_id,
-            params: ReviewStartParams {
-                thread_id: thread_id.clone(),
-                delivery: Some(ReviewDelivery::Detached),
-                target: ReviewTarget::Custom {
-                    instructions: "detached review".to_string(),
-                },
+    let review_id = mcp
+        .send_review_start_request(ReviewStartParams {
+            thread_id: thread_id.clone(),
+            delivery: Some(ReviewDelivery::Detached),
+            target: ReviewTarget::Custom {
+                instructions: "detached review".to_string(),
             },
         })
         .await?;
+    let notice: DeprecationNoticeNotification = timeout(
+        DEFAULT_READ_TIMEOUT,
+        mcp.read_notification("deprecationNotice"),
+    )
+    .await??;
+    assert_eq!(
+        notice,
+        DeprecationNoticeNotification {
+            summary: "review/start with delivery \"detached\" is deprecated and will be removed in a future release.".to_string(),
+            details: Some("Use thread/start followed by review/start with delivery \"inline\" for a separate review thread, or thread/fork followed by turn/start with your own review instructions.".to_string()),
+        }
+    );
+    let ReviewStartResponse {
+        turn,
+        review_thread_id,
+    } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(review_id)).await??;
 
     assert_eq!(turn.status, TurnStatus::InProgress);
     assert_eq!(turn.items_view, TurnItemsView::NotLoaded);