Skip to content

Commit 2e7991d

Browse files
committed
Fix btw overlay delivery and presentation
Treat /btw text and completion notifications as lossless app-server events so a saturated event bridge cannot leave the modal stuck in Answering state. Replace the generic pager chrome with a compact BTW modal and request redraws when side-answer state changes. Tests: - RUSTC_WRAPPER=sccache CC=clang cargo test -p codex-app-server --lib guaranteed_delivery_helpers_cover_terminal_server_notifications - RUSTC_WRAPPER=sccache CC=clang cargo test -p codex-app-server-client --lib event_requires_delivery_marks_transcript_and_terminal_events - RUSTC_WRAPPER=sccache CC=clang cargo test -p codex-tui --lib btw_notifications_collect_answer_and_emit_completion - RUSTC_WRAPPER=sccache CC=clang cargo test -p codex-tui --lib btw_overlay_snapshot_uses_compact_modal_chrome - RUSTC_WRAPPER=sccache CC=clang just fix -p codex-tui - RUSTC_WRAPPER=sccache CC=clang just fix -p codex-app-server-client - RUSTC_WRAPPER=sccache CC=clang just fix -p codex-app-server
1 parent 63b3bd3 commit 2e7991d

8 files changed

Lines changed: 274 additions & 25 deletions

File tree

codex-rs/app-server-client/src/lib.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ fn event_requires_delivery(event: &InProcessServerEvent) -> bool {
162162

163163
/// Returns `true` for notifications that must survive backpressure.
164164
///
165-
/// Transcript events (`AgentMessageDelta`, `PlanDelta`, reasoning deltas) and
166-
/// the authoritative `ItemCompleted` / `TurnCompleted` form the lossless tier
167-
/// of the event stream. Dropping any of these corrupts the visible assistant
168-
/// output or leaves surfaces waiting for a completion signal that already
165+
/// Transcript events (`AgentMessageDelta`, `PlanDelta`, reasoning deltas), `/btw`
166+
/// lifecycle events, and the authoritative `ItemCompleted` / `TurnCompleted` form
167+
/// the lossless tier of the event stream. Dropping any of these corrupts the visible
168+
/// assistant output or leaves surfaces waiting for a completion signal that already
169169
/// fired. Everything else (`CommandExecutionOutputDelta`, progress, etc.) is
170170
/// best-effort and may be dropped with only cosmetic impact.
171171
///
@@ -180,6 +180,8 @@ pub(crate) fn server_notification_requires_delivery(notification: &ServerNotific
180180
| ServerNotification::PlanDelta(_)
181181
| ServerNotification::ReasoningSummaryTextDelta(_)
182182
| ServerNotification::ReasoningTextDelta(_)
183+
| ServerNotification::BtwTextDelta(_)
184+
| ServerNotification::BtwCompleted(_)
183185
)
184186
}
185187

@@ -2072,6 +2074,27 @@ mod tests {
20722074
)
20732075
)
20742076
));
2077+
assert!(event_requires_delivery(
2078+
&InProcessServerEvent::ServerNotification(
2079+
codex_app_server_protocol::ServerNotification::BtwTextDelta(
2080+
codex_app_server_protocol::BtwTextDeltaNotification {
2081+
btw_id: "btw".to_string(),
2082+
delta: "hello".to_string(),
2083+
}
2084+
)
2085+
)
2086+
));
2087+
assert!(event_requires_delivery(
2088+
&InProcessServerEvent::ServerNotification(
2089+
codex_app_server_protocol::ServerNotification::BtwCompleted(
2090+
codex_app_server_protocol::BtwCompletedNotification {
2091+
btw_id: "btw".to_string(),
2092+
answer: Some("done".to_string()),
2093+
error: None,
2094+
}
2095+
)
2096+
)
2097+
));
20752098
assert!(!event_requires_delivery(&InProcessServerEvent::Lagged {
20762099
skipped: 1
20772100
}));

codex-rs/app-server/src/in_process.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,12 @@ pub const DEFAULT_IN_PROCESS_CHANNEL_CAPACITY: usize = CHANNEL_CAPACITY;
102102
type PendingClientRequestResponse = std::result::Result<Result, JSONRPCErrorError>;
103103

104104
fn server_notification_requires_delivery(notification: &ServerNotification) -> bool {
105-
matches!(notification, ServerNotification::TurnCompleted(_))
105+
matches!(
106+
notification,
107+
ServerNotification::TurnCompleted(_)
108+
| ServerNotification::BtwTextDelta(_)
109+
| ServerNotification::BtwCompleted(_)
110+
)
106111
}
107112

