Skip to content

Persist upstream Radar review artifacts #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yvette-carlisle merged 1 commit into from
Jun 10, 2026
Merged

Persist upstream Radar review artifacts #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions artifacts/github/bundles/openai-codex-pr-22879.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
{
"analysis_mode": "pr_first",
"commits": [
{
"author": "canvrno-oai",
"committed_at": "2026-05-15T18:09:02Z",
"message": "reject Esc steer submission during /review",
"sha": "900347bbffae41f08467ae2ff7552ea3564847fe",
"url": "https://github.com/openai/codex/commit/900347bbffae41f08467ae2ff7552ea3564847fe"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-15T18:15:31Z",
"message": "snapshot",
"sha": "80f3514fe4fa2d0c811797571ac8e813ed28c10d",
"url": "https://github.com/openai/codex/commit/80f3514fe4fa2d0c811797571ac8e813ed28c10d"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-15T18:46:58Z",
"message": "Updated snapshot with new text",
"sha": "95d77fadad869b547af91f67bebdfc40d9927675",
"url": "https://github.com/openai/codex/commit/95d77fadad869b547af91f67bebdfc40d9927675"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-15T19:11:44Z",
"message": "snap metadata",
"sha": "7f8f4e5d1a414d102075fa0fe87acdfff7f12f46",
"url": "https://github.com/openai/codex/commit/7f8f4e5d1a414d102075fa0fe87acdfff7f12f46"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-16T02:00:27Z",
"message": "retry turn interrupts after active-turn mismatch - Ctrl + C case",
"sha": "375aab3440badf14d4f94ce5bb672fa800fea228",
"url": "https://github.com/openai/codex/commit/375aab3440badf14d4f94ce5bb672fa800fea228"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-26T20:00:33Z",
"message": "Set active turn for review threads",
"sha": "061e1ad76c24a244534a5f27f70ae713b7d610b2",
"url": "https://github.com/openai/codex/commit/061e1ad76c24a244534a5f27f70ae713b7d610b2"
},
{
"author": "canvrno-oai",
"committed_at": "2026-05-26T22:08:05Z",
"message": "Let lifecycle notifications update active turn cache",
"sha": "bbdb159af09fb5fbf4c672c7e9679a41725f6e7c",
"url": "https://github.com/openai/codex/commit/bbdb159af09fb5fbf4c672c7e9679a41725f6e7c"
}
],
"default_branch": "main",
"docs_refs": [],
"examples_refs": [],
"extracted_flags": [
"TUI",
"JSONRPCE",
"REVIEW_STEER_UNAVAILABLE_MESSAGE",
"NONE"
],
"files": [
{
"additions": 19,
"deletions": 0,
"patch_excerpt": "@@ -680,6 +680,25 @@ fn archived_session_guidance(err: &color_eyre::eyre::Report) -> Option<String> {\n Some(message.to_string())\n }\n \n+fn active_turn_interrupt_race(error: &TypedRequestError) -> Option<String> {\n+ let TypedRequestError::Server { method, source } = error else {\n+ return None;\n+ };\n+ if method != \"turn/interrupt\" {\n+ return None;\n+ }\n+ let mismatch_prefix = \"expected active turn id \";\n+ let mismatch_separator = \" but found \";\n+ Some(\n+ source\n+ .message\n+ .strip_prefix(mismatch_prefix)?\n+ .split_once(mismatch_separator)?\n+ .1\n+ .to_string(),\n+ )\n+}\n+\n impl App {\n pub fn chatwidget_init_for_forked_or_resumed_thread(\n &self,",
"path": "codex-rs/tui/src/app.rs",
"status": "modified"
},
{
"additions": 17,
"deletions": 0,
"patch_excerpt": "@@ -4614,6 +4614,23 @@ fn active_turn_steer_race_extracts_actual_turn_id_from_mismatch() {\n );\n }\n \n+#[test]\n+fn active_turn_interrupt_race_extracts_actual_turn_id_from_mismatch() {\n+ let error = TypedRequestError::Server {\n+ method: \"turn/interrupt\".to_string(),\n+ source: JSONRPCErrorError {\n+ code: -32602,\n+ message: \"expected active turn id turn-expected but found turn-actual\".to_string(),\n+ data: None,\n+ },\n+ };\n+\n+ assert_eq!(\n+ active_turn_interrupt_race(&error),\n+ Some(\"turn-actual\".to_string())\n+ );\n+}\n+\n #[tokio::test]\n async fn fresh_session_config_uses_current_service_tier() {\n let mut app = make_test_app().await;",
"path": "codex-rs/tui/src/app/tests.rs",
"status": "modified"
},
{
"additions": 38,
"deletions": 3,
"patch_excerpt": "@@ -500,9 +500,39 @@ impl App {\n match op {\n AppCommand::Interrupt { .. } => {\n if let Some(turn_id) = self.active_turn_id_for_thread(thread_id).await {\n- app_server.turn_interrupt(thread_id, turn_id).await?;\n+ let mut interrupt_turn_id = turn_id;\n+ for retried_after_turn_mismatch in [false, true] {\n+ match app_server\n+ .turn_interrupt(thread_id, interrupt_turn_id.clone())\n+ .await\n+ {\n+ Ok(()) => return Ok(true),\n+ Err(error) if !retried_after_turn_mismatch => {\n+ let Some(actual_turn_id) = active_turn_interrupt_race(&error)\n+ else {\n+ return Err(er...",
"path": "codex-rs/tui/src/app/thread_routing.rs",
"status": "modified"
},
{
"additions": 6,
"deletions": 4,
"patch_excerpt": "@@ -736,7 +736,7 @@ impl AppServerSession {\n &mut self,\n thread_id: ThreadId,\n turn_id: String,\n- ) -> Result<()> {\n+ ) -> std::result::Result<(), TypedRequestError> {\n let request_id = self.next_request_id();\n let _: TurnInterruptResponse = self\n .client\n@@ -747,12 +747,14 @@ impl AppServerSession {\n turn_id,\n },\n })\n- .await\n- .wrap_err(\"turn/interrupt failed in TUI\")?;\n+ .await?;\n Ok(())\n }\n \n- pub(crate) async fn startup_interrupt(&mut self, thread_id: ThreadId) -> Result<()> {\n+ pub(crate) async fn startup_interrupt(\n+ &mut self,\n+ thread_id: ThreadId,\n+ ) -> std::result::Result<(), TypedRequestError> {\n self.turn_interrupt(thread_id, String::new()).await\n }",
"path": "codex-rs/tui/src/app_server_session.rs",
"status": "modified"
},
{
"additions": 14,
"deletions": 0,
"patch_excerpt": "@@ -112,6 +112,20 @@ impl ChatWidget {\n return;\n }\n \n+ const REVIEW_STEER_UNAVAILABLE_MESSAGE: &str = \"Steer messages aren't supported during /review. Press Ctrl+C now to cancel the review.\";\n+\n+ if self.chat_keymap.interrupt_turn.is_pressed(key_event)\n+ && self.review.is_review_mode\n+ && (!self.input_queue.pending_steers.is_empty()\n+ || !self.input_queue.rejected_steers_queue.is_empty())\n+ && self.bottom_pane.is_task_running()\n+ && self.bottom_pane.no_modal_or_popup_active()\n+ && !self.should_handle_vim_insert_escape(key_event)\n+ {\n+ self.add_warning_message(REVIEW_STEER_UNAVAILABLE_MESSAGE.to_string());\n+ return;\n+ }\n+\n if self.chat_keymap.interrupt_turn.is_pressed(key_event)\n && !self.input_queue.pending_steers.is_empty()\n ...",
"path": "codex-rs/tui/src/chatwidget/interaction.rs",
"status": "modified"
},
{
"additions": 4,
"deletions": 4,
"patch_excerpt": "@@ -1,7 +1,7 @@\n ---\n source: tui/src/chatwidget/tests/review_mode.rs\n-expression: lines_to_single_string(last)\n+assertion_line: 307\n+expression: last\n ---\n-⚠ Steer messages aren't supported during /review. Send your message after the\n- review finishes, or press Ctrl+C now to cancel the review.\n-\n+⚠ Steer messages aren't supported during /review. Press Ctrl+C now to cancel the\n+ review.",
"path": "codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_submission_warning_snapshot.snap",
"status": "modified"
},
{
"additions": 23,
"deletions": 0,
"patch_excerpt": "@@ -285,6 +285,29 @@ async fn steer_rejection_queues_review_follow_up_before_existing_queued_messages\n }\n }\n \n+#[tokio::test]\n+async fn esc_with_review_queued_steers_shows_warning_and_does_not_interrupt() {\n+ let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;\n+ chat.thread_id = Some(ThreadId::new());\n+ handle_turn_started(&mut chat, \"turn-1\");\n+ handle_entered_review_mode(&mut chat, \"feature branch\");\n+ let _ = drain_insert_history(&mut rx);\n+ chat.input_queue\n+ .pending_steers\n+ .push_back(pending_steer(\"review follow-up\"));\n+ chat.refresh_pending_input_preview();\n+\n+ chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));\n+\n+ assert!(!chat.input_queue.submit_pending_steers_after_interrupt);\n+ assert_eq!(chat.input_queue.pending_steers.len(), 1);\n+ assert_no_submit_op(&mut op_rx);\n...",
"path": "codex-rs/tui/src/chatwidget/tests/review_mode.rs",
"status": "modified"
}
],
"linked_issues": [
"#22815"
],
"notes": [
"Built from GitHub pull-request, commits, files, and repo endpoints."
],
"primary_pr": {
"body": "This changes the `/review` escape path so `Esc` no longer behaves like the normal queued-follow-up interrupt flow while a review is running. Steering is not currently supported in `/review` mode, without this change users are able to attempt a steer but it leads to a crash (see #22815). If the user has already tried to send additional guidance during `/review`, the TUI now keeps the review running and shows a warning that steer messages are not supported in that mode, while still pointing users to `Ctrl+C` if they actually want to cancel. It also adds regression coverage for the review-specific warning behavior. When users do cancel with Ctrl+C during /review, the TUI now tolerates the active-turn race that can happen during review handoff, and any queued steer messages are restored to the composer instead of being discarded.\r\n\r\n- Special-case `Esc` during an active `/review` when follow-up steer input is pending or has already been deferred.\r\n- Show a clear warning instead of interrupting the running review.\r\n- Make the Ctrl+C cancel path during /review resilient to active-turn races, while preserving any queued steer text by restoring it to the composer.\r\n- Add review-mode test coverage for the warning path.\r\n\r\n## Testing\r\n\r\n1. Start a `/review` with a diff large enough that the review stays active for more than a few seconds. \r\n\r\n2. While the review is still running, type a follow-up / steer message, submit it, and then press `Esc`. \r\n Before: `Esc` causes the TUI to close abruptly. \r\n After: the review keeps running and the transcript shows a warning that steer messages are not supported during `/review`, with guidance to use `Ctrl+C` if you want to cancel.\r\n\r\n3. Press `Ctrl+C` if you actually want to stop the review. \r\n Before: (after restarting the test since Pt. 2 crashed) this is the intentional cancellation path. \r\n After: this remains the intentional cancellation path, and any queued follow-up steer text is restored to the composer instead of being lost.\r\n \r\n## Note:\r\n`/review` mode explicitly does not support steering at this time (as noted in `turn_processer.rs`, if we want to explore that in the future this code will need to be modified). This change keeps unsupported steer attempts from crashing the TUI and preserves queued follow-up text if the user cancels with Ctrl+C.",
"labels": [],
"merged_at": "2026-06-09T16:41:58Z",
"number": 22879,
"state": "merged",
"title": "fix: Prevent /review crash when entering Esc on steer message",
"url": "https://github.com/openai/codex/pull/22879"
},
"repo": "openai/codex",
"schema": "github_change_bundle/v1"
}
Loading