mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-17 15:34:54 +00:00
fix(tui/order): enforce ticketed background events
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
use std::sync::mpsc::Sender;
|
||||
|
||||
use crate::app_event::{AppEvent, BackgroundPlacement};
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::session_log;
|
||||
use codex_core::protocol::OrderMeta;
|
||||
|
||||
@@ -61,21 +62,6 @@ impl AppEventSender {
|
||||
}
|
||||
}
|
||||
|
||||
/// Emit a background event using the provided placement strategy. Defaults
|
||||
/// to appending at the end of the current history window.
|
||||
///
|
||||
/// IMPORTANT: UI code should call this (or other history helpers) rather
|
||||
/// than constructing `Event { event_seq: 0, .. }` manually. Protocol events
|
||||
/// must come from `codex-core` via `Session::make_event` so the per-turn
|
||||
/// sequence stays consistent.
|
||||
pub(crate) fn send_background_event_with_placement(
|
||||
&self,
|
||||
message: impl Into<String>,
|
||||
placement: BackgroundPlacement,
|
||||
) {
|
||||
self.send_background_event_with_placement_and_order(message, placement, None);
|
||||
}
|
||||
|
||||
pub(crate) fn send_background_event_with_placement_and_order(
|
||||
&self,
|
||||
message: impl Into<String>,
|
||||
@@ -89,9 +75,30 @@ impl AppEventSender {
|
||||
});
|
||||
}
|
||||
|
||||
/// Convenience: append a background event at the end of the history.
|
||||
pub(crate) fn send_background_event(&self, message: impl Into<String>) {
|
||||
self.send_background_event_with_placement(message, BackgroundPlacement::Tail);
|
||||
pub(crate) fn send_background_event_with_ticket(
|
||||
&self,
|
||||
ticket: &BackgroundOrderTicket,
|
||||
message: impl Into<String>,
|
||||
) {
|
||||
let order = ticket.next_order();
|
||||
self.send_background_event_with_placement_and_order(
|
||||
message,
|
||||
BackgroundPlacement::Tail,
|
||||
Some(order),
|
||||
);
|
||||
}
|
||||
|
||||
pub(crate) fn send_background_before_next_output_with_ticket(
|
||||
&self,
|
||||
ticket: &BackgroundOrderTicket,
|
||||
message: impl Into<String>,
|
||||
) {
|
||||
let order = ticket.next_order();
|
||||
self.send_background_event_with_placement_and_order(
|
||||
message,
|
||||
BackgroundPlacement::BeforeNextOutput,
|
||||
Some(order),
|
||||
);
|
||||
}
|
||||
|
||||
pub(crate) fn send_background_event_with_order(
|
||||
@@ -106,15 +113,4 @@ impl AppEventSender {
|
||||
);
|
||||
}
|
||||
|
||||
/// Convenience: place a background event before the next provider/tool output.
|
||||
pub(crate) fn send_background_event_before_next_output(
|
||||
&self,
|
||||
message: impl Into<String>,
|
||||
) {
|
||||
self.send_background_event_with_placement(
|
||||
message,
|
||||
BackgroundPlacement::BeforeNextOutput,
|
||||
);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ use ratatui::layout::Rect;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::user_approval_widget::ApprovalRequest;
|
||||
use crate::user_approval_widget::UserApprovalWidget;
|
||||
|
||||
@@ -15,29 +16,38 @@ use std::collections::VecDeque;
|
||||
/// Modal overlay asking the user to approve/deny a sequence of requests.
|
||||
pub(crate) struct ApprovalModalView<'a> {
|
||||
current: UserApprovalWidget<'a>,
|
||||
queue: VecDeque<ApprovalRequest>,
|
||||
queue: VecDeque<(ApprovalRequest, BackgroundOrderTicket)>,
|
||||
app_event_tx: AppEventSender,
|
||||
}
|
||||
|
||||
impl ApprovalModalView<'_> {
|
||||
pub fn new(request: ApprovalRequest, app_event_tx: AppEventSender) -> Self {
|
||||
pub fn new(
|
||||
request: ApprovalRequest,
|
||||
ticket: BackgroundOrderTicket,
|
||||
app_event_tx: AppEventSender,
|
||||
) -> Self {
|
||||
Self {
|
||||
current: UserApprovalWidget::new(request, app_event_tx.clone()),
|
||||
current: UserApprovalWidget::new(request, ticket, app_event_tx.clone()),
|
||||
queue: VecDeque::new(),
|
||||
app_event_tx,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn enqueue_request(&mut self, req: ApprovalRequest) {
|
||||
self.queue.push_back(req);
|
||||
pub fn enqueue_request(
|
||||
&mut self,
|
||||
req: ApprovalRequest,
|
||||
ticket: BackgroundOrderTicket,
|
||||
) {
|
||||
self.queue.push_back((req, ticket));
|
||||
}
|
||||
|
||||
/// Advance to next request if the current one is finished.
|
||||
fn maybe_advance(&mut self) {
|
||||
if self.current.is_complete() {
|
||||
if let Some(req) = self.queue.pop_front() {
|
||||
self.current = UserApprovalWidget::new(req, self.app_event_tx.clone());
|
||||
}
|
||||
if let Some((req, ticket)) = self.queue.pop_front() {
|
||||
self.current =
|
||||
UserApprovalWidget::new(req, ticket, self.app_event_tx.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -66,8 +76,12 @@ impl<'a> BottomPaneView<'a> for ApprovalModalView<'a> {
|
||||
(&self.current).render_ref(area, buf);
|
||||
}
|
||||
|
||||
fn try_consume_approval_request(&mut self, req: ApprovalRequest) -> Option<ApprovalRequest> {
|
||||
self.enqueue_request(req);
|
||||
fn try_consume_approval_request(
|
||||
&mut self,
|
||||
req: ApprovalRequest,
|
||||
ticket: BackgroundOrderTicket,
|
||||
) -> Option<(ApprovalRequest, BackgroundOrderTicket)> {
|
||||
self.enqueue_request(req, ticket);
|
||||
None
|
||||
}
|
||||
}
|
||||
@@ -91,8 +105,10 @@ mod tests {
|
||||
let (tx_raw, _rx) = channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let first = make_exec_request();
|
||||
let mut view = ApprovalModalView::new(first, tx);
|
||||
view.enqueue_request(make_exec_request());
|
||||
let ticket = BackgroundOrderTicket::test_for_request(1);
|
||||
let mut view = ApprovalModalView::new(first, ticket, tx);
|
||||
let ticket2 = BackgroundOrderTicket::test_for_request(2);
|
||||
view.enqueue_request(make_exec_request(), ticket2);
|
||||
|
||||
let (tx_raw2, _rx2) = channel::<AppEvent>();
|
||||
let mut pane = BottomPane::new(super::super::BottomPaneParams {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::user_approval_widget::ApprovalRequest;
|
||||
use crossterm::event::KeyEvent;
|
||||
use std::any::Any;
|
||||
@@ -56,8 +57,9 @@ pub(crate) trait BottomPaneView<'a> {
|
||||
fn try_consume_approval_request(
|
||||
&mut self,
|
||||
request: ApprovalRequest,
|
||||
) -> Option<ApprovalRequest> {
|
||||
Some(request)
|
||||
ticket: BackgroundOrderTicket,
|
||||
) -> Option<(ApprovalRequest, BackgroundOrderTicket)> {
|
||||
Some((request, ticket))
|
||||
}
|
||||
|
||||
/// Handle pasted text while this view is active. Return whether a redraw
|
||||
|
||||
@@ -17,6 +17,7 @@ use ratatui::widgets::{Block, Borders, Clear, Paragraph, Widget};
|
||||
use crate::account_label::{account_display_label, account_mode_priority};
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
|
||||
use super::bottom_pane_view::{BottomPaneView, ConditionalUpdate};
|
||||
use super::form_text_field::FormTextField;
|
||||
@@ -28,8 +29,16 @@ pub(crate) struct LoginAccountsView {
|
||||
}
|
||||
|
||||
impl LoginAccountsView {
|
||||
pub fn new(codex_home: PathBuf, app_event_tx: AppEventSender) -> (Self, Rc<RefCell<LoginAccountsState>>) {
|
||||
let state = Rc::new(RefCell::new(LoginAccountsState::new(codex_home, app_event_tx)));
|
||||
pub fn new(
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
) -> (Self, Rc<RefCell<LoginAccountsState>>) {
|
||||
let state = Rc::new(RefCell::new(LoginAccountsState::new(
|
||||
codex_home,
|
||||
app_event_tx,
|
||||
tail_ticket,
|
||||
)));
|
||||
(Self { state: state.clone() }, state)
|
||||
}
|
||||
}
|
||||
@@ -88,6 +97,7 @@ enum ViewMode {
|
||||
pub(crate) struct LoginAccountsState {
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
accounts: Vec<AccountRow>,
|
||||
active_account_id: Option<String>,
|
||||
selected: usize,
|
||||
@@ -97,10 +107,15 @@ pub(crate) struct LoginAccountsState {
|
||||
}
|
||||
|
||||
impl LoginAccountsState {
|
||||
fn new(codex_home: PathBuf, app_event_tx: AppEventSender) -> Self {
|
||||
fn new(
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
) -> Self {
|
||||
let mut state = Self {
|
||||
codex_home,
|
||||
app_event_tx,
|
||||
tail_ticket,
|
||||
accounts: Vec::new(),
|
||||
active_account_id: None,
|
||||
selected: 0,
|
||||
@@ -113,6 +128,11 @@ impl LoginAccountsState {
|
||||
state
|
||||
}
|
||||
|
||||
fn send_tail(&self, message: impl Into<String>) {
|
||||
self.app_event_tx
|
||||
.send_background_event_with_ticket(&self.tail_ticket, message);
|
||||
}
|
||||
|
||||
pub fn weak_handle(state: &Rc<RefCell<Self>>) -> Weak<RefCell<Self>> {
|
||||
Rc::downgrade(state)
|
||||
}
|
||||
@@ -276,8 +296,7 @@ impl LoginAccountsState {
|
||||
let mode = account.mode;
|
||||
if self.activate_account(account.id.clone(), mode) {
|
||||
self.mode = ViewMode::List;
|
||||
self.app_event_tx
|
||||
.send_background_event(format!("Switched to {label}"));
|
||||
self.send_tail(format!("Switched to {label}"));
|
||||
self.is_complete = true;
|
||||
}
|
||||
}
|
||||
@@ -560,8 +579,16 @@ pub(crate) struct LoginAddAccountView {
|
||||
}
|
||||
|
||||
impl LoginAddAccountView {
|
||||
pub fn new(codex_home: PathBuf, app_event_tx: AppEventSender) -> (Self, Rc<RefCell<LoginAddAccountState>>) {
|
||||
let state = Rc::new(RefCell::new(LoginAddAccountState::new(codex_home, app_event_tx)));
|
||||
pub fn new(
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
) -> (Self, Rc<RefCell<LoginAddAccountState>>) {
|
||||
let state = Rc::new(RefCell::new(LoginAddAccountState::new(
|
||||
codex_home,
|
||||
app_event_tx,
|
||||
tail_ticket,
|
||||
)));
|
||||
(Self { state: state.clone() }, state)
|
||||
}
|
||||
}
|
||||
@@ -598,22 +625,33 @@ enum AddStep {
|
||||
pub(crate) struct LoginAddAccountState {
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
step: AddStep,
|
||||
feedback: Option<Feedback>,
|
||||
is_complete: bool,
|
||||
}
|
||||
|
||||
impl LoginAddAccountState {
|
||||
fn new(codex_home: PathBuf, app_event_tx: AppEventSender) -> Self {
|
||||
fn new(
|
||||
codex_home: PathBuf,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
) -> Self {
|
||||
Self {
|
||||
codex_home,
|
||||
app_event_tx,
|
||||
tail_ticket,
|
||||
step: AddStep::Choose { selected: 0 },
|
||||
feedback: None,
|
||||
is_complete: false,
|
||||
}
|
||||
}
|
||||
|
||||
fn send_tail(&self, message: impl Into<String>) {
|
||||
self.app_event_tx
|
||||
.send_background_event_with_ticket(&self.tail_ticket, message);
|
||||
}
|
||||
|
||||
pub fn weak_handle(state: &Rc<RefCell<Self>>) -> Weak<RefCell<Self>> {
|
||||
Rc::downgrade(state)
|
||||
}
|
||||
@@ -660,8 +698,7 @@ impl LoginAddAccountState {
|
||||
message: "API key connected".to_string(),
|
||||
is_error: false,
|
||||
});
|
||||
self.app_event_tx
|
||||
.send_background_event("Added API key account");
|
||||
self.send_tail("Added API key account".to_string());
|
||||
self.app_event_tx
|
||||
.send(AppEvent::LoginUsingChatGptChanged { using_chatgpt_auth: false });
|
||||
self.finish_and_show_accounts();
|
||||
@@ -827,9 +864,8 @@ impl LoginAddAccountState {
|
||||
pub fn on_chatgpt_complete(&mut self, result: Result<(), String>) {
|
||||
match result {
|
||||
Ok(()) => {
|
||||
self.feedback = Some(Feedback { message: "ChatGPT account connected".to_string(), is_error: false });
|
||||
self.app_event_tx
|
||||
.send_background_event("ChatGPT account connected");
|
||||
self.feedback = Some(Feedback { message: "ChatGPT account connected".to_string(), is_error: false });
|
||||
self.send_tail("ChatGPT account connected".to_string());
|
||||
self.app_event_tx
|
||||
.send(AppEvent::LoginUsingChatGptChanged { using_chatgpt_auth: true });
|
||||
self.finish_and_show_accounts();
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::user_approval_widget::ApprovalRequest;
|
||||
use bottom_pane_view::BottomPaneView;
|
||||
use crate::util::buffer::fill_rect;
|
||||
@@ -530,21 +531,25 @@ impl BottomPane<'_> {
|
||||
}
|
||||
|
||||
/// Called when the agent requests user approval.
|
||||
pub fn push_approval_request(&mut self, request: ApprovalRequest) {
|
||||
let request = if let Some(view) = self.active_view.as_mut() {
|
||||
match view.try_consume_approval_request(request) {
|
||||
Some(request) => request,
|
||||
pub fn push_approval_request(
|
||||
&mut self,
|
||||
request: ApprovalRequest,
|
||||
ticket: BackgroundOrderTicket,
|
||||
) {
|
||||
let (request, ticket) = if let Some(view) = self.active_view.as_mut() {
|
||||
match view.try_consume_approval_request(request, ticket.clone()) {
|
||||
Some((request, ticket)) => (request, ticket),
|
||||
None => {
|
||||
self.request_redraw();
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
request
|
||||
(request, ticket)
|
||||
};
|
||||
|
||||
// Otherwise create a new approval modal overlay.
|
||||
let modal = ApprovalModalView::new(request, self.app_event_tx.clone());
|
||||
let modal = ApprovalModalView::new(request, ticket, self.app_event_tx.clone());
|
||||
self.active_view = Some(Box::new(modal));
|
||||
self.active_view_kind = ActiveViewKind::Other;
|
||||
// Hide any overlay status while a modal is visible.
|
||||
@@ -569,10 +574,17 @@ impl BottomPane<'_> {
|
||||
}
|
||||
|
||||
/// Show the theme selection UI
|
||||
pub fn show_theme_selection(&mut self, _current_theme: ThemeName) {
|
||||
pub fn show_theme_selection(
|
||||
&mut self,
|
||||
_current_theme: ThemeName,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
before_ticket: BackgroundOrderTicket,
|
||||
) {
|
||||
let view = ThemeSelectionView::new(
|
||||
crate::theme::current_theme_name(),
|
||||
self.app_event_tx.clone(),
|
||||
tail_ticket,
|
||||
before_ticket,
|
||||
);
|
||||
self.active_view = Some(Box::new(view));
|
||||
self.active_view_kind = ActiveViewKind::Other;
|
||||
@@ -1045,6 +1057,7 @@ impl WidgetRef for &BottomPane<'_> {
|
||||
mod tests_removed {
|
||||
use super::*;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use std::sync::mpsc::channel;
|
||||
@@ -1067,7 +1080,7 @@ mod tests_removed {
|
||||
enhanced_keys_supported: false,
|
||||
using_chatgpt_auth: false,
|
||||
});
|
||||
pane.push_approval_request(exec_request());
|
||||
pane.push_approval_request(exec_request(), BackgroundOrderTicket::test_for_request(1));
|
||||
assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c());
|
||||
assert!(pane.ctrl_c_quit_hint_visible());
|
||||
assert_eq!(CancellationEvent::Ignored, pane.on_ctrl_c());
|
||||
@@ -1085,7 +1098,7 @@ mod tests_removed {
|
||||
});
|
||||
|
||||
// Create an approval modal (active view).
|
||||
pane.push_approval_request(exec_request());
|
||||
pane.push_approval_request(exec_request(), BackgroundOrderTicket::test_for_request(1));
|
||||
// Attempt to update status; this should NOT create an overlay while modal is visible.
|
||||
pane.update_status_text("running command".to_string());
|
||||
|
||||
@@ -1119,7 +1132,7 @@ mod tests_removed {
|
||||
pane.update_status_text("waiting for model".to_string());
|
||||
|
||||
// Push an approval modal (e.g., command approval) which should hide the status view.
|
||||
pane.push_approval_request(exec_request());
|
||||
pane.push_approval_request(exec_request(), BackgroundOrderTicket::test_for_request(1));
|
||||
|
||||
// Simulate pressing 'n' (No) on the modal.
|
||||
use crossterm::event::KeyCode;
|
||||
|
||||
@@ -7,6 +7,7 @@ use ratatui::widgets::{Block, Borders, Clear, Paragraph, Widget};
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
|
||||
use super::bottom_pane_view::BottomPaneView;
|
||||
use super::BottomPane;
|
||||
@@ -20,15 +21,21 @@ pub(crate) enum NotificationsMode {
|
||||
pub(crate) struct NotificationsSettingsView {
|
||||
mode: NotificationsMode,
|
||||
app_event_tx: AppEventSender,
|
||||
ticket: BackgroundOrderTicket,
|
||||
selected_row: usize,
|
||||
is_complete: bool,
|
||||
}
|
||||
|
||||
impl NotificationsSettingsView {
|
||||
pub fn new(mode: NotificationsMode, app_event_tx: AppEventSender) -> Self {
|
||||
pub fn new(
|
||||
mode: NotificationsMode,
|
||||
app_event_tx: AppEventSender,
|
||||
ticket: BackgroundOrderTicket,
|
||||
) -> Self {
|
||||
Self {
|
||||
mode,
|
||||
app_event_tx,
|
||||
ticket,
|
||||
selected_row: 0,
|
||||
is_complete: false,
|
||||
}
|
||||
@@ -47,11 +54,15 @@ impl NotificationsSettingsView {
|
||||
} else {
|
||||
entries.join(", ")
|
||||
};
|
||||
self.app_event_tx.send_background_event(format!(
|
||||
"TUI notifications are filtered in config: [{}]",
|
||||
filters
|
||||
));
|
||||
self.app_event_tx.send_background_event(
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
format!(
|
||||
"TUI notifications are filtered in config: [{}]",
|
||||
filters
|
||||
),
|
||||
);
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
"Edit ~/.code/config.toml [tui].notifications to change filters.".to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ use ratatui::widgets::Widget;
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::theme::{custom_theme_is_dark, map_theme_for_palette, palette_mode, PaletteMode};
|
||||
|
||||
use super::BottomPane;
|
||||
@@ -42,11 +43,18 @@ pub(crate) struct ThemeSelectionView {
|
||||
just_entered_themes: bool,
|
||||
just_entered_spinner: bool,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
before_ticket: BackgroundOrderTicket,
|
||||
is_complete: bool,
|
||||
}
|
||||
|
||||
impl ThemeSelectionView {
|
||||
pub fn new(current_theme: ThemeName, app_event_tx: AppEventSender) -> Self {
|
||||
pub fn new(
|
||||
current_theme: ThemeName,
|
||||
app_event_tx: AppEventSender,
|
||||
tail_ticket: BackgroundOrderTicket,
|
||||
before_ticket: BackgroundOrderTicket,
|
||||
) -> Self {
|
||||
let current_theme = map_theme_for_palette(current_theme, custom_theme_is_dark());
|
||||
let themes = Self::get_theme_options();
|
||||
let selected_theme_index = themes
|
||||
@@ -76,6 +84,8 @@ impl ThemeSelectionView {
|
||||
just_entered_themes: false,
|
||||
just_entered_spinner: false,
|
||||
app_event_tx,
|
||||
tail_ticket,
|
||||
before_ticket,
|
||||
is_complete: false,
|
||||
}
|
||||
}
|
||||
@@ -295,6 +305,18 @@ impl ThemeSelectionView {
|
||||
self.mode = Mode::Overview;
|
||||
}
|
||||
|
||||
fn send_tail(&self, message: impl Into<String>) {
|
||||
self.app_event_tx
|
||||
.send_background_event_with_ticket(&self.tail_ticket, message);
|
||||
}
|
||||
|
||||
fn send_before_next_output(&self, message: impl Into<String>) {
|
||||
self.app_event_tx.send_background_before_next_output_with_ticket(
|
||||
&self.before_ticket,
|
||||
message,
|
||||
);
|
||||
}
|
||||
|
||||
/// Spawn a background task that creates a custom spinner using the LLM with a JSON schema
|
||||
fn kickoff_spinner_creation(
|
||||
&self,
|
||||
@@ -302,6 +324,7 @@ impl ThemeSelectionView {
|
||||
progress_tx: std::sync::mpsc::Sender<ProgressMsg>,
|
||||
) {
|
||||
let tx = self.app_event_tx.clone();
|
||||
let before_ticket = self.before_ticket.clone();
|
||||
std::thread::spawn(move || {
|
||||
let rt = match tokio::runtime::Builder::new_multi_thread()
|
||||
.enable_all()
|
||||
@@ -309,10 +332,10 @@ impl ThemeSelectionView {
|
||||
{
|
||||
Ok(rt) => rt,
|
||||
Err(e) => {
|
||||
tx.send_background_event_before_next_output(format!(
|
||||
"Failed to start runtime: {}",
|
||||
e
|
||||
));
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Failed to start runtime: {}", e),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -320,7 +343,13 @@ impl ThemeSelectionView {
|
||||
// Load current config (CLI-style) and construct a one-off ModelClient
|
||||
let cfg = match codex_core::config::Config::load_with_cli_overrides(vec![], codex_core::config::ConfigOverrides::default()) {
|
||||
Ok(c) => c,
|
||||
Err(e) => { tx.send_background_event_before_next_output(format!("Config error: {}", e)); return; }
|
||||
Err(e) => {
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Config error: {}", e),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
// Use the same auth preference as the active Codex session.
|
||||
// When logged in with ChatGPT, prefer ChatGPT auth; otherwise fall back to API key.
|
||||
@@ -383,7 +412,10 @@ impl ThemeSelectionView {
|
||||
let mut stream = match client.stream(&prompt).await {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
tx.send_background_event_before_next_output(format!("Request error: {}", e));
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Request error: {}", e),
|
||||
);
|
||||
tracing::info!("spinner request error: {}", e);
|
||||
return;
|
||||
}
|
||||
@@ -610,6 +642,7 @@ impl ThemeSelectionView {
|
||||
example["colors"]["progress"] = serde_json::Value::String(v);
|
||||
}
|
||||
|
||||
let before_ticket = self.before_ticket.clone();
|
||||
std::thread::spawn(move || {
|
||||
let rt = match tokio::runtime::Builder::new_multi_thread()
|
||||
.enable_all()
|
||||
@@ -617,17 +650,23 @@ impl ThemeSelectionView {
|
||||
{
|
||||
Ok(rt) => rt,
|
||||
Err(e) => {
|
||||
tx.send_background_event_before_next_output(format!(
|
||||
"Failed to start runtime: {}",
|
||||
e
|
||||
));
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Failed to start runtime: {}", e),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let _ = rt.block_on(async move {
|
||||
let cfg = match codex_core::config::Config::load_with_cli_overrides(vec![], codex_core::config::ConfigOverrides::default()) {
|
||||
Ok(c) => c,
|
||||
Err(e) => { tx.send_background_event_before_next_output(format!("Config error: {}", e)); return; }
|
||||
Err(e) => {
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Config error: {}", e),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let auth_mgr = codex_core::AuthManager::shared_with_mode_and_originator(
|
||||
cfg.codex_home.clone(),
|
||||
@@ -709,7 +748,10 @@ impl ThemeSelectionView {
|
||||
let mut stream = match client.stream(&prompt).await {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
tx.send_background_event_before_next_output(format!("Request error: {}", e));
|
||||
tx.send_background_before_next_output_with_ticket(
|
||||
&before_ticket,
|
||||
format!("Request error: {}", e),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -1301,9 +1343,7 @@ impl<'a> BottomPaneView<'a> for ThemeSelectionView {
|
||||
self.current_spinner = "custom".to_string();
|
||||
self.app_event_tx
|
||||
.send(AppEvent::UpdateSpinner("custom".to_string()));
|
||||
self
|
||||
.app_event_tx
|
||||
.send_background_event("Custom spinner saved".to_string());
|
||||
self.send_tail("Custom spinner saved".to_string());
|
||||
go_overview = true;
|
||||
}
|
||||
} else {
|
||||
@@ -1432,20 +1472,15 @@ impl<'a> BottomPaneView<'a> for ThemeSelectionView {
|
||||
}
|
||||
// Informative status depending on whether we set active
|
||||
if s.preview_on.get() {
|
||||
self
|
||||
.app_event_tx
|
||||
.send_background_event_before_next_output(
|
||||
format!("Set theme to {}", name),
|
||||
);
|
||||
self.send_before_next_output(format!(
|
||||
"Set theme to {}",
|
||||
name
|
||||
));
|
||||
} else {
|
||||
self
|
||||
.app_event_tx
|
||||
.send_background_event_before_next_output(
|
||||
format!(
|
||||
"Saved custom theme {} (not active)",
|
||||
name
|
||||
),
|
||||
);
|
||||
self.send_before_next_output(format!(
|
||||
"Saved custom theme {} (not active)",
|
||||
name
|
||||
));
|
||||
}
|
||||
go_overview = true;
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::sync::{Arc, Mutex};
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::colors;
|
||||
use crate::util::buffer::fill_rect;
|
||||
use super::bottom_pane_view::BottomPaneView;
|
||||
@@ -23,6 +24,7 @@ pub struct UpdateSharedState {
|
||||
|
||||
pub(crate) struct UpdateSettingsView {
|
||||
app_event_tx: AppEventSender,
|
||||
ticket: BackgroundOrderTicket,
|
||||
field: usize,
|
||||
is_complete: bool,
|
||||
auto_enabled: bool,
|
||||
@@ -36,6 +38,7 @@ pub(crate) struct UpdateSettingsView {
|
||||
impl UpdateSettingsView {
|
||||
pub fn new(
|
||||
app_event_tx: AppEventSender,
|
||||
ticket: BackgroundOrderTicket,
|
||||
current_version: String,
|
||||
auto_enabled: bool,
|
||||
command: Option<Vec<String>>,
|
||||
@@ -45,6 +48,7 @@ impl UpdateSettingsView {
|
||||
) -> Self {
|
||||
Self {
|
||||
app_event_tx,
|
||||
ticket,
|
||||
field: 0,
|
||||
is_complete: false,
|
||||
auto_enabled,
|
||||
@@ -69,49 +73,61 @@ impl UpdateSettingsView {
|
||||
.expect("update shared state poisoned")
|
||||
.clone();
|
||||
|
||||
if self.command.is_none() {
|
||||
if let Some(instructions) = &self.manual_instructions {
|
||||
self.app_event_tx
|
||||
.send_background_event(instructions.clone());
|
||||
if self.command.is_none() {
|
||||
if let Some(instructions) = &self.manual_instructions {
|
||||
self.app_event_tx
|
||||
.send_background_event_with_ticket(&self.ticket, instructions.clone());
|
||||
}
|
||||
return;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if state.checking {
|
||||
self.app_event_tx
|
||||
.send_background_event("Still checking for updates…".to_string());
|
||||
return;
|
||||
}
|
||||
if let Some(err) = &state.error {
|
||||
self.app_event_tx
|
||||
.send_background_event(format!("❌ /update failed: {err}"));
|
||||
return;
|
||||
}
|
||||
let Some(latest) = state.latest_version.clone() else {
|
||||
self.app_event_tx
|
||||
.send_background_event("✅ Code is already up to date.".to_string());
|
||||
return;
|
||||
};
|
||||
if state.checking {
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
"Still checking for updates…".to_string(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
if let Some(err) = &state.error {
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
format!("❌ /update failed: {err}"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
let Some(latest) = state.latest_version.clone() else {
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
"✅ Code is already up to date.".to_string(),
|
||||
);
|
||||
return;
|
||||
};
|
||||
|
||||
let command = self.command.clone().expect("command checked above");
|
||||
let command = self.command.clone().expect("command checked above");
|
||||
let display = self
|
||||
.command_display
|
||||
.clone()
|
||||
.unwrap_or_else(|| command.join(" "));
|
||||
|
||||
self.app_event_tx.send_background_event(format!(
|
||||
"⬆️ Update available: {} → {}. Opening guided upgrade with `{}`…",
|
||||
self.current_version, latest, display
|
||||
));
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
format!(
|
||||
"⬆️ Update available: {} → {}. Opening guided upgrade with `{}`…",
|
||||
self.current_version, latest, display
|
||||
),
|
||||
);
|
||||
self.app_event_tx.send(AppEvent::RunUpdateCommand {
|
||||
command,
|
||||
display: display.clone(),
|
||||
latest_version: Some(latest.clone()),
|
||||
});
|
||||
self.app_event_tx.send_background_event(format!(
|
||||
"↻ Complete the guided terminal steps for `{}` then restart Code to finish upgrading to {}.",
|
||||
display, latest
|
||||
));
|
||||
self.app_event_tx.send_background_event_with_ticket(
|
||||
&self.ticket,
|
||||
format!(
|
||||
"↻ Complete the guided terminal steps for `{}` then restart Code to finish upgrading to {}.",
|
||||
display, latest
|
||||
),
|
||||
);
|
||||
self.is_complete = true;
|
||||
}
|
||||
|
||||
|
||||
+364
-181
File diff suppressed because it is too large
Load Diff
@@ -1002,10 +1002,14 @@ pub(super) fn handle_exec_end_now(
|
||||
chat
|
||||
.bottom_pane
|
||||
.update_status_text("command completed".to_string());
|
||||
let gh_ticket = chat.make_background_tail_ticket();
|
||||
let tx = chat.app_event_tx.clone();
|
||||
let cfg = chat.config.clone();
|
||||
crate::chatwidget::gh_actions::maybe_watch_after_push(
|
||||
chat.app_event_tx.clone(),
|
||||
chat.config.clone(),
|
||||
tx,
|
||||
cfg,
|
||||
&command_for_watch,
|
||||
gh_ticket,
|
||||
);
|
||||
} else {
|
||||
chat
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::git_info::collect_git_info;
|
||||
use std::process::Command;
|
||||
@@ -30,7 +31,12 @@ pub(super) fn get_github_token() -> Option<(String, TokenSource)> {
|
||||
/// Start a background task to watch GitHub Actions for the latest push and
|
||||
/// surface a failure message if any run for the pushed commit completes with a
|
||||
/// non-success conclusion.
|
||||
pub(super) fn maybe_watch_after_push(app_event_tx: AppEventSender, config: Config, command: &[String]) {
|
||||
pub(super) fn maybe_watch_after_push(
|
||||
app_event_tx: AppEventSender,
|
||||
config: Config,
|
||||
command: &[String],
|
||||
ticket: BackgroundOrderTicket,
|
||||
) {
|
||||
// Only proceed when enabled in config and the command appears to be a git push.
|
||||
if !config.github.check_workflows_on_push {
|
||||
return;
|
||||
@@ -43,6 +49,7 @@ pub(super) fn maybe_watch_after_push(app_event_tx: AppEventSender, config: Confi
|
||||
// Spawn a detached task so we don't block UI; clone what we need.
|
||||
let tx = app_event_tx.clone();
|
||||
tokio::spawn(async move {
|
||||
let ticket = ticket.clone();
|
||||
// Gather repo info (branch, sha, origin URL) from the current cwd.
|
||||
let Some(git) = collect_git_info(&config.cwd).await else { return; };
|
||||
let (Some(head_sha), branch_opt, Some(repo_url)) = (git.commit_hash, git.branch, git.repository_url) else { return; };
|
||||
@@ -89,7 +96,16 @@ pub(super) fn maybe_watch_after_push(app_event_tx: AppEventSender, config: Confi
|
||||
let conclusion = run.get("conclusion").and_then(|v| v.as_str()).unwrap_or("unknown");
|
||||
if conclusion != "success" {
|
||||
let html = run.get("html_url").and_then(|v| v.as_str()).unwrap_or("");
|
||||
surface_failure(&tx, &owner, &repo, &branch, &head_sha, html, conclusion);
|
||||
surface_failure(
|
||||
&tx,
|
||||
&ticket,
|
||||
&owner,
|
||||
&repo,
|
||||
&branch,
|
||||
&head_sha,
|
||||
html,
|
||||
conclusion,
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -112,7 +128,16 @@ pub(super) fn maybe_watch_after_push(app_event_tx: AppEventSender, config: Confi
|
||||
let conclusion = run.get("conclusion").and_then(|v| v.as_str()).unwrap_or("unknown");
|
||||
let html = run.get("html_url").and_then(|v| v.as_str()).unwrap_or("");
|
||||
if conclusion != "success" {
|
||||
surface_failure(&tx, &owner, &repo, &branch, &head_sha, html, conclusion);
|
||||
surface_failure(
|
||||
&tx,
|
||||
&ticket,
|
||||
&owner,
|
||||
&repo,
|
||||
&branch,
|
||||
&head_sha,
|
||||
html,
|
||||
conclusion,
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -148,14 +173,23 @@ fn parse_owner_repo(url: &str) -> Option<(String, String)> {
|
||||
None
|
||||
}
|
||||
|
||||
fn surface_failure(tx: &AppEventSender, owner: &str, repo: &str, branch: &str, sha: &str, url: &str, conclusion: &str) {
|
||||
fn surface_failure(
|
||||
tx: &AppEventSender,
|
||||
ticket: &BackgroundOrderTicket,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
branch: &str,
|
||||
sha: &str,
|
||||
url: &str,
|
||||
conclusion: &str,
|
||||
) {
|
||||
let short = &sha[..std::cmp::min(7, sha.len())];
|
||||
let msg = if url.is_empty() {
|
||||
format!("❌ GitHub Actions failed for {owner}/{repo}@{short} on {branch}: {conclusion}")
|
||||
} else {
|
||||
format!("❌ GitHub Actions failed for {owner}/{repo}@{short} on {branch}: {conclusion} — {url}")
|
||||
};
|
||||
tx.send_background_event(msg);
|
||||
tx.send_background_event_with_ticket(ticket, msg);
|
||||
}
|
||||
|
||||
async fn sleep_ms(ms: u64) { tokio::time::sleep(std::time::Duration::from_millis(ms)).await; }
|
||||
|
||||
@@ -1222,15 +1222,17 @@ fn observer_report_replaces_prompt_and_resets_countdown() {
|
||||
fn background_event_before_next_output_precedes_later_cells() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
|
||||
|
||||
let tail_ticket = chat.make_background_tail_ticket();
|
||||
chat.insert_background_event_with_placement(
|
||||
"initial".to_string(),
|
||||
BackgroundPlacement::Tail,
|
||||
None,
|
||||
Some(tail_ticket.next_order()),
|
||||
);
|
||||
let before_ticket = chat.make_background_before_next_output_ticket();
|
||||
chat.insert_background_event_with_placement(
|
||||
"guard".to_string(),
|
||||
BackgroundPlacement::BeforeNextOutput,
|
||||
None,
|
||||
Some(before_ticket.next_order()),
|
||||
);
|
||||
chat.push_background_tail("tail".to_string());
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ use shlex::split as shlex_split;
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::chatwidget::BackgroundOrderTicket;
|
||||
use crate::exec_command::strip_bash_lc_and_escape;
|
||||
use codex_core::protocol::ApprovedCommandMatchKind;
|
||||
|
||||
@@ -72,6 +73,7 @@ enum SelectAction {
|
||||
pub(crate) struct UserApprovalWidget<'a> {
|
||||
approval_request: ApprovalRequest,
|
||||
app_event_tx: AppEventSender,
|
||||
before_ticket: BackgroundOrderTicket,
|
||||
confirmation_prompt: Paragraph<'a>,
|
||||
select_options: Vec<SelectOption>,
|
||||
|
||||
@@ -84,7 +86,11 @@ pub(crate) struct UserApprovalWidget<'a> {
|
||||
}
|
||||
|
||||
impl UserApprovalWidget<'_> {
|
||||
pub(crate) fn new(approval_request: ApprovalRequest, app_event_tx: AppEventSender) -> Self {
|
||||
pub(crate) fn new(
|
||||
approval_request: ApprovalRequest,
|
||||
before_ticket: BackgroundOrderTicket,
|
||||
app_event_tx: AppEventSender,
|
||||
) -> Self {
|
||||
let confirmation_prompt = match &approval_request {
|
||||
ApprovalRequest::Exec {
|
||||
command, reason, ..
|
||||
@@ -154,6 +160,7 @@ impl UserApprovalWidget<'_> {
|
||||
Self {
|
||||
approval_request,
|
||||
app_event_tx,
|
||||
before_ticket,
|
||||
confirmation_prompt,
|
||||
select_options,
|
||||
selected_option: 0,
|
||||
@@ -276,9 +283,8 @@ impl UserApprovalWidget<'_> {
|
||||
format!("{}\nfeedback:\n{}", message, feedback)
|
||||
};
|
||||
// Insert above the upcoming command begin so the decision reads first.
|
||||
self
|
||||
.app_event_tx
|
||||
.send_background_event_before_next_output(message);
|
||||
self.app_event_tx
|
||||
.send_background_before_next_output_with_ticket(&self.before_ticket, message);
|
||||
|
||||
// If the user aborted an exec approval, immediately cancel any running task
|
||||
// so the UI reflects their intent (clear spinner/status) without waiting
|
||||
@@ -572,7 +578,7 @@ mod tests {
|
||||
command: vec!["echo".to_string()],
|
||||
reason: None,
|
||||
};
|
||||
let mut widget = UserApprovalWidget::new(req, tx);
|
||||
let mut widget = UserApprovalWidget::new(req, BackgroundOrderTicket::test_for_request(1), tx);
|
||||
widget.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
|
||||
assert!(widget.is_complete());
|
||||
let events: Vec<AppEvent> = rx.try_iter().collect();
|
||||
@@ -594,7 +600,7 @@ mod tests {
|
||||
command: vec!["git".into(), "status".into()],
|
||||
reason: None,
|
||||
};
|
||||
let mut widget = UserApprovalWidget::new(req, tx);
|
||||
let mut widget = UserApprovalWidget::new(req, BackgroundOrderTicket::test_for_request(1), tx);
|
||||
widget.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE));
|
||||
|
||||
let events: Vec<AppEvent> = rx.try_iter().collect();
|
||||
@@ -623,7 +629,7 @@ mod tests {
|
||||
],
|
||||
reason: None,
|
||||
};
|
||||
let mut widget = UserApprovalWidget::new(req, tx);
|
||||
let mut widget = UserApprovalWidget::new(req, BackgroundOrderTicket::test_for_request(1), tx);
|
||||
widget.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE));
|
||||
|
||||
let events: Vec<AppEvent> = rx.try_iter().collect();
|
||||
@@ -680,7 +686,7 @@ mod tests {
|
||||
command: vec!["echo".to_string()],
|
||||
reason: None,
|
||||
};
|
||||
let mut widget = UserApprovalWidget::new(req, tx);
|
||||
let mut widget = UserApprovalWidget::new(req, BackgroundOrderTicket::test_for_request(1), tx);
|
||||
widget.handle_key_event(KeyEvent::new(KeyCode::Char('Y'), KeyModifiers::NONE));
|
||||
assert!(widget.is_complete());
|
||||
let events: Vec<AppEvent> = rx.try_iter().collect();
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
# Background Ordering Refactor Plan
|
||||
|
||||
## Objective
|
||||
|
||||
Guarantee that all UI background events carry immutable order metadata captured at the moment the originating action begins, so banners can no longer drift into later turns. The refactor removes the fallback heuristic that re-evaluates global UI state at delivery time and replaces it with explicit handles that freeze the request ordinal and sequence.
|
||||
|
||||
## Scope
|
||||
|
||||
- `codex-rs/tui/src/chatwidget.rs`
|
||||
- `codex-rs/tui/src/app_event_sender.rs`
|
||||
- Async producers that currently call `AppEventSender::send_background_event*`
|
||||
- Associated unit tests under `codex-rs/tui/src/chatwidget.rs`
|
||||
|
||||
## Constraints & Signals
|
||||
|
||||
- Preserve existing `OrderMeta` semantics (`output_index = i32::MAX`) for tail background notices.
|
||||
- Avoid regressions to snapshot/restore flows that rely on `UiBackgroundOrderHandle` sequence rehydration.
|
||||
- Maintain compatibility for `BackgroundPlacement::BeforeNextOutput`, but refuse tail inserts without an explicit order.
|
||||
- Update or add tests covering:
|
||||
- Slash commands (`/branch`) emitting banners after user prompts without a provider turn.
|
||||
- Ghost snapshot timeout notices.
|
||||
- Restored sessions emitting late events.
|
||||
|
||||
## Implementation Strategy
|
||||
|
||||
### 1. Promote Order Handles to Public API
|
||||
|
||||
- Expose a new lightweight wrapper (e.g., `BackgroundOrderTicket`) that encapsulates `UiBackgroundOrderHandle`.
|
||||
- Store it in `codex-rs/tui/src/app_event_sender.rs` so async producers can hold a clone even after moving off the main thread.
|
||||
- Provide constructors on `ChatWidget`:
|
||||
- `fn make_background_tail_ticket(&mut self) -> BackgroundOrderTicket`
|
||||
- `fn make_background_before_next_output_ticket(&mut self) -> BackgroundOrderTicket`
|
||||
|
||||
### 2. Eliminate Orderless Tail Sends
|
||||
|
||||
- Deprecate `AppEventSender::send_background_event` and `send_background_event_before_next_output` by replacing them with:
|
||||
- `send_background_event_with_ticket(ticket, message)`
|
||||
- `send_background_before_next_output_with_ticket(ticket, message)`
|
||||
- Enforce at compile-time: no tail placement path accepts `order: None`.
|
||||
- Update `AppEvent::InsertBackgroundEvent` producers to require an `OrderMeta` (or ticket) argument.
|
||||
|
||||
### 3. Capture Tickets at Slash Command Dispatch
|
||||
|
||||
- In `ChatWidget::handle_branch_command`, grab a tail ticket before spawning the async block and pass it into the closure.
|
||||
- Repeat for other slash commands (`/merge`, `/cmd`, `/update`, etc.) and any UI-only flows that spawn tasks.
|
||||
- For synchronous notices (e.g., “Creating branch worktree…”), use the “before next output” ticket captured before the prompt increments the pending counter.
|
||||
|
||||
### 4. Reset Pending Prompt State on UI-Only Commands
|
||||
|
||||
- Introduce a helper `fn consume_pending_prompt_for_ui_only_turn(&mut self)` that decrements the counter once the slash command response is queued.
|
||||
- Call it immediately after scheduling the command’s banners so the UI state reflects “current turn resolved.”
|
||||
- Ensure it does not fire when a provider request truly starts (`TaskStarted` still clears the counter).
|
||||
|
||||
### 5. Update Async Helpers & Services
|
||||
|
||||
- `ghost_snapshot.rs`, auto-upgrade, GitHub Actions watcher, browser connectors, and cloud task helpers must accept a ticket captured at trigger time.
|
||||
- For global helpers (`AppEventSender` clones passed into other modules), thread the ticket through their APIs so they no longer call the legacy `send_background_event`.
|
||||
|
||||
### 6. Remove Fallback Logic
|
||||
|
||||
- Delete the branch in `insert_background_event_with_placement` that synthesizes order metadata when `order.is_none()`.
|
||||
- Simplify `system_order_key` and `background_tail_request_ordinal` to assume `order` is always present for tail placements.
|
||||
- Add defensive logging/assertions that fire if new code attempts to insert without order metadata.
|
||||
|
||||
### 7. Expand Test Coverage
|
||||
|
||||
- Extend existing tests to cover the previously failing scenario:
|
||||
- Seed `pending_user_prompts_for_next_turn = 1`, capture a ticket, enqueue a tail event, assert `OrderKey.req` matches the current request.
|
||||
- Add async-style tests that simulate `/branch` completion without `TaskStarted`, verifying history ordering remains stable after subsequent user prompts.
|
||||
- Snapshot tests for restored sessions to confirm sequence rehydration still advances monotonically.
|
||||
|
||||
### 8. Migration & Cleanup
|
||||
|
||||
- Provide shims during transition (feature-gated) if needed, but plan to remove the old API once all call sites migrate.
|
||||
- Update developer docs to explain ticket usage for background events.
|
||||
- Run `./build-fast.sh` and targeted unit tests.
|
||||
|
||||
## Current Progress (2025-10-02)
|
||||
|
||||
### ✅ Step 1 – Ticket API
|
||||
|
||||
- `BackgroundOrderTicket` exposed with cloning semantics and production/test constructors.
|
||||
- Tail and before-next-output helpers (`make_background_tail_ticket`, `make_background_before_next_output_ticket`) now power all UI-issued banners.
|
||||
- Conversation bootstrap failures, `/branch`, ghost snapshots, auto-upgrade notices, GitHub Actions watcher, browser/CDP flows, cloud tasks, `/cloud` subcommands, and slash-command banners all mint tickets up front.
|
||||
|
||||
### ✅ Step 2 – Ticket Plumbing Throughout the UI
|
||||
|
||||
- All chatwidget flows (browser toggles, cloud tasks, branch/merge, Chrome connection, `/login`, `/update`, `/github`, etc.) now thread `BackgroundOrderTicket` instances into async closures and visible toasts.
|
||||
- `consume_pending_prompt_for_ui_only_turn()` clears stale prompt counters so slash commands no longer misclassify delayed banners as the “next” turn.
|
||||
- Bottom pane views (update settings, notifications, theme selection, login/add-account) and background helpers now require tickets at construction time.
|
||||
|
||||
### ✅ Step 3 – Approval & Modal flows
|
||||
|
||||
- `UserApprovalWidget` accepts a before-next-output ticket; decisions send ordered banners instead of relying on fallback synthesis.
|
||||
- `ApprovalModalView` queues `(ApprovalRequest, BackgroundOrderTicket)` pairs so multiple approvals preserve per-request ordinals.
|
||||
- Tests were updated to exercise the new APIs via `BackgroundOrderTicket::test_for_request`.
|
||||
|
||||
### ✅ Step 4 – Fallback Removal & API Cleanup
|
||||
|
||||
- `ChatWidget::insert_background_event_with_placement` now refuses tail inserts without explicit order metadata (logs and drops the message instead of synthesizing).
|
||||
- Deprecated helpers (`send_background_event`, `send_background_event_before_next_output`) were removed from `AppEventSender`.
|
||||
- `push_background_tail` and related helpers internally mint tickets; legacy `order: None` paths are gone.
|
||||
- Regression test `background_event_before_next_output_precedes_later_cells` now verifies ordered insertions using explicit tickets.
|
||||
|
||||
## Remaining Focus Areas
|
||||
|
||||
1. **Additional regression coverage** – add tests capturing:
|
||||
- Pending prompt > 0 when slash commands emit multi-stage banners.
|
||||
- Session restore + late async completion to ensure monotonic sequence rehydration.
|
||||
2. **Docs & developer guidance** – document ticket usage patterns for async producers (README/DEVELOPING or a dedicated dev note).
|
||||
3. **Manual QA** – run `/branch`, `/cloud`, browser toggles, approval modals, and login flows end-to-end to confirm grouping across user prompts.
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
- [ ] Add targeted unit tests covering pending prompt scenarios with explicit tickets.
|
||||
- [ ] Document ticket best practices for new async producers.
|
||||
- [ ] Manual walkthrough keeps `/branch` banners ordered after subsequent prompts.
|
||||
- [ ] Manual walkthrough keeps approval decisions adjacent to the associated command/tool begin cell.
|
||||
- [ ] `./build-fast.sh` and focused tests pass without warnings.
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
- All background banners stay adjacent to their originating prompt across repeated `/branch`, `/merge`, ghost snapshot, and auto-upgrade scenarios.
|
||||
- No warnings about “legacy background event without order” remain.
|
||||
- `tail_background_event_keeps_position_vs_next_output` passes with pending prompts set both to 0 and 1.
|
||||
- Manual TUI walkthrough confirms ordering stability after multiple delayed async completions.
|
||||
Reference in New Issue
Block a user