108113
/// Input needed to start an in-process app-server runtime.
@@ -721,6 +726,8 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
721726
#[cfg(test)]
722727
mod tests {
723728
use super::*;
729+
use codex_app_server_protocol::BtwCompletedNotification;
730+
use codex_app_server_protocol::BtwTextDeltaNotification;
724731
use codex_app_server_protocol::ClientInfo;
725732
use codex_app_server_protocol::ConfigRequirementsReadResponse;
726733
use codex_app_server_protocol::SessionSource as ApiSessionSource;
@@ -886,5 +893,18 @@ mod tests {
886893
},
887894
})
888895
));
896+
assert!(server_notification_requires_delivery(
897+
&ServerNotification::BtwTextDelta(BtwTextDeltaNotification {
898+
btw_id: "btw-1".to_string(),
899+
delta: "hello".to_string(),
900+
})
901+
));
902+
assert!(server_notification_requires_delivery(
903+
&ServerNotification::BtwCompleted(BtwCompletedNotification {
904+
btw_id: "btw-1".to_string(),
905+
answer: Some("done".to_string()),
906+
error: None,
907+
})
908+
));
889909
}
890910
}

codex-rs/tui/src/app/btw.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,10 @@ impl App {
6464
self.pending_btw_questions
6565
.insert(btw_id.clone(), Arc::clone(&state));
6666
let _ = tui.enter_alt_screen();
67-
let overlay_cell: Box<dyn history_cell::HistoryCell> = Box::new(
67+
let overlay_cell: Arc<dyn history_cell::HistoryCell> = Arc::new(
6868
history_cell::new_btw_question_cell(Arc::clone(&state), cwd.as_path()),
6969
);
70-
let overlay_renderable: Box<dyn Renderable> = Box::new(overlay_cell);
71-
self.overlay = Some(Overlay::new_static_with_renderables(
72-
vec![overlay_renderable],
73-
"B T W".to_string(),
74-
self.keymap.pager.clone(),
75-
));
70+
self.overlay = Some(Overlay::new_btw(overlay_cell, self.keymap.pager.clone()));
7671
tui.frame_requester().schedule_frame();
7772

7873
let response = app_server
@@ -104,6 +99,7 @@ impl App {
10499
if let Ok(mut state) = state.lock() {
105100
state.push_delta(&notification.delta);
106101
}
102+
self.app_event_tx.send(AppEvent::RequestRedraw);
107103
true
108104
}
109105
ServerNotification::BtwCompleted(notification) => {
@@ -113,6 +109,7 @@ impl App {
113109
if let Ok(mut state) = state.lock() {
114110
state.complete(notification.answer.clone(), notification.error.clone());
115111
}
112+
self.app_event_tx.send(AppEvent::RequestRedraw);
116113
true
117114
}
118115
_ => false,

codex-rs/tui/src/app/event_dispatch.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ impl App {
1616
event: AppEvent,
1717
) -> Result<AppRunControl> {
1818
match event {
19+
AppEvent::RequestRedraw => {
20+
tui.frame_requester().schedule_frame();
21+
}
1922
AppEvent::NewSession => {
2023
self.start_fresh_session_with_summary_hint(
2124
tui, app_server, /*session_start_source*/ None,

codex-rs/tui/src/app_event.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ pub(crate) enum KeymapEditIntent {
126126
#[allow(clippy::large_enum_variant)]
127127
#[derive(Debug)]
128128
pub(crate) enum AppEvent {
129+
/// Schedule a frame because state changed outside the direct TUI event path.
130+
RequestRedraw,
131+
129132
/// Open the agent picker for switching active threads.
130133
OpenAgentPicker,
131134
/// Stop a spawned subagent from the background task panel.

codex-rs/tui/src/history_cell.rs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -691,14 +691,16 @@ impl HistoryCell for BtwQuestionCell {
691691
let Ok(state) = self.state.lock() else {
692692
return vec!["/btw state unavailable".red().into()];
693693
};
694-
let mut lines = vec![vec!["/btw ".magenta().bold(), state.question.clone().dim()].into()];
694+
let mut lines = vec![
695+
vec!["Question ".magenta().bold(), state.question.clone().into()].into(),
696+
Line::from(""),
697+
];
695698

696699
if let Some(error) = &state.error {
697700
lines.push(vec![" ".into(), format!("■ {error}").red()].into());
698701
} else if state.answer.trim().is_empty() && !state.completed {
699702
lines.push(
700703
vec![
701-
" ".into(),
702704
activity_indicator(
703705
Some(state.start_time),
704706
MotionMode::from_animations_enabled(state.animations_enabled),
@@ -721,17 +723,7 @@ impl HistoryCell for BtwQuestionCell {
721723
Some(self.cwd.as_path()),
722724
&mut answer_lines,
723725
);
724-
lines.extend(prefix_lines(answer_lines, " ".into(), " ".into()));
725-
}
726-
727-
if state.completed {
728-
lines.push(
729-
vec![
730-
" ".into(),
731-
"Space, Enter, or Escape to dismiss".dark_gray(),
732-
]
733-
.into(),
734-
);
726+
lines.extend(answer_lines);
735727
}
736728
lines
737729
}

0 commit comments

Comments
 (0)