Add Exploring support for read-only git commands

This commit is contained in:
James Peter
2025-09-17 09:32:44 +10:00
parent fd8f72589f
commit b6b9fc4186
4 changed files with 260 additions and 43 deletions
+95 -3
View File
@@ -21,6 +21,9 @@ pub enum ParsedCommand {
query: Option<String>,
path: Option<String>,
},
ReadCommand {
cmd: String,
},
Unknown {
cmd: String,
},
@@ -35,6 +38,7 @@ impl From<ParsedCommand> for codex_protocol::parse_command::ParsedCommand {
ParsedCommand::Read { cmd, name } => P::Read { cmd, name },
ParsedCommand::ListFiles { cmd, path } => P::ListFiles { cmd, path },
ParsedCommand::Search { cmd, query, path } => P::Search { cmd, query, path },
ParsedCommand::ReadCommand { cmd } => P::ReadCommand { cmd },
ParsedCommand::Unknown { cmd } => P::Unknown { cmd },
}
}
@@ -100,10 +104,10 @@ mod tests {
}
#[test]
fn git_status_is_unknown() {
fn git_status_is_read_command() {
assert_parsed(
&vec_str(&["git", "status"]),
vec![ParsedCommand::Unknown {
vec![ParsedCommand::ReadCommand {
cmd: "git status".to_string(),
}],
);
@@ -114,12 +118,32 @@ mod tests {
let inner = "git status | wc -l";
assert_parsed(
&vec_str(&["bash", "-lc", inner]),
vec![ParsedCommand::Unknown {
vec![ParsedCommand::ReadCommand {
cmd: "git status".to_string(),
}],
);
}
#[test]
fn git_diff_staged_is_read_command() {
assert_parsed(
&vec_str(&["git", "--no-pager", "diff", "--staged", "--stat"]),
vec![ParsedCommand::ReadCommand {
cmd: "git --no-pager diff --staged --stat".to_string(),
}],
);
}
#[test]
fn git_add_remains_unknown() {
assert_parsed(
&vec_str(&["git", "add", "."]),
vec![ParsedCommand::Unknown {
cmd: "git add .".to_string(),
}],
);
}
#[test]
fn bash_lc_redirect_not_quoted() {
let inner = "echo foo > bar";
@@ -1407,6 +1431,59 @@ fn drop_small_formatting_commands(mut commands: Vec<Vec<String>>) -> Vec<Vec<Str
commands
}
fn git_flag_consumes_value(flag: &str) -> bool {
matches!(
flag,
"-C" | "-c"
| "--git-dir"
| "--work-tree"
| "--namespace"
| "--super-prefix"
| "--config"
| "--config-env"
)
}
fn git_subcommand_index(args: &[String]) -> Option<usize> {
let mut i = 0;
while i < args.len() {
let token = args[i].as_str();
if token == "--" {
return if i + 1 < args.len() {
Some(i + 1)
} else {
None
};
}
if token.starts_with('-') {
if git_flag_consumes_value(token) && i + 1 < args.len() {
i += 1;
}
i += 1;
continue;
}
return Some(i);
}
None
}
fn git_command_is_read_only(subcmd: &str, args: &[String]) -> bool {
match subcmd {
"status" => true,
"diff" => args
.iter()
.all(|arg| !(arg == "-o" || arg == "--output" || arg.starts_with("--output="))),
"log" => true,
"show" => true,
"rev-parse" => true,
"rev-list" => true,
"describe" => true,
"ls-files" => true,
"ls-tree" => true,
_ => false,
}
}
fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
match main_cmd.split_first() {
Some((head, tail)) if head == "ls" => {
@@ -1432,6 +1509,21 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
path,
}
}
Some((head, tail)) if head == "git" => {
let cmd = shlex_join(main_cmd);
let is_read = git_subcommand_index(tail)
.map(|idx| {
let sub = tail[idx].as_str();
let rest = &tail[idx + 1..];
git_command_is_read_only(sub, rest)
})
.unwrap_or(false);
if is_read {
ParsedCommand::ReadCommand { cmd }
} else {
ParsedCommand::Unknown { cmd }
}
}
Some((head, tail)) if head == "rg" => {
let args_no_connector = trim_at_connector(tail);
let has_files_flag = args_no_connector.iter().any(|a| a == "--files");
+3
View File
@@ -18,6 +18,9 @@ pub enum ParsedCommand {
query: Option<String>,
path: Option<String>,
},
ReadCommand {
cmd: String,
},
Unknown {
cmd: String,
},
+132 -39
View File
@@ -3,8 +3,9 @@
use super::ChatWidget;
use crate::app_event::AppEvent;
use crate::height_manager::HeightEvent;
use crate::history_cell::{self, HistoryCell};
use crate::history_cell::CommandOutput;
use crate::history_cell::{self, HistoryCell};
use codex_core::parse_command::ParsedCommand;
use codex_core::protocol::{ExecCommandBeginEvent, ExecCommandEndEvent, OrderMeta};
fn find_trailing_explore_agg(chat: &ChatWidget<'_>) -> Option<usize> {
@@ -41,7 +42,9 @@ pub(super) fn finalize_exec_cell_at(
stdout: String,
stderr: String,
) {
if idx >= chat.history_cells.len() { return; }
if idx >= chat.history_cells.len() {
return;
}
if let Some(exec) = chat.history_cells[idx]
.as_any()
.downcast_ref::<history_cell::ExecCell>()
@@ -50,7 +53,11 @@ pub(super) fn finalize_exec_cell_at(
let completed = history_cell::new_completed_exec_command(
exec.command.clone(),
exec.parsed.clone(),
CommandOutput { exit_code, stdout, stderr },
CommandOutput {
exit_code,
stdout,
stderr,
},
);
chat.history_replace_at(idx, Box::new(completed));
}
@@ -68,7 +75,13 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
.collect();
for (call_id, maybe_idx, explore_entry) in &running {
if let Some(idx) = maybe_idx {
finalize_exec_cell_at(chat, *idx, 130, stdout_empty.clone(), interrupted_msg.clone());
finalize_exec_cell_at(
chat,
*idx,
130,
stdout_empty.clone(),
interrupted_msg.clone(),
);
}
if let Some((agg_idx, entry_idx)) = explore_entry {
if *agg_idx < chat.history_cells.len() {
@@ -76,7 +89,10 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
.as_any_mut()
.downcast_mut::<history_cell::ExploreAggregationCell>()
{
agg.update_status(*entry_idx, history_cell::ExploreEntryStatus::Error { exit_code: None });
agg.update_status(
*entry_idx,
history_cell::ExploreEntryStatus::Error { exit_code: None },
);
}
}
}
@@ -128,7 +144,9 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
.as_any()
.downcast_ref::<history_cell::RunningToolCallCell>()
.is_some_and(|rt| rt.has_title("Web Search..."));
if is_ws { target_idx = Some(idx); }
if is_ws {
target_idx = Some(idx);
}
}
if target_idx.is_none() {
for i in (0..chat.history_cells.len()).rev() {
@@ -136,7 +154,10 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
.as_any()
.downcast_ref::<history_cell::RunningToolCallCell>()
{
if rt.has_title("Web Search...") { target_idx = Some(i); break; }
if rt.has_title("Web Search...") {
target_idx = Some(i);
break;
}
}
}
}
@@ -231,7 +252,9 @@ pub(super) fn finalize_all_running_due_to_answer(chat: &mut ChatWidget<'_>) {
.as_any()
.downcast_ref::<history_cell::RunningToolCallCell>()
.is_some_and(|rt| rt.has_title("Web Search..."));
if is_ws { target_idx = Some(idx); }
if is_ws {
target_idx = Some(idx);
}
}
if target_idx.is_none() {
for i in (0..chat.history_cells.len()).rev() {
@@ -239,7 +262,10 @@ pub(super) fn finalize_all_running_due_to_answer(chat: &mut ChatWidget<'_>) {
.as_any()
.downcast_ref::<history_cell::RunningToolCallCell>()
{
if rt.has_title("Web Search...") { target_idx = Some(i); break; }
if rt.has_title("Web Search...") {
target_idx = Some(i);
break;
}
}
}
}
@@ -278,7 +304,9 @@ pub(super) fn try_merge_completed_exec_at(chat: &mut ChatWidget<'_>, idx: usize)
_ => return,
};
let new_kind = to_kind(new_exec);
if matches!(new_kind, history_cell::ExecKind::Run) { return; }
if matches!(new_kind, history_cell::ExecKind::Run) {
return;
}
if let Some(prev_exec) = chat.history_cells[idx - 1]
.as_any()
@@ -313,9 +341,8 @@ pub(super) fn try_merge_completed_exec_at(chat: &mut ChatWidget<'_>, idx: usize)
.downcast_mut::<history_cell::MergedExecCell>()
{
if prev_merged.exec_kind() == new_kind {
if let Some(current_exec) = right[0]
.as_any()
.downcast_ref::<history_cell::ExecCell>()
if let Some(current_exec) =
right[0].as_any().downcast_ref::<history_cell::ExecCell>()
{
prev_merged.push_exec(current_exec);
did_merge_into_prev = true;
@@ -333,14 +360,37 @@ pub(super) fn try_merge_completed_exec_at(chat: &mut ChatWidget<'_>, idx: usize)
}
}
pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBeginEvent, order: &OrderMeta) {
if chat.ended_call_ids.contains(&super::ExecCallId(ev.call_id.clone())) { return; }
for cell in &chat.history_cells { cell.trigger_fade(); }
pub(super) fn handle_exec_begin_now(
chat: &mut ChatWidget<'_>,
ev: ExecCommandBeginEvent,
order: &OrderMeta,
) {
if chat
.ended_call_ids
.contains(&super::ExecCallId(ev.call_id.clone()))
{
return;
}
for cell in &chat.history_cells {
cell.trigger_fade();
}
let parsed_command = ev.parsed_cmd.clone();
let action = history_cell::action_enum_from_parsed(&parsed_command);
chat.height_manager.borrow_mut().record_event(HeightEvent::RunBegin);
chat.height_manager
.borrow_mut()
.record_event(HeightEvent::RunBegin);
if matches!(action, history_cell::ExecAction::Read | history_cell::ExecAction::Search | history_cell::ExecAction::List) {
let has_read_command = parsed_command
.iter()
.any(|p| matches!(p, ParsedCommand::ReadCommand { .. }));
if matches!(
action,
history_cell::ExecAction::Read
| history_cell::ExecAction::Search
| history_cell::ExecAction::List
) || has_read_command
{
let mut created_new = false;
let mut agg_idx = chat.exec.running_explore_agg_index.and_then(|idx| {
if idx < chat.history_cells.len()
@@ -361,7 +411,10 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
if agg_idx.is_none() {
let key = ChatWidget::order_key_from_order_meta(order);
let idx = chat.history_insert_with_key_global(Box::new(history_cell::ExploreAggregationCell::new()), key);
let idx = chat.history_insert_with_key_global(
Box::new(history_cell::ExploreAggregationCell::new()),
key,
);
created_new = true;
agg_idx = Some(idx);
}
@@ -370,12 +423,14 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
let entry_idx = chat.history_cells[idx]
.as_any_mut()
.downcast_mut::<history_cell::ExploreAggregationCell>()
.and_then(|agg| agg.push_from_parsed(
&parsed_command,
history_cell::ExploreEntryStatus::Running,
&ev.cwd,
&chat.config.cwd,
));
.and_then(|agg| {
agg.push_from_parsed(
&parsed_command,
history_cell::ExploreEntryStatus::Running,
&ev.cwd,
&chat.config.cwd,
)
});
if let Some(entry_idx) = entry_idx {
chat.exec.running_explore_agg_index = Some(idx);
chat.exec.running_commands.insert(
@@ -441,8 +496,13 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
}
}
pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndEvent, order: &OrderMeta) {
chat.ended_call_ids.insert(super::ExecCallId(ev.call_id.clone()));
pub(super) fn handle_exec_end_now(
chat: &mut ChatWidget<'_>,
ev: ExecCommandEndEvent,
order: &OrderMeta,
) {
chat.ended_call_ids
.insert(super::ExecCallId(ev.call_id.clone()));
// If this call was already marked as cancelled, drop the End to avoid
// inserting a duplicate completed cell after the user interrupt.
if chat
@@ -452,9 +512,20 @@ pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndE
chat.maybe_hide_spinner();
return;
}
let ExecCommandEndEvent { call_id, exit_code, duration: _, stdout, stderr } = ev;
let cmd = chat.exec.running_commands.remove(&super::ExecCallId(call_id.clone()));
chat.height_manager.borrow_mut().record_event(HeightEvent::RunEnd);
let ExecCommandEndEvent {
call_id,
exit_code,
duration: _,
stdout,
stderr,
} = ev;
let cmd = chat
.exec
.running_commands
.remove(&super::ExecCallId(call_id.clone()));
chat.height_manager
.borrow_mut()
.record_event(HeightEvent::RunEnd);
let (command, parsed, history_index, explore_entry) = match cmd {
Some(super::RunningCommand {
command,
@@ -514,7 +585,11 @@ pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndE
let mut completed_opt = Some(history_cell::new_completed_exec_command(
command,
parsed,
CommandOutput { exit_code, stdout, stderr },
CommandOutput {
exit_code,
stdout,
stderr,
},
));
let mut replaced = false;
@@ -526,24 +601,40 @@ pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndE
.map(|e| {
if let Some(ref c) = completed_opt {
e.output.is_none() && e.command == c.command
} else { false }
} else {
false
}
})
.unwrap_or(false);
if is_match {
if let Some(c) = completed_opt.take() { chat.history_replace_and_maybe_merge(idx, Box::new(c)); }
if let Some(c) = completed_opt.take() {
chat.history_replace_and_maybe_merge(idx, Box::new(c));
}
replaced = true;
}
}
if !replaced {
let mut found: Option<usize> = None;
for i in (0..chat.history_cells.len()).rev() {
if let Some(exec) = chat.history_cells[i].as_any().downcast_ref::<history_cell::ExecCell>() {
let is_same = if let Some(ref c) = completed_opt { exec.command == c.command } else { false };
if exec.output.is_none() && is_same { found = Some(i); break; }
if let Some(exec) = chat.history_cells[i]
.as_any()
.downcast_ref::<history_cell::ExecCell>()
{
let is_same = if let Some(ref c) = completed_opt {
exec.command == c.command
} else {
false
};
if exec.output.is_none() && is_same {
found = Some(i);
break;
}
}
}
if let Some(i) = found {
if let Some(c) = completed_opt.take() { chat.history_replace_and_maybe_merge(i, Box::new(c)); }
if let Some(c) = completed_opt.take() {
chat.history_replace_and_maybe_merge(i, Box::new(c));
}
replaced = true;
}
}
@@ -559,7 +650,8 @@ pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndE
}
if exit_code == 0 {
chat.bottom_pane.update_status_text("command completed".to_string());
chat.bottom_pane
.update_status_text("command completed".to_string());
// If this was a successful `git push`, start background GH Actions watch if enabled.
crate::chatwidget::gh_actions::maybe_watch_after_push(
chat.app_event_tx.clone(),
@@ -567,7 +659,8 @@ pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndE
&command_for_watch,
);
} else {
chat.bottom_pane.update_status_text(format!("command failed (exit {})", exit_code));
chat.bottom_pane
.update_status_text(format!("command failed (exit {})", exit_code));
}
chat.maybe_hide_spinner();
}
+30 -1
View File
@@ -363,6 +363,9 @@ enum ExploreSummary {
annotation: Option<String>,
range: Option<(u32, u32)>,
},
Command {
command: String,
},
Fallback {
text: String,
},
@@ -377,6 +380,9 @@ struct ExploreEntry {
impl ExploreEntry {
fn label(&self) -> &'static str {
if matches!(self.summary, ExploreSummary::Command { .. }) {
return "Ran";
}
match self.action {
ExecAction::Read => "Read",
ExecAction::Search => "Searched",
@@ -435,6 +441,7 @@ impl ExploreEntry {
}
spans
}
ExploreSummary::Command { command } => highlight_command_summary(command),
ExploreSummary::Fallback { text } => vec![Span::styled(
text.clone(),
Style::default().fg(crate::colors::text()),
@@ -451,6 +458,21 @@ impl ExploreEntry {
}
}
fn highlight_command_summary(command: &str) -> Vec<Span<'static>> {
let normalized = normalize_shell_command_display(command);
let display_line = insert_line_breaks_after_double_ampersand(&normalized);
let highlighted = crate::syntax_highlight::highlight_code_block(&display_line, Some("bash"));
if let Some(mut first) = highlighted.into_iter().next() {
emphasize_shell_command_name(&mut first);
first.spans
} else {
vec![Span::styled(
display_line,
Style::default().fg(crate::colors::text()),
)]
}
}
fn normalize_separators(mut value: String) -> String {
if value.is_empty() {
return value;
@@ -615,7 +637,12 @@ impl ExploreAggregationCell {
}
_ => None,
}),
ExecAction::Run => None,
ExecAction::Run => parsed.iter().find_map(|p| match p {
ParsedCommand::ReadCommand { cmd } => Some(ExploreSummary::Command {
command: cmd.clone(),
}),
_ => None,
}),
};
let summary = summary.or_else(|| {
@@ -2880,6 +2907,7 @@ fn exec_render_parts_parsed(
(None, None) => ("Search".to_string(), cmd.clone()),
}
}
ParsedCommand::ReadCommand { cmd } => ("Run".to_string(), cmd.clone()),
// Upstream variants not present in our core parser are ignored or treated as generic runs
ParsedCommand::Unknown { cmd } => {
// Suppress separator helpers like `echo ---` which are used
@@ -5984,6 +6012,7 @@ fn new_parsed_command(
(None, None) => ("Search".to_string(), cmd.clone()),
}
}
ParsedCommand::ReadCommand { cmd } => ("Run".to_string(), cmd.clone()),
// Upstream-only variants handled as generic runs in this fork
ParsedCommand::Unknown { cmd } => {
let t = cmd.trim();