mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
ui changes
This commit is contained in:
@@ -0,0 +1 @@
|
||||
bye
|
||||
+109
-43
@@ -1074,6 +1074,47 @@ impl App {
|
||||
submitted
|
||||
}
|
||||
|
||||
async fn reject_patch_approval_with_notes(
|
||||
&mut self,
|
||||
thread_id: ThreadId,
|
||||
approval_id: String,
|
||||
text: String,
|
||||
) {
|
||||
let denied = self
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::PatchApproval {
|
||||
id: approval_id,
|
||||
decision: ReviewDecision::Denied,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
if !denied {
|
||||
return;
|
||||
}
|
||||
|
||||
let steered = self
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: text.clone(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
if steered && self.active_thread_id == Some(thread_id) {
|
||||
self.chat_widget.render_submitted_user_message(
|
||||
text,
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async fn refresh_pending_thread_approvals(&mut self) {
|
||||
let channels: Vec<(ThreadId, Arc<Mutex<ThreadEventStore>>)> = self
|
||||
.thread_event_channels
|
||||
@@ -2167,29 +2208,8 @@ impl App {
|
||||
approval_id,
|
||||
text,
|
||||
} => {
|
||||
let denied = self
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::PatchApproval {
|
||||
id: approval_id,
|
||||
decision: ReviewDecision::Denied,
|
||||
},
|
||||
)
|
||||
self.reject_patch_approval_with_notes(thread_id, approval_id, text)
|
||||
.await;
|
||||
if denied {
|
||||
let _ = self
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text,
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
AppEvent::DiffResult(text) => {
|
||||
// Clear the in-progress state in the bottom pane
|
||||
@@ -3881,6 +3901,41 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reject_patch_with_notes_renders_follow_up_in_active_transcript() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let thread_id = ThreadId::new();
|
||||
app.active_thread_id = Some(thread_id);
|
||||
|
||||
reject_patch_with_notes_for_test(
|
||||
&mut app,
|
||||
thread_id,
|
||||
"call-2",
|
||||
"i want the content to say bye instead.",
|
||||
)
|
||||
.await;
|
||||
drain_insert_history_cells(&mut app, &mut app_event_rx);
|
||||
|
||||
let user_messages: Vec<String> = app
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.filter_map(|cell| {
|
||||
cell.as_any()
|
||||
.downcast_ref::<UserHistoryCell>()
|
||||
.map(|cell| cell.message.clone())
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
user_messages,
|
||||
vec!["i want the content to say bye instead.".to_string()]
|
||||
);
|
||||
assert_snapshot!(
|
||||
"reject_patch_with_notes_follow_up_user_prompt",
|
||||
render_transcript_cells_for_snapshot(&app, 80)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reject_patch_with_notes_skips_steering_when_deny_fails() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
@@ -4652,31 +4707,42 @@ mod tests {
|
||||
approval_id: &str,
|
||||
text: &str,
|
||||
) {
|
||||
let denied = app
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::PatchApproval {
|
||||
id: approval_id.to_string(),
|
||||
decision: ReviewDecision::Denied,
|
||||
},
|
||||
)
|
||||
app.reject_patch_approval_with_notes(thread_id, approval_id.to_string(), text.to_string())
|
||||
.await;
|
||||
if denied {
|
||||
let _ = app
|
||||
.submit_op_to_thread(
|
||||
thread_id,
|
||||
Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: text.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
fn drain_insert_history_cells(
|
||||
app: &mut App,
|
||||
app_event_rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
) {
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
app.transcript_cells.push(cell.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn render_transcript_cells_for_snapshot(app: &App, width: u16) -> String {
|
||||
app.transcript_cells
|
||||
.iter()
|
||||
.enumerate()
|
||||
.flat_map(|(idx, cell)| {
|
||||
let mut lines = cell.display_lines(width);
|
||||
if idx > 0 && !lines.is_empty() {
|
||||
lines.insert(0, Line::from(""));
|
||||
}
|
||||
lines
|
||||
})
|
||||
.map(|line| {
|
||||
line.spans
|
||||
.iter()
|
||||
.map(|span| span.content.as_ref())
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
fn test_otel_manager(config: &Config, model: &str) -> OtelManager {
|
||||
let model_info = codex_core::test_support::construct_model_info_offline(model, config);
|
||||
OtelManager::new(
|
||||
|
||||
@@ -379,7 +379,7 @@ impl ApprovalOverlay {
|
||||
if self.patch_focus_is_notes() {
|
||||
"enter to send | tab to go back | esc to interrupt".to_string()
|
||||
} else {
|
||||
"enter or tab to edit rejection | esc to interrupt".to_string()
|
||||
"enter or tab to edit follow up | esc to interrupt".to_string()
|
||||
}
|
||||
} else if self.patch_selected_index() == Some(PATCH_REJECT_OPTION_INDEX) {
|
||||
"tab to follow up | esc to interrupt".to_string()
|
||||
|
||||
@@ -2571,10 +2571,14 @@ impl ChatWidget {
|
||||
&mut self,
|
||||
event: codex_protocol::protocol::PatchApplyEndEvent,
|
||||
) {
|
||||
// If the patch was successful, just let the "Edited" block stand.
|
||||
// Otherwise, add a failure block.
|
||||
if !event.success {
|
||||
self.add_to_history(history_cell::new_patch_apply_failure(event.stderr));
|
||||
match event.status {
|
||||
codex_protocol::protocol::PatchApplyStatus::Completed => {}
|
||||
codex_protocol::protocol::PatchApplyStatus::Failed => {
|
||||
self.add_to_history(history_cell::new_patch_apply_failure(event.stderr));
|
||||
}
|
||||
codex_protocol::protocol::PatchApplyStatus::Declined => {
|
||||
self.add_to_history(history_cell::new_patch_apply_declined());
|
||||
}
|
||||
}
|
||||
// Mark that actual work was done (patch applied)
|
||||
self.had_work_activity = true;
|
||||
@@ -4217,12 +4221,26 @@ impl ChatWidget {
|
||||
});
|
||||
}
|
||||
|
||||
// Show replayable user content in conversation history.
|
||||
let local_image_paths = local_images
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect::<Vec<_>>();
|
||||
self.render_submitted_user_message(
|
||||
text,
|
||||
text_elements,
|
||||
local_image_paths,
|
||||
remote_image_urls,
|
||||
);
|
||||
}
|
||||
|
||||
pub(crate) fn render_submitted_user_message(
|
||||
&mut self,
|
||||
text: String,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_image_paths: Vec<PathBuf>,
|
||||
remote_image_urls: Vec<String>,
|
||||
) {
|
||||
if !text.is_empty() {
|
||||
let local_image_paths = local_images
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect::<Vec<_>>();
|
||||
self.last_rendered_user_message_event =
|
||||
Some(Self::rendered_user_message_event_from_parts(
|
||||
text.clone(),
|
||||
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
assertion_line: 7580
|
||||
expression: rendered
|
||||
---
|
||||
• Accepting revision
|
||||
@@ -7558,6 +7558,32 @@ async fn apply_patch_events_emit_history_cells() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_patch_declined_shows_accepting_revision_without_detail() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "declined".into(),
|
||||
msg: EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: "call-declined".into(),
|
||||
turn_id: "turn-declined".into(),
|
||||
stdout: String::new(),
|
||||
stderr: "patch rejected by user".into(),
|
||||
success: false,
|
||||
changes: HashMap::new(),
|
||||
status: CorePatchApplyStatus::Declined,
|
||||
}),
|
||||
});
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = lines_to_single_string(cells.last().expect("declined patch cell"));
|
||||
assert_snapshot!("apply_patch_declined_accepting_revision", rendered);
|
||||
assert!(
|
||||
!rendered.contains("patch rejected by user"),
|
||||
"declined patch row should not show the low-level rejection detail"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_patch_manual_approval_adjusts_header() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
@@ -2162,8 +2162,7 @@ pub(crate) fn new_patch_event(
|
||||
pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
|
||||
// Failure title
|
||||
lines.push(Line::from("✘ Failed to apply patch".magenta().bold()));
|
||||
lines.push(vec!["• ".dim(), "Failed to apply patch".red().bold()].into());
|
||||
|
||||
if !stderr.trim().is_empty() {
|
||||
let output = output_lines(
|
||||
@@ -2185,6 +2184,12 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
|
||||
PlainHistoryCell { lines }
|
||||
}
|
||||
|
||||
pub(crate) fn new_patch_apply_declined() -> PlainHistoryCell {
|
||||
PlainHistoryCell {
|
||||
lines: vec![vec!["• ".dim(), "Accepting revision".bold()].into()],
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn new_view_image_tool_call(path: PathBuf, cwd: &Path) -> PlainHistoryCell {
|
||||
let display_path = display_path_for(&path, cwd);
|
||||
|
||||
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
---
|
||||
source: tui/src/app.rs
|
||||
assertion_line: 3932
|
||||
expression: "render_transcript_cells_for_snapshot(&app, 80)"
|
||||
---
|
||||
|
||||
› i want the content to say bye instead.
|
||||
Reference in New Issue
Block a user