From 98971bfc9040414d6e4fcc894c0186cf6e4707b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 5 Feb 2026 22:43:49 +0800 Subject: [PATCH 01/20] feat: add E2E testing infrastructure for TUI --- .config/nextest.toml | 5 +- Cargo.lock | 145 +++++++++- crates/coco-tui/Cargo.toml | 3 + .../coco-tui/src/components/messages/combo.rs | 8 +- crates/coco-tui/src/e2e_tests.rs | 2 + crates/coco-tui/src/e2e_tests/startup.rs | 24 ++ crates/coco-tui/src/e2e_tests/support.rs | 248 ++++++++++++++++++ crates/coco-tui/src/lib.rs | 2 + 8 files changed, 424 insertions(+), 13 deletions(-) create mode 100644 crates/coco-tui/src/e2e_tests.rs create mode 100644 crates/coco-tui/src/e2e_tests/startup.rs create mode 100644 crates/coco-tui/src/e2e_tests/support.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index e14b1f4..48da9bd 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -1,5 +1,8 @@ [profile.default] -default-filter = 'not test(tests::networking::)' +default-filter = 'not test(tests::networking::) and not test(e2e_tests::)' [profile.networking] default-filter = 'test(tests::networking::)' + +[profile.e2e] +default-filter = 'test(e2e_tests::)' diff --git a/Cargo.lock b/Cargo.lock index b1044cd..64d5cca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -127,6 +127,12 @@ version = "1.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" +[[package]] +name = "arrayvec" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" + [[package]] name = "async-trait" version = "0.1.89" @@ -298,6 +304,12 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fd1289c04a9ea8cb22300a459a72a385d7c73d3259e2ed7dcb2af674838cfa9" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + [[package]] name = "cfg_aliases" version = "0.2.1" @@ -402,10 +414,12 @@ dependencies = [ "futures", "indoc", "lazy_static", + "portable-pty", "ratatui", "serde", "serde_json", "snafu", + "tempfile", "time", "tokio", "tokio-util", @@ -414,6 +428,7 @@ dependencies = [ "tracing-subscriber", "tui-textarea", "uuid", + "vt100", ] [[package]] @@ -751,6 +766,12 @@ dependencies = [ "litrs", ] +[[package]] +name = "downcast-rs" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" + [[package]] name = "dtor" version = "0.1.0" @@ -1541,6 +1562,18 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "nix" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" +dependencies = [ + "bitflags 2.10.0", + "cfg-if", + "cfg_aliases 0.1.1", + "libc", +] + [[package]] name = "nix" version = "0.29.0" @@ -1549,7 +1582,7 @@ checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" dependencies = [ "bitflags 2.10.0", "cfg-if", - "cfg_aliases", + "cfg_aliases 0.2.1", "libc", "memoffset", ] @@ -1562,7 +1595,7 @@ checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ "bitflags 2.10.0", "cfg-if", - "cfg_aliases", + "cfg_aliases 0.2.1", "libc", ] @@ -1847,6 +1880,27 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f89776e4d69bb58bc6993e99ffa1d11f228b839984854c7daeb5d37f87cbe950" +[[package]] +name = "portable-pty" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4a596a2b3d2752d94f51fac2d4a96737b8705dddd311a32b9af47211f08671e" +dependencies = [ + "anyhow", + "bitflags 1.3.2", + "downcast-rs", + "filedescriptor", + "lazy_static", + "libc", + "log", + "nix 0.28.0", + "serial2", + "shared_library", + "shell-words", + "winapi", + "winreg", +] + [[package]] name = "potential_utf" version = "0.1.3" @@ -1911,7 +1965,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e20a958963c291dc322d98411f541009df2ced7b5a4f2bd52337638cfccf20" dependencies = [ "bytes", - "cfg_aliases", + "cfg_aliases 0.2.1", "pin-project-lite", "quinn-proto", "quinn-udp", @@ -1951,7 +2005,7 @@ version = "0.5.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "addec6a0dcad8a8d96a771f815f0eaf55f9d1805756410b39f5fa81332574cbd" dependencies = [ - "cfg_aliases", + "cfg_aliases 0.2.1", "libc", "once_cell", "socket2", @@ -2067,7 +2121,7 @@ dependencies = [ "thiserror 2.0.17", "unicode-segmentation", "unicode-truncate", - "unicode-width", + "unicode-width 0.2.0", ] [[package]] @@ -2131,7 +2185,7 @@ dependencies = [ "strum", "time", "unicode-segmentation", - "unicode-width", + "unicode-width 0.2.0", ] [[package]] @@ -2526,6 +2580,17 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serial2" +version = "0.2.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cc76fa68e25e771492ca1e3c53d447ef0be3093e05cd3b47f4b712ba10c6f3c" +dependencies = [ + "cfg-if", + "libc", + "winapi", +] + [[package]] name = "sha2" version = "0.10.9" @@ -2546,6 +2611,22 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shared_library" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a9e7e0f2bfae24d8a5b5a66c5b257a83c7412304311512a0c054cd5e619da11" +dependencies = [ + "lazy_static", + "libc", +] + +[[package]] +name = "shell-words" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc6fe69c597f9c37bfeeeeeb33da3530379845f10be461a66d16d03eca2ded77" + [[package]] name = "shlex" version = "1.3.0" @@ -3263,7 +3344,7 @@ dependencies = [ "crossterm 0.29.0", "ratatui-core", "ratatui-widgets", - "unicode-width", + "unicode-width 0.2.0", ] [[package]] @@ -3298,9 +3379,15 @@ checksum = "16b380a1238663e5f8a691f9039c73e1cdae598a30e9855f541d29b08b53e9a5" dependencies = [ "itertools", "unicode-segmentation", - "unicode-width", + "unicode-width 0.2.0", ] +[[package]] +name = "unicode-width" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" + [[package]] name = "unicode-width" version = "0.2.0" @@ -3368,6 +3455,39 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "vt100" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84cd863bf0db7e392ba3bd04994be3473491b31e66340672af5d11943c6274de" +dependencies = [ + "itoa", + "log", + "unicode-width 0.1.14", + "vte", +] + +[[package]] +name = "vte" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5022b5fbf9407086c180e9557be968742d839e68346af7792b8592489732197" +dependencies = [ + "arrayvec", + "utf8parse", + "vte_generate_state_changes", +] + +[[package]] +name = "vte_generate_state_changes" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e369bee1b05d510a7b4ed645f5faa90619e05437111783ea5848f28d97d3c2e" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "vtparse" version = "0.6.2" @@ -3950,6 +4070,15 @@ version = "0.7.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "21a0236b59786fed61e2a80582dd500fe61f18b5dca67a4a067d0bc9039339cf" +[[package]] +name = "winreg" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "80d0f4e272c85def139476380b12f9ac60926689dd2e01d4923222f40580869d" +dependencies = [ + "winapi", +] + [[package]] name = "winsafe" version = "0.0.19" diff --git a/crates/coco-tui/Cargo.toml b/crates/coco-tui/Cargo.toml index e53ccfc..3d280f4 100644 --- a/crates/coco-tui/Cargo.toml +++ b/crates/coco-tui/Cargo.toml @@ -65,6 +65,9 @@ time = { workspace = true, features = [ ] } [dev-dependencies] +portable-pty = "0.9.0" +tempfile = "3.10.1" +vt100 = "0.15.2" [build-dependencies] time = { workspace = true, features = ["formatting"] } diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index 90b3cd4..2109f4d 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -1351,8 +1351,8 @@ mod tests { KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE) } - fn test_key_esc() -> KeyEvent { - KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE) + fn test_key_backspace() -> KeyEvent { + KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE) } fn make_starter(name: &str) -> code_combo::Starter { @@ -1462,7 +1462,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn combo_enters_and_exits_actionable_messages_with_enter_and_esc() { + async fn combo_enters_and_exits_actionable_messages_with_enter_and_backspace() { let mut combo = Combo::new(TEST_ID, TEST_NAME); combo.handle_event(&Event::Combo(ComboEvent::Executing { id: TEST_ID.to_string(), @@ -1507,7 +1507,7 @@ mod tests { assert!(combo.is_child_focused); assert_eq!(combo.messages.selected_idx(), Some(0)); - combo.handle_key_event(&test_key_esc()); + combo.handle_key_event(&test_key_backspace()); assert!(!combo.is_child_focused); assert_eq!(combo.messages.selected_idx(), None); } diff --git a/crates/coco-tui/src/e2e_tests.rs b/crates/coco-tui/src/e2e_tests.rs new file mode 100644 index 0000000..4d400e4 --- /dev/null +++ b/crates/coco-tui/src/e2e_tests.rs @@ -0,0 +1,2 @@ +mod startup; +mod support; diff --git a/crates/coco-tui/src/e2e_tests/startup.rs b/crates/coco-tui/src/e2e_tests/startup.rs new file mode 100644 index 0000000..9dbfb81 --- /dev/null +++ b/crates/coco-tui/src/e2e_tests/startup.rs @@ -0,0 +1,24 @@ +use std::time::Duration; + +use tempfile::TempDir; +use vt100::Parser; + +use super::support::{ + KillOnDrop, shutdown_tui, spawn_reader, spawn_tui, wait_for_screen_contains, write_config, +}; + +#[test] +fn tui_starts_and_exits_on_ctrl_c() { + let temp = TempDir::new().expect("tempdir"); + let config_dir = write_config(&temp); + let (mut child, reader, mut writer) = spawn_tui(Some(&config_dir), &[]); + let mut guard = KillOnDrop::new(child.clone_killer()); + let rx = spawn_reader(reader); + let mut parser = Parser::new(24, 120, 0); + + wait_for_screen_contains(&mut parser, &rx, "Ready", Duration::from_secs(20)); + + let status = shutdown_tui(&mut *child, &mut writer, &parser); + guard.disarm(); + assert!(status.success(), "tui exit failed: {status:?}"); +} diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs new file mode 100644 index 0000000..2f88428 --- /dev/null +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -0,0 +1,248 @@ +use std::{ + io::{Read, Write}, + path::{Path, PathBuf}, + sync::mpsc::{Receiver, RecvTimeoutError}, + thread, + time::{Duration, Instant}, +}; + +use portable_pty::{ + Child, ChildKiller, CommandBuilder, ExitStatus, NativePtySystem, PtySize, PtySystem, +}; +use tempfile::TempDir; +use vt100::Parser; + +pub(crate) type PtyChild = dyn Child + Send; + +pub(crate) fn write_config(dir: &TempDir) -> PathBuf { + let config_dir = dir.path().join("coco"); + std::fs::create_dir_all(&config_dir).expect("create config dir"); + let config_path = config_dir.join("config.toml"); + std::fs::write(&config_path, "providers = []\n").expect("write config"); + config_dir +} + +#[allow(dead_code)] +pub(crate) fn require_e2e_config_dir() -> PathBuf { + let config_dir = + PathBuf::from(std::env::var("COCO_E2E_CONFIG_DIR").expect("set COCO_E2E_CONFIG_DIR")); + assert!( + config_dir.join("config.toml").exists(), + "missing config.toml under COCO_E2E_CONFIG_DIR" + ); + config_dir +} + +fn repo_root() -> PathBuf { + let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + manifest_dir + .parent() + .and_then(|path| path.parent()) + .map(PathBuf::from) + .expect("resolve repo root") +} + +pub(crate) fn spawn_tui( + config_dir: Option<&Path>, + args: &[&str], +) -> (Box, Box, Box) { + let pty_system = NativePtySystem::default(); + let pair = pty_system + .openpty(PtySize { + rows: 24, + cols: 120, + pixel_width: 0, + pixel_height: 0, + }) + .expect("open pty"); + + let mut cmd = CommandBuilder::new("cargo"); + cmd.args(["run", "-p", "coco-tui", "--bin", "coco", "--"]); + cmd.cwd(repo_root()); + + if let Some(dir) = config_dir { + cmd.args(["--config-dir", dir.to_string_lossy().as_ref()]); + } + + cmd.env("COCO_LOG", "trace"); + + if !args.is_empty() { + cmd.args(args); + } + + let child = pair.slave.spawn_command(cmd).expect("spawn coco"); + let reader = pair.master.try_clone_reader().expect("clone reader"); + let writer = pair.master.take_writer().expect("take writer"); + (child, reader, writer) +} + +pub(crate) fn wait_for_screen_contains( + parser: &mut Parser, + rx: &Receiver>, + needle: &str, + timeout: Duration, +) { + let start = Instant::now(); + loop { + let elapsed = start.elapsed(); + if elapsed > timeout { + let screen = parser.screen().contents(); + panic!("timeout waiting for screen to contain: {needle}\n--- screen ---\n{screen}"); + } + + let remaining = timeout.saturating_sub(elapsed); + match rx.recv_timeout(remaining.min(Duration::from_millis(200))) { + Ok(chunk) => { + if !chunk.is_empty() { + parser.process(&chunk); + } + } + Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => { + panic!("pty reader thread exited before finding: {needle}"); + } + } + + let screen = parser.screen().contents(); + if screen.contains(needle) { + return; + } + } +} + +#[allow(dead_code)] +pub(crate) fn send_enter(writer: &mut dyn Write) { + writer.write_all(&[0x0d]).expect("write enter"); + writer.flush().expect("flush"); +} + +#[allow(dead_code)] +pub(crate) fn send_alt_enter(writer: &mut dyn Write) { + writer.write_all(&[0x1b, 0x0d]).expect("write alt-enter"); + writer.flush().expect("flush"); +} + +#[allow(dead_code)] +pub(crate) fn send_text(writer: &mut dyn Write, text: &str) { + writer.write_all(text.as_bytes()).expect("write text"); + writer.flush().expect("flush"); +} + +fn send_ctrl_c(writer: &mut dyn Write) { + writer.write_all(&[0x03]).expect("write ctrl-c"); + writer.flush().expect("flush"); +} + +fn send_ctrl_q(writer: &mut dyn Write) { + writer.write_all(&[0x11]).expect("write ctrl-q"); + writer.flush().expect("flush"); +} + +pub(crate) fn spawn_reader(mut reader: Box) -> Receiver> { + let (tx, rx) = std::sync::mpsc::channel(); + thread::spawn(move || { + let mut buf = [0u8; 4096]; + loop { + match reader.read(&mut buf) { + Ok(0) => { + let _ = tx.send(Vec::new()); + break; + } + Ok(n) => { + if tx.send(buf[..n].to_vec()).is_err() { + break; + } + } + Err(err) if err.kind() == std::io::ErrorKind::Interrupted => { + continue; + } + Err(_) => { + let _ = tx.send(Vec::new()); + break; + } + } + } + }); + rx +} + +fn wait_for_exit( + child: &mut PtyChild, + parser: &Parser, + timeout: Duration, +) -> portable_pty::ExitStatus { + let start = Instant::now(); + loop { + match child.try_wait().expect("try_wait child") { + Some(status) => return status, + None => { + if start.elapsed() > timeout { + let screen = parser.screen().contents(); + panic!("timeout waiting for child to exit\n--- screen ---\n{screen}"); + } + thread::sleep(Duration::from_millis(50)); + } + } + } +} + +fn wait_for_exit_optional( + child: &mut PtyChild, + timeout: Duration, +) -> Option { + let start = Instant::now(); + loop { + match child.try_wait().expect("try_wait child") { + Some(status) => return Some(status), + None => { + if start.elapsed() > timeout { + return None; + } + thread::sleep(Duration::from_millis(50)); + } + } + } +} + +pub(crate) struct KillOnDrop { + killer: Option>, +} + +impl KillOnDrop { + pub(crate) fn new(killer: Box) -> Self { + Self { + killer: Some(killer), + } + } + + pub(crate) fn disarm(&mut self) { + self.killer = None; + } +} + +impl Drop for KillOnDrop { + fn drop(&mut self) { + if let Some(mut killer) = self.killer.take() { + let _ = killer.kill(); + } + } +} + +pub(crate) fn shutdown_tui( + child: &mut PtyChild, + writer: &mut dyn Write, + parser: &Parser, +) -> ExitStatus { + send_ctrl_c(writer); + thread::sleep(Duration::from_millis(200)); + send_ctrl_c(writer); + + if let Some(status) = wait_for_exit_optional(child, Duration::from_secs(2)) { + return status; + } + + send_ctrl_q(writer); + thread::sleep(Duration::from_millis(200)); + send_ctrl_q(writer); + wait_for_exit(child, parser, Duration::from_secs(5)) +} diff --git a/crates/coco-tui/src/lib.rs b/crates/coco-tui/src/lib.rs index 9ed7909..0b8bb9d 100644 --- a/crates/coco-tui/src/lib.rs +++ b/crates/coco-tui/src/lib.rs @@ -8,6 +8,8 @@ pub mod session; pub mod widgets; #[macro_use] pub mod components; +#[cfg(test)] +mod e2e_tests; pub mod events; mod idle_tracker; pub mod logging; From 726c82475e8cb877bcbbac4176e71843050c9a93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 5 Feb 2026 23:48:31 +0800 Subject: [PATCH 02/20] feat: support interactive combo ask flow --- crates/coco-tui/src/main.rs | 13 +- src/cli.rs | 7 +- src/cmd/prompt.rs | 3 +- src/combo/runner.rs | 361 +++++++++++++++++++++++++++++++++++- src/combo/session.rs | 7 + src/combo/starter.rs | 8 + src/main.rs | 13 +- 7 files changed, 406 insertions(+), 6 deletions(-) diff --git a/crates/coco-tui/src/main.rs b/crates/coco-tui/src/main.rs index d3ef58e..2c20cd8 100644 --- a/crates/coco-tui/src/main.rs +++ b/crates/coco-tui/src/main.rs @@ -64,6 +64,9 @@ enum Commands { /// Response schemas in field:description format (repeatable) #[arg(long, value_name = "field:description")] schemas: Vec, + /// Enable interactive ask loop until `coco reply` is called + #[arg(short = 'i', long)] + interactive: bool, /// Prompt text to send via session socket (or read from stdin when omitted) #[arg(trailing_var_arg = true)] prompt: Vec, @@ -112,7 +115,15 @@ impl TryFrom for ClientCommand { fn try_from(value: Commands) -> std::result::Result { let command = match value { Commands::Metadata { fields } => ClientCommand::Metadata { fields }, - Commands::Ask { prompt, schemas } => ClientCommand::Ask { prompt, schemas }, + Commands::Ask { + prompt, + schemas, + interactive, + } => ClientCommand::Ask { + prompt, + schemas, + interactive, + }, Commands::Tell { prompt } => ClientCommand::Tell { prompt }, Commands::Reply { fields } => ClientCommand::Reply { fields }, Commands::Record { diff --git a/src/cli.rs b/src/cli.rs index 9c02d53..a679761 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,6 +10,7 @@ pub enum ClientCommand { Ask { prompt: Vec, schemas: Vec, + interactive: bool, }, Tell { prompt: Vec, @@ -55,7 +56,11 @@ pub async fn handle_client_command( ClientCommand::Metadata { fields } => crate::cmd::handle_metadata(fields) .await .whatever_context("failed to handle metadata"), - ClientCommand::Ask { prompt, schemas } => crate::cmd::handle_ask(prompt.join(" "), schemas) + ClientCommand::Ask { + prompt, + schemas, + interactive, + } => crate::cmd::handle_ask(prompt.join(" "), schemas, interactive) .await .whatever_context("failed to handle ask"), ClientCommand::Tell { prompt } => crate::cmd::handle_tell(prompt.join(" ")) diff --git a/src/cmd/prompt.rs b/src/cmd/prompt.rs index 5310e6d..31faac2 100644 --- a/src/cmd/prompt.rs +++ b/src/cmd/prompt.rs @@ -5,7 +5,7 @@ use tracing::info; use crate::{PromptPayload, PromptSchema, ReplyPayload, SessionSocketClient, error::Result}; -pub async fn handle_ask(prompt: String, schemas: Vec) -> Result<()> { +pub async fn handle_ask(prompt: String, schemas: Vec, interactive: bool) -> Result<()> { let prompt = resolve_prompt(prompt).await?; let explicit_schemas = !schemas.is_empty(); let mut schemas = parse_prompt_schemas(&schemas)?; @@ -19,6 +19,7 @@ pub async fn handle_ask(prompt: String, schemas: Vec) -> Result<()> { prompt, reply: true, schemas, + interactive, thinking: None, }; let response = client diff --git a/src/combo/runner.rs b/src/combo/runner.rs index b79779d..45221f2 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -350,6 +350,7 @@ async fn execute_combo( StarterEvent::PromptRequest { prompt, schemas, + interactive, responder, thinking, } => { @@ -363,7 +364,27 @@ async fn execute_combo( }); summary_parts.push(format!("[Prompt] {}", prompt)); - if reply_agent.offload_combo_reply() { + if interactive { + let result = handle_interactive_combo_reply( + &mut reply_agent, + &schemas, + &combo_name, + cancel_token.clone(), + session_socket_path.clone(), + &on_event_for_emit, + ) + .await; + if let Err(err) = result { + let message = err.to_string(); + emit_combo_event( + &on_event_for_emit, + ComboEvent::ReplyToolError { + message: message.clone(), + }, + ); + let _ = responder.send(Err(message)); + } + } else if reply_agent.offload_combo_reply() { let result = handle_offload_combo_reply_with_retry( &mut reply_agent, &schemas, @@ -374,12 +395,14 @@ async fn execute_combo( ) .await; if let Err(err) = result { + let message = err.to_string(); emit_combo_event( &on_event_for_emit, ComboEvent::ReplyToolError { - message: err.to_string(), + message: message.clone(), }, ); + let _ = responder.send(Err(message)); } } else { let disable_stream = @@ -963,6 +986,339 @@ fn extract_thinking_blocks(message: &Message) -> Vec { } } +fn extract_tool_uses(message: &Message) -> Vec { + match &message.content { + Content::Text(_) => Vec::new(), + Content::Multiple(blocks) => blocks + .iter() + .filter_map(|block| { + if let Block::ToolUse(tool_use) = block { + Some(tool_use.clone()) + } else { + None + } + }) + .collect(), + } +} + +fn build_interactive_offload_reply_directive(schemas: &[PromptSchema]) -> String { + let field_args: Vec = schemas + .iter() + .map(|schema| format!("--{}=", schema.name)) + .collect(); + let field_descriptions: Vec = schemas + .iter() + .map(|schema| format!("- --{}=: {}", schema.name, schema.description)) + .collect(); + format!( + r#"You can interact in multiple rounds and call tools as needed. +When you have enough information, finish by calling the bash tool with: +``` +coco reply {field_args} +``` + +Required fields: +{field_list} + +Until then, keep working with normal assistant behavior and tool usage."#, + field_args = field_args.join(" "), + field_list = field_descriptions.join("\n"), + ) +} + +fn build_interactive_offload_reply_reminder(schemas: &[PromptSchema]) -> String { + let field_args: Vec = schemas + .iter() + .map(|schema| format!("--{}=...", schema.name)) + .collect(); + format!( + "Continue if needed. When ready, finish with: coco reply {}", + field_args.join(" ") + ) +} + +async fn execute_interactive_tool_use( + agent: &mut Agent, + tool_use: &ToolUse, + cancel_token: CancellationToken, +) -> (Final, bool) { + let final_output: Arc>> = + Arc::new(std::sync::Mutex::new(None)); + let final_output_for_cb = final_output.clone(); + let permission_requested = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let permission_requested_for_cb = permission_requested.clone(); + let exec_status = agent + .execute_with_output( + &tool_use.id, + &tool_use.name, + Input::Starter(tool_use.input.clone()), + cancel_token.clone(), + move |output| match output { + crate::Output::AskPermission => { + permission_requested_for_cb.store(true, std::sync::atomic::Ordering::SeqCst); + } + crate::Output::Success(final_output_value) => { + if let Ok(mut guard) = final_output_for_cb.lock() { + *guard = Some((final_output_value, false)); + } + } + crate::Output::Failure(final_output_value) => { + if let Ok(mut guard) = final_output_for_cb.lock() { + *guard = Some((final_output_value, true)); + } + } + crate::Output::ToolOutput(_) + | crate::Output::TextEdit(_) + | crate::Output::SubagentOutput(_) + | crate::Output::ComboOutput(_) + | crate::Output::Denied => {} + }, + ) + .await; + if cancel_token.is_cancelled() { + return (Final::Message("tool execution cancelled".to_string()), true); + } + if let Some((output, is_error)) = final_output.lock().ok().and_then(|guard| guard.clone()) { + return (output, is_error); + } + if permission_requested.load(std::sync::atomic::Ordering::SeqCst) { + return ( + Final::Message("permission required for tool execution".to_string()), + true, + ); + } + match exec_status { + Ok(crate::ExecuteStatus::Completed) => (Final::Message("tool executed".to_string()), false), + Ok(crate::ExecuteStatus::Cancelled) => { + (Final::Message("tool execution cancelled".to_string()), true) + } + Err(err) => ( + Final::Message(format!("tool execution failed: {err}")), + true, + ), + } +} + +async fn handle_interactive_combo_reply( + agent: &mut Agent, + schemas: &[PromptSchema], + combo_name: &str, + cancel_token: CancellationToken, + session_socket_path: PathBuf, + on_event: &ComboEventCallback, +) -> Result<(), ComboReplyError> { + if cancel_token.is_cancelled() { + return Err(ComboReplyError::Cancelled); + } + agent + .append_message(Message::user(Content::Text( + build_interactive_offload_reply_directive(schemas), + ))) + .await; + + let max_turns = 50usize; + for _ in 0..max_turns { + if cancel_token.is_cancelled() { + return Err(ComboReplyError::Cancelled); + } + let disable_stream = agent.disable_stream_for_current_model(); + let response = if disable_stream { + agent + .chat_with_history() + .await + .map_err(|e| ComboReplyError::ChatFailed { + message: e.to_string(), + })? + } else { + let on_event_stream = on_event.clone(); + let stream_name = combo_name.to_string(); + agent + .chat_stream_with_history(cancel_token.clone(), move |update| match update { + ChatStreamUpdate::Reset => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStreamReset { + name: stream_name.clone(), + }, + ); + } + ChatStreamUpdate::Plain { index, text } => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStream { + name: stream_name.clone(), + index, + kind: ComboEventStreamKind::Plain, + text, + }, + ); + } + ChatStreamUpdate::Thinking { index, text } => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStream { + name: stream_name.clone(), + index, + kind: ComboEventStreamKind::Thinking, + text, + }, + ); + } + }) + .await + .map_err(|e| ComboReplyError::ChatFailed { + message: e.to_string(), + })? + }; + + let tool_uses = extract_tool_uses(&response.message); + if tool_uses.is_empty() { + agent + .append_message(Message::user(Content::Text( + build_interactive_offload_reply_reminder(schemas), + ))) + .await; + continue; + } + + let mut tool_results = Vec::new(); + let mut completed = false; + let mut guidance: Option = None; + + for tool_use in tool_uses { + emit_combo_event( + on_event, + ComboEvent::ReplyToolUse { + name: combo_name.to_string(), + tool_use: tool_use.clone(), + thinking: Vec::new(), + offload: true, + }, + ); + + if tool_use.name == BASH_TOOL_NAME { + let bash_input: BashInput = serde_json::from_value(tool_use.input.clone()) + .map_err(|err| ComboReplyError::InvalidBashInput { + message: err.to_string(), + })?; + let command = bash_input.command.clone(); + let command_kind = classify_offload_command(&command); + let (output, is_error) = match command_kind { + OffloadCommandKind::Unsafe => { + let reason = match bash_unsafe_reason(&command) { + Ok(_) => "command not allowlisted".to_string(), + Err(reason) => reason, + }; + ( + Final::Message(format!( + "command rejected: {reason}; expected coco reply" + )), + true, + ) + } + OffloadCommandKind::Safe | OffloadCommandKind::Coco => { + let bash_input_value = + serde_json::to_value(&bash_input).map_err(|err| { + ComboReplyError::InvalidBashInput { + message: err.to_string(), + } + })?; + let extra_envs = vec![( + "COCO_SESSION_SOCK".to_string(), + session_socket_path.to_string_lossy().to_string(), + )]; + let output = run_bash_chunked( + Input::Starter(bash_input_value), + &extra_envs, + cancel_token.clone(), + |_| {}, + ) + .await; + if cancel_token.is_cancelled() { + return Err(ComboReplyError::Cancelled); + } + match output { + Ok(Output::Final(output)) => (output, false), + Ok(Output::TextEdit(_)) => ( + Final::Message("unexpected tool output from bash".to_string()), + true, + ), + Err(output) => (output, true), + } + } + }; + + emit_combo_event( + on_event, + ComboEvent::ReplyToolResult { + name: combo_name.to_string(), + tool_use_id: tool_use.id.clone(), + is_error, + output: output.clone(), + }, + ); + tool_results.push(Block::tool_result( + &tool_use.id, + Some(is_error), + final_to_tool_content(&output), + )); + + match command_kind { + OffloadCommandKind::Coco if !is_error => { + completed = true; + break; + } + OffloadCommandKind::Coco => { + guidance = Some(build_offload_reply_guidance(schemas, &command, true)); + } + OffloadCommandKind::Unsafe => { + guidance = Some(build_offload_reply_guidance(schemas, &command, false)); + } + OffloadCommandKind::Safe => {} + } + continue; + } + + let (output, is_error) = + execute_interactive_tool_use(agent, &tool_use, cancel_token.clone()).await; + emit_combo_event( + on_event, + ComboEvent::ReplyToolResult { + name: combo_name.to_string(), + tool_use_id: tool_use.id.clone(), + is_error, + output: output.clone(), + }, + ); + tool_results.push(Block::tool_result( + &tool_use.id, + Some(is_error), + final_to_tool_content(&output), + )); + } + + if !tool_results.is_empty() { + agent + .append_message(Message::user(Content::Multiple(tool_results))) + .await; + } + + if completed { + return Ok(()); + } + + let prompt = guidance.unwrap_or_else(|| build_interactive_offload_reply_reminder(schemas)); + agent + .append_message(Message::user(Content::Text(prompt))) + .await; + } + + Err(ComboReplyError::ChatFailed { + message: "interactive ask exceeded max turns without coco reply".to_string(), + }) +} + fn build_offload_reply_directive(schemas: &[PromptSchema]) -> String { let field_args: Vec = schemas .iter() @@ -996,6 +1352,7 @@ fn build_offload_reply_retry_directive(schemas: &[PromptSchema]) -> String { format!("The previous response did not produce a valid coco reply. Retry.\n\n{directive}") } +#[derive(Clone, Copy)] enum OffloadCommandKind { Coco, Safe, diff --git a/src/combo/session.rs b/src/combo/session.rs index 7994289..a770f08 100644 --- a/src/combo/session.rs +++ b/src/combo/session.rs @@ -96,6 +96,8 @@ pub struct PromptPayload { pub prompt: String, #[serde(default)] pub reply: bool, + #[serde(default, skip_serializing_if = "is_false")] + pub interactive: bool, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub schemas: Vec, #[serde(skip_serializing_if = "Option::is_none")] @@ -108,6 +110,10 @@ pub struct PromptSchema { pub description: String, } +fn is_false(value: &bool) -> bool { + !*value +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct ThinkingConfig { pub enabled: bool, @@ -816,6 +822,7 @@ mod tests { let payload = PromptPayload { prompt: "Hello".to_string(), reply: true, + interactive: false, schemas: vec![PromptSchema { name: "message".to_string(), description: "reply message".to_string(), diff --git a/src/combo/starter.rs b/src/combo/starter.rs index bb94f78..f216a9d 100644 --- a/src/combo/starter.rs +++ b/src/combo/starter.rs @@ -73,6 +73,7 @@ pub enum StarterEvent { PromptRequest { prompt: String, schemas: Vec, + interactive: bool, thinking: Option, responder: PromptResponseSender, }, @@ -866,6 +867,12 @@ async fn handle_session_connection( } .build()); } + if payload.interactive && !payload.reply { + return Err(InvalidSnafu { + reason: "interactive prompt requires reply mode".to_string(), + } + .build()); + } if payload.reply { if discovery { return Err(InvalidSnafu { @@ -897,6 +904,7 @@ async fn handle_session_connection( .send(StarterEvent::PromptRequest { prompt: payload.prompt, schemas: payload.schemas, + interactive: payload.interactive, thinking, responder, }) diff --git a/src/main.rs b/src/main.rs index f493868..875588a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -30,6 +30,9 @@ enum Commands { /// defaults to a message field and prints the message string. #[arg(long, value_name = "field:description")] schemas: Vec, + /// Enable interactive ask loop until `coco reply` is called + #[arg(short = 'i', long)] + interactive: bool, /// Prompt text to send via session socket (or read from stdin when omitted) #[arg(trailing_var_arg = true)] prompt: Vec, @@ -77,7 +80,15 @@ async fn main() -> code_combo::Result<()> { let args = Args::from_arg_matches(&matches).unwrap_or_else(|err| err.exit()); let command = match args.command { Commands::Metadata { fields } => ClientCommand::Metadata { fields }, - Commands::Ask { prompt, schemas } => ClientCommand::Ask { prompt, schemas }, + Commands::Ask { + prompt, + schemas, + interactive, + } => ClientCommand::Ask { + prompt, + schemas, + interactive, + }, Commands::Tell { prompt } => ClientCommand::Tell { prompt }, Commands::Record { wrap_result, From 4d2991f514b16fe7e51814abcebfd4d2c0dceaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 5 Feb 2026 23:58:12 +0800 Subject: [PATCH 03/20] test: cover interactive ask session and starter flow --- src/combo/session.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++ src/combo/starter.rs | 45 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/combo/session.rs b/src/combo/session.rs index a770f08..8a92453 100644 --- a/src/combo/session.rs +++ b/src/combo/session.rs @@ -859,6 +859,54 @@ mod tests { Ok(()) } + #[tokio::test] + #[snafu::report] + async fn send_prompt_wait_response_over_socket_with_interactive() -> Result<()> { + let (_dir, socket_path) = unique_socket_path()?; + let server = SessionSocketServer::bind(&socket_path) + .await + .whatever_context("failed to bind socket")?; + + let payload = PromptPayload { + prompt: "Hello".to_string(), + reply: true, + interactive: true, + schemas: vec![PromptSchema { + name: "message".to_string(), + description: "reply message".to_string(), + }], + thinking: None, + }; + let client = SessionSocketClient::connect(&socket_path) + .await + .whatever_context("failed to connect socket path")?; + + let send_payload = payload.clone(); + let send_task = tokio::spawn(async move { + client + .send_prompt_wait_response(send_payload) + .await + .expect("send prompt with response") + }); + + let mut conn = server.accept().await.whatever_context("failed to accept")?; + let event = conn + .read_client_message() + .await + .whatever_context("failed to read client message")?; + assert_eq!(event, ClientMessage::Prompt(payload)); + + let response = r#"{"message":"ok"}"#.to_string(); + conn.send_server_message(&ServerMessage::PromptResponse(response.clone())) + .await + .whatever_context("failed to send prompt response")?; + + let received = send_task.await.whatever_context("failed to join")?; + assert_eq!(received, response); + + Ok(()) + } + #[tokio::test] #[snafu::report] async fn combo_run_stream_over_socket() -> Result<()> { diff --git a/src/combo/starter.rs b/src/combo/starter.rs index f216a9d..664270f 100644 --- a/src/combo/starter.rs +++ b/src/combo/starter.rs @@ -1487,6 +1487,51 @@ mod tests { Ok(()) } + #[tokio::test] + async fn execute_starter_records_coco_ask_interactive_prompt_with_schemas() + -> Result<(), Box> { + let bash = find_bash(); + let session_env = session_env_with_coco(); + let (_guard, file_path) = create_temp_combo( + "ask_reply_interactive.sh", + formatdoc! {r#" + #!{bash} + + coco metadata name=ask_reply_interactive || exit 0 + + coco ask -i --schemas response:message "Please do the thing" + "#} + .as_str(), + ) + .await?; + + let mut execution = StarterCommand::new(&file_path) + .session_env(session_env) + .execute(); + let mut prompt_text = None; + let mut interactive_flag = None; + while let Some(event) = execution.next().await { + if let StarterEvent::PromptRequest { + prompt, + interactive, + responder, + .. + } = event + { + prompt_text = Some(prompt.clone()); + interactive_flag = Some(interactive); + let _ = responder.send(Ok("ok".to_string())); + } + } + let Starter { combo, .. } = execution.wait().await?; + let combo = combo?; + assert_eq!(combo.metadata.name, "ask_reply_interactive"); + assert_eq!(prompt_text.as_deref(), Some("Please do the thing")); + assert_eq!(interactive_flag, Some(true)); + + Ok(()) + } + #[test] fn parse_reply_fields_accepts_required_fields() { let schemas = vec![ From e9d6721a36e811268933670b274ce7d75a36b141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 10:58:53 +0800 Subject: [PATCH 04/20] test: use real config and binaries in e2e --- crates/coco-tui/src/e2e_tests/startup.rs | 37 ++++++++++++-- crates/coco-tui/src/e2e_tests/support.rs | 64 +++++++++++++++++++----- 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/crates/coco-tui/src/e2e_tests/startup.rs b/crates/coco-tui/src/e2e_tests/startup.rs index 9dbfb81..2ff9068 100644 --- a/crates/coco-tui/src/e2e_tests/startup.rs +++ b/crates/coco-tui/src/e2e_tests/startup.rs @@ -4,13 +4,13 @@ use tempfile::TempDir; use vt100::Parser; use super::support::{ - KillOnDrop, shutdown_tui, spawn_reader, spawn_tui, wait_for_screen_contains, write_config, + KillOnDrop, coco_binary, require_e2e_config_dir, shutdown_tui, spawn_reader, spawn_tui, + spawn_tui_with_env, wait_for_screen_contains, }; #[test] fn tui_starts_and_exits_on_ctrl_c() { - let temp = TempDir::new().expect("tempdir"); - let config_dir = write_config(&temp); + let config_dir = require_e2e_config_dir(); let (mut child, reader, mut writer) = spawn_tui(Some(&config_dir), &[]); let mut guard = KillOnDrop::new(child.clone_killer()); let rx = spawn_reader(reader); @@ -22,3 +22,34 @@ fn tui_starts_and_exits_on_ctrl_c() { guard.disarm(); assert!(status.success(), "tui exit failed: {status:?}"); } + +#[test] +fn combo_run_not_found_is_visible_in_tui() { + let config_dir = require_e2e_config_dir(); + let temp = TempDir::new().expect("tempdir"); + let socket_path = temp.path().join("coco-e2e.sock"); + let socket = socket_path.to_string_lossy().to_string(); + let (mut child, reader, mut writer) = spawn_tui_with_env( + Some(&config_dir), + &[], + &[("COCO_SESSION_SOCK", socket.as_str())], + ); + let mut guard = KillOnDrop::new(child.clone_killer()); + let rx = spawn_reader(reader); + let mut parser = Parser::new(24, 120, 0); + + wait_for_screen_contains(&mut parser, &rx, "Ready", Duration::from_secs(20)); + + let status = std::process::Command::new(coco_binary()) + .args(["combo", "run", "__missing_combo__"]) + .env("COCO_SESSION_SOCK", &socket) + .status() + .expect("run combo client command"); + assert!(!status.success(), "missing combo should return non-zero"); + + wait_for_screen_contains(&mut parser, &rx, "Not found", Duration::from_secs(20)); + + let status = shutdown_tui(&mut *child, &mut writer, &parser); + guard.disarm(); + assert!(status.success(), "tui exit failed: {status:?}"); +} diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index 2f88428..bff744c 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -1,4 +1,5 @@ use std::{ + env, io::{Read, Write}, path::{Path, PathBuf}, sync::mpsc::{Receiver, RecvTimeoutError}, @@ -9,20 +10,10 @@ use std::{ use portable_pty::{ Child, ChildKiller, CommandBuilder, ExitStatus, NativePtySystem, PtySize, PtySystem, }; -use tempfile::TempDir; use vt100::Parser; pub(crate) type PtyChild = dyn Child + Send; -pub(crate) fn write_config(dir: &TempDir) -> PathBuf { - let config_dir = dir.path().join("coco"); - std::fs::create_dir_all(&config_dir).expect("create config dir"); - let config_path = config_dir.join("config.toml"); - std::fs::write(&config_path, "providers = []\n").expect("write config"); - config_dir -} - -#[allow(dead_code)] pub(crate) fn require_e2e_config_dir() -> PathBuf { let config_dir = PathBuf::from(std::env::var("COCO_E2E_CONFIG_DIR").expect("set COCO_E2E_CONFIG_DIR")); @@ -33,7 +24,7 @@ pub(crate) fn require_e2e_config_dir() -> PathBuf { config_dir } -fn repo_root() -> PathBuf { +pub(crate) fn repo_root() -> PathBuf { let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); manifest_dir .parent() @@ -42,9 +33,54 @@ fn repo_root() -> PathBuf { .expect("resolve repo root") } +fn resolve_coco_bin_from_env() -> Option { + if let Ok(path) = env::var("COCO_TEST_BIN") { + return Some(PathBuf::from(path)); + } + if let Ok(path) = env::var("COCO_TUI_BIN") { + return Some(PathBuf::from(path)); + } + if let Ok(path) = env::var("CARGO_BIN_EXE_coco") { + return Some(PathBuf::from(path)); + } + None +} + +fn resolve_coco_bin_from_target() -> PathBuf { + let profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); + let base = if let Some(target_dir) = env::var_os("CARGO_TARGET_DIR") { + PathBuf::from(target_dir).join(profile) + } else { + repo_root().join("target").join(profile) + }; + let mut path = base.join("coco"); + if cfg!(windows) { + path.set_extension("exe"); + } + path +} + +pub(crate) fn coco_binary() -> PathBuf { + let path = resolve_coco_bin_from_env().unwrap_or_else(resolve_coco_bin_from_target); + assert!( + path.exists(), + "coco binary not found at {:?}; build `cargo build -p coco-tui --bin coco` or set COCO_TUI_BIN/COCO_TEST_BIN", + path + ); + path +} + pub(crate) fn spawn_tui( config_dir: Option<&Path>, args: &[&str], +) -> (Box, Box, Box) { + spawn_tui_with_env(config_dir, args, &[]) +} + +pub(crate) fn spawn_tui_with_env( + config_dir: Option<&Path>, + args: &[&str], + envs: &[(&str, &str)], ) -> (Box, Box, Box) { let pty_system = NativePtySystem::default(); let pair = pty_system @@ -56,8 +92,7 @@ pub(crate) fn spawn_tui( }) .expect("open pty"); - let mut cmd = CommandBuilder::new("cargo"); - cmd.args(["run", "-p", "coco-tui", "--bin", "coco", "--"]); + let mut cmd = CommandBuilder::new(coco_binary()); cmd.cwd(repo_root()); if let Some(dir) = config_dir { @@ -65,6 +100,9 @@ pub(crate) fn spawn_tui( } cmd.env("COCO_LOG", "trace"); + for (key, value) in envs { + cmd.env(key, value); + } if !args.is_empty() { cmd.args(args); From 491fd2201082e21c975ec007108e5fc3b782346b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 15:00:58 +0800 Subject: [PATCH 05/20] test: add unit tests for interactive offload reply functions --- src/combo/runner.rs | 69 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 45221f2..a406785 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -1775,6 +1775,19 @@ fn display_combo_arg(value: &str) -> String { mod tests { use super::*; + fn prompt_schemas() -> Vec { + vec![ + PromptSchema { + name: "message".to_string(), + description: "Final answer".to_string(), + }, + PromptSchema { + name: "status".to_string(), + description: "Execution status".to_string(), + }, + ] + } + #[test] fn serialize_run_combo_output() { let output = RunComboOutput { @@ -1790,4 +1803,60 @@ mod tests { assert_eq!(json["tool_calls"], 3); assert!(json.get("error").is_none() || json["error"].is_null()); } + + #[test] + fn interactive_offload_directive_contains_required_fields() { + let directive = build_interactive_offload_reply_directive(&prompt_schemas()); + assert!(directive.contains("interact in multiple rounds")); + assert!(directive.contains("coco reply --message= --status=")); + assert!(directive.contains("- --message=: Final answer")); + assert!(directive.contains("- --status=: Execution status")); + } + + #[test] + fn interactive_offload_reminder_contains_reply_command() { + let reminder = build_interactive_offload_reply_reminder(&prompt_schemas()); + assert!(reminder.contains("Continue if needed")); + assert!(reminder.contains("coco reply --message=... --status=...")); + } + + #[test] + fn is_coco_reply_command_accepts_expected_forms() { + assert!(is_coco_reply_command("coco reply --message='hello world'")); + assert!(is_coco_reply_command( + "/usr/local/bin/coco reply --message='hello world'" + )); + assert!(!is_coco_reply_command("coco ask hello")); + assert!(!is_coco_reply_command("echo coco reply --message=hello")); + } + + #[test] + fn classify_offload_command_distinguishes_coco_safe_and_unsafe() { + assert!(matches!( + classify_offload_command("coco reply --message=done --status=ok"), + OffloadCommandKind::Coco + )); + assert!(matches!( + classify_offload_command("ls -la"), + OffloadCommandKind::Safe + )); + assert!(matches!( + classify_offload_command("rm -rf /"), + OffloadCommandKind::Unsafe + )); + } + + #[test] + fn offload_reply_guidance_reflects_command_status() { + let blocked = build_offload_reply_guidance(&prompt_schemas(), "rm -rf /", false); + assert!(blocked.contains("previous tool call was blocked")); + assert!(blocked.contains("command: rm -rf /")); + assert!(blocked.contains("- message: Final answer")); + assert!(blocked.contains("- status: Execution status")); + + let executed = build_offload_reply_guidance(&prompt_schemas(), "ls -la", true); + assert!(executed.contains("previous tool call was executed")); + assert!(executed.contains("command: ls -la")); + assert!(executed.contains("coco reply --message=... --status=...")); + } } From 3b3dc441e8e1907e828a0bc13c742aaf310206c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 15:06:54 +0800 Subject: [PATCH 06/20] test: add mock e2e for combo interactive ask feedback --- crates/coco-tui/src/e2e_tests.rs | 1 + .../src/e2e_tests/combo_interactive.rs | 91 +++++ crates/coco-tui/src/e2e_tests/support.rs | 386 +++++++++++++++++- 3 files changed, 475 insertions(+), 3 deletions(-) create mode 100644 crates/coco-tui/src/e2e_tests/combo_interactive.rs diff --git a/crates/coco-tui/src/e2e_tests.rs b/crates/coco-tui/src/e2e_tests.rs index 4d400e4..4e093e0 100644 --- a/crates/coco-tui/src/e2e_tests.rs +++ b/crates/coco-tui/src/e2e_tests.rs @@ -1,2 +1,3 @@ +mod combo_interactive; mod startup; mod support; diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs new file mode 100644 index 0000000..4f6266d --- /dev/null +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -0,0 +1,91 @@ +use std::time::Duration; + +use vt100::Parser; + +use super::support::{ + KillOnDrop, MockOpenAiServer, assert_screen_not_contains, create_mock_e2e_config, + send_alt_enter, send_text, shutdown_tui, spawn_reader, spawn_tui, wait_for_screen_contains, +}; + +const COMBO_NAME: &str = "e2e_mock_interactive"; + +fn mock_combo_script() -> String { + format!( + r#"#!/usr/bin/env bash +set -euo pipefail + +input="${{*:-Long time no see}}" + +coco metadata name={COMBO_NAME} description="mock interactive combo" || exit 0 + +response="$(coco ask -i --schemas result:result --schemas reason:reason "Process input: ${{input}}")" +echo "$response" +"# + ) +} + +#[test] +fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { + let mock = MockOpenAiServer::start("E2E_FEEDBACK_TOKEN"); + let temp = create_mock_e2e_config(mock.base_url(), false, COMBO_NAME, &mock_combo_script()); + let config_dir = temp.path().join("coco"); + let (mut child, reader, mut writer) = spawn_tui( + Some(&config_dir), + &["combo", "run", COMBO_NAME, "Long time no see"], + ); + let mut guard = KillOnDrop::new(child.clone_killer()); + let rx = spawn_reader(reader); + let mut parser = Parser::new(24, 120, 0); + + wait_for_screen_contains( + &mut parser, + &rx, + "Combo: e2e_mock_interactive", + Duration::from_secs(30), + ); + + wait_for_screen_contains( + &mut parser, + &rx, + "Please provide feedback first", + Duration::from_secs(20), + ); + + assert_screen_not_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(2), + ); + + send_text(&mut writer, "Use user feedback E2E_FEEDBACK_TOKEN"); + send_alt_enter(&mut writer); + + wait_for_screen_contains( + &mut parser, + &rx, + "E2E_FEEDBACK_TOKEN", + Duration::from_secs(30), + ); + + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + + let status = shutdown_tui(&mut *child, &mut writer, &parser); + guard.disarm(); + assert!(status.success(), "tui exit failed: {status:?}"); + + assert!( + mock.saw_feedback_token(), + "mock provider never observed user feedback token in LLM request history" + ); + assert!( + mock.request_count() >= 2, + "expected at least two model requests, got {}", + mock.request_count() + ); +} diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index bff744c..6b4944f 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -1,15 +1,22 @@ use std::{ - env, - io::{Read, Write}, + env, fs, + io::{BufRead, BufReader, Read, Write}, + net::{TcpListener, TcpStream}, path::{Path, PathBuf}, - sync::mpsc::{Receiver, RecvTimeoutError}, + sync::{ + Arc, Mutex, + atomic::{AtomicBool, Ordering}, + mpsc::{Receiver, RecvTimeoutError}, + }, thread, time::{Duration, Instant}, }; +use code_combo::{EnvString, load_config_with_overrides, workspace_config_path}; use portable_pty::{ Child, ChildKiller, CommandBuilder, ExitStatus, NativePtySystem, PtySize, PtySystem, }; +use tempfile::TempDir; use vt100::Parser; pub(crate) type PtyChild = dyn Child + Send; @@ -24,6 +31,349 @@ pub(crate) fn require_e2e_config_dir() -> PathBuf { config_dir } +#[allow(dead_code)] +pub(crate) fn missing_provider_envs(config_dir: &Path) -> Vec { + let config_path = config_dir.join("config.toml"); + let workspace_path = workspace_config_path(); + let config = load_config_with_overrides(&config_path, config_dir, Some(&workspace_path)) + .expect("load e2e config"); + + let mut missing = Vec::new(); + for provider in config.providers { + if let EnvString::EnvVar { name, .. } = provider.api_key + && env::var(&name).is_err() + && !missing.contains(&name) + { + missing.push(name); + } + } + missing +} + +#[allow(dead_code)] +pub(crate) fn create_e2e_config_with_auto_accept(auto_accept_edits: bool) -> TempDir { + let source_config_dir = require_e2e_config_dir(); + let temp = TempDir::new().expect("create e2e temp config dir"); + let config_dir = temp.path().join("coco"); + fs::create_dir_all(&config_dir).expect("create temp config dir"); + + let source_config_path = source_config_dir.join("config.toml"); + let target_config_path = config_dir.join("config.toml"); + fs::copy(&source_config_path, &target_config_path).expect("copy base config"); + + let override_path = config_dir.join("config.overwrite.toml"); + fs::write( + &override_path, + format!("[agent]\nauto_accept_edits = {auto_accept_edits}\n"), + ) + .expect("write runtime overrides"); + + temp +} + +pub(crate) fn create_mock_e2e_config( + base_url: &str, + auto_accept_edits: bool, + combo_name: &str, + combo_script: &str, +) -> TempDir { + let temp = TempDir::new().expect("create e2e temp config dir"); + let config_dir = temp.path().join("coco"); + let combos_dir = config_dir.join("combos"); + fs::create_dir_all(&combos_dir).expect("create temp combo dir"); + + let config_path = config_dir.join("config.toml"); + let config_content = format!( + r#"[ui.notifications] +enabled = false + +[agent] +auto_accept_edits = {auto_accept_edits} + +[[providers]] +name = "mock-openai" +kind = "openai" +api_key = "mock-api-key" +base_url = "{base_url}" +disable_stream = true +offload_combo_reply = true +"# + ); + fs::write(&config_path, config_content).expect("write mock e2e config"); + + let combo_path = combos_dir.join(format!("{combo_name}.sh")); + fs::write(&combo_path, combo_script).expect("write combo script"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&combo_path) + .expect("combo metadata") + .permissions(); + perms.set_mode(0o755); + fs::set_permissions(&combo_path, perms).expect("set combo executable permissions"); + } + + temp +} + +#[derive(Debug, Default)] +struct MockOpenAiState { + request_count: usize, + saw_feedback_token: bool, +} + +pub(crate) struct MockOpenAiServer { + base_url: String, + shutdown: Arc, + state: Arc>, + worker: Option>, +} + +impl MockOpenAiServer { + pub(crate) fn start(feedback_token: impl Into) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind mock openai listener"); + listener + .set_nonblocking(true) + .expect("set listener nonblocking"); + let addr = listener.local_addr().expect("get listener addr"); + let base_url = format!("http://{addr}"); + let shutdown = Arc::new(AtomicBool::new(false)); + let state = Arc::new(Mutex::new(MockOpenAiState::default())); + let feedback_token = feedback_token.into(); + let worker = thread::spawn({ + let shutdown = shutdown.clone(); + let state = state.clone(); + let feedback_token = feedback_token.clone(); + move || { + while !shutdown.load(Ordering::Relaxed) { + match listener.accept() { + Ok((mut stream, _)) => { + handle_mock_openai_connection(&mut stream, &feedback_token, &state); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(10)); + } + Err(_) => break, + } + } + } + }); + Self { + base_url, + shutdown, + state, + worker: Some(worker), + } + } + + pub(crate) fn base_url(&self) -> &str { + &self.base_url + } + + pub(crate) fn request_count(&self) -> usize { + self.state.lock().expect("lock mock state").request_count + } + + pub(crate) fn saw_feedback_token(&self) -> bool { + self.state + .lock() + .expect("lock mock state") + .saw_feedback_token + } +} + +impl Drop for MockOpenAiServer { + fn drop(&mut self) { + self.shutdown.store(true, Ordering::Relaxed); + // Unblock accept quickly. + let _ = TcpStream::connect(self.base_url.trim_start_matches("http://")); + if let Some(worker) = self.worker.take() { + let _ = worker.join(); + } + } +} + +fn handle_mock_openai_connection( + stream: &mut TcpStream, + feedback_token: &str, + state: &Arc>, +) { + let Ok((path, body)) = read_http_request(stream) else { + let _ = write_http_response(stream, 400, br#"{"error":"bad request"}"#); + return; + }; + if path != "/v1/chat/completions" && path != "/chat/completions" { + let _ = write_http_response(stream, 404, br#"{"error":"not found"}"#); + return; + } + + let request: serde_json::Value = match serde_json::from_slice(&body) { + Ok(value) => value, + Err(_) => { + let _ = write_http_response(stream, 400, br#"{"error":"invalid json"}"#); + return; + } + }; + let response = build_mock_openai_response(&request, feedback_token, state); + let body = serde_json::to_vec(&response).expect("serialize mock openai response"); + let _ = write_http_response(stream, 200, &body); +} + +fn read_http_request(stream: &mut TcpStream) -> Result<(String, Vec), std::io::Error> { + let mut reader = BufReader::new(stream); + let mut request_line = String::new(); + reader.read_line(&mut request_line)?; + if request_line.trim().is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "missing request line", + )); + } + let mut parts = request_line.split_whitespace(); + let _method = parts + .next() + .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidData, "missing method"))?; + let path = parts + .next() + .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidData, "missing path"))? + .to_string(); + + let mut content_length = 0usize; + loop { + let mut line = String::new(); + reader.read_line(&mut line)?; + if line == "\r\n" { + break; + } + if let Some(value) = line.strip_prefix("Content-Length:") { + content_length = value.trim().parse::().unwrap_or(0); + } else if let Some(value) = line.strip_prefix("content-length:") { + content_length = value.trim().parse::().unwrap_or(0); + } + } + + let mut body = vec![0u8; content_length]; + reader.read_exact(&mut body)?; + Ok((path, body)) +} + +fn write_http_response( + stream: &mut TcpStream, + status: u16, + body: &[u8], +) -> Result<(), std::io::Error> { + let status_text = match status { + 200 => "OK", + 400 => "Bad Request", + 404 => "Not Found", + _ => "Internal Server Error", + }; + let head = format!( + "HTTP/1.1 {status} {status_text}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ); + stream.write_all(head.as_bytes())?; + stream.write_all(body)?; + stream.flush()?; + Ok(()) +} + +fn build_mock_openai_response( + request: &serde_json::Value, + feedback_token: &str, + state: &Arc>, +) -> serde_json::Value { + let user_text = request + .get("messages") + .and_then(|value| value.as_array()) + .map(|messages| { + messages + .iter() + .filter_map(|message| { + let role = message.get("role").and_then(|value| value.as_str())?; + if role != "user" { + return None; + } + message + .get("content") + .and_then(|value| value.as_str()) + .map(str::to_string) + }) + .collect::>() + .join("\n") + }) + .unwrap_or_default(); + + let is_summary_prompt = user_text.contains("Summarize the combo execution for the user."); + let saw_feedback = user_text.contains(feedback_token); + + { + let mut guard = state.lock().expect("lock mock state"); + guard.request_count += 1; + guard.saw_feedback_token |= saw_feedback; + } + + if is_summary_prompt { + return mock_text_response( + "- status: success\n- interaction: feedback captured\n- output: reply fields generated", + ); + } + if saw_feedback { + let command = "coco reply --result='mock polished result' --reason='used user feedback'"; + return mock_bash_tool_call_response(command); + } + + // Slow down no-feedback turns so the UI has time to capture manual input. + thread::sleep(Duration::from_millis(120)); + mock_text_response(&format!( + "Please provide feedback first (include token: {feedback_token})." + )) +} + +fn mock_text_response(content: &str) -> serde_json::Value { + serde_json::json!({ + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "content": content + }, + "finish_reason": "stop" + }], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 10, + "total_tokens": 20 + } + }) +} + +fn mock_bash_tool_call_response(command: &str) -> serde_json::Value { + let arguments = serde_json::json!({ "command": command }).to_string(); + serde_json::json!({ + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "tool_calls": [{ + "id": "call_mock_bash", + "type": "function", + "function": { + "name": "bash", + "arguments": arguments + } + }] + }, + "finish_reason": "tool_calls" + }], + "usage": { + "prompt_tokens": 12, + "completion_tokens": 14, + "total_tokens": 26 + } + }) +} + pub(crate) fn repo_root() -> PathBuf { let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); manifest_dir @@ -148,6 +498,36 @@ pub(crate) fn wait_for_screen_contains( } } +pub(crate) fn assert_screen_not_contains( + parser: &mut Parser, + rx: &Receiver>, + needle: &str, + timeout: Duration, +) { + let start = Instant::now(); + loop { + let elapsed = start.elapsed(); + if elapsed > timeout { + return; + } + let remaining = timeout.saturating_sub(elapsed); + match rx.recv_timeout(remaining.min(Duration::from_millis(100))) { + Ok(chunk) => { + if !chunk.is_empty() { + parser.process(&chunk); + } + } + Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => return, + } + let screen = parser.screen().contents(); + assert!( + !screen.contains(needle), + "unexpected content found in screen: {needle}\n--- screen ---\n{screen}" + ); + } +} + #[allow(dead_code)] pub(crate) fn send_enter(writer: &mut dyn Write) { writer.write_all(&[0x0d]).expect("write enter"); From a70014a2a256b59d3c383b0e9e203e39ad2dcb86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 15:11:35 +0800 Subject: [PATCH 07/20] test: make startup e2e config self-contained --- crates/coco-tui/src/e2e_tests/startup.rs | 8 +++++--- crates/coco-tui/src/e2e_tests/support.rs | 25 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/coco-tui/src/e2e_tests/startup.rs b/crates/coco-tui/src/e2e_tests/startup.rs index 2ff9068..b9e5c21 100644 --- a/crates/coco-tui/src/e2e_tests/startup.rs +++ b/crates/coco-tui/src/e2e_tests/startup.rs @@ -4,13 +4,14 @@ use tempfile::TempDir; use vt100::Parser; use super::support::{ - KillOnDrop, coco_binary, require_e2e_config_dir, shutdown_tui, spawn_reader, spawn_tui, + KillOnDrop, coco_binary, create_minimal_e2e_config, shutdown_tui, spawn_reader, spawn_tui, spawn_tui_with_env, wait_for_screen_contains, }; #[test] fn tui_starts_and_exits_on_ctrl_c() { - let config_dir = require_e2e_config_dir(); + let temp = create_minimal_e2e_config(true); + let config_dir = temp.path().join("coco"); let (mut child, reader, mut writer) = spawn_tui(Some(&config_dir), &[]); let mut guard = KillOnDrop::new(child.clone_killer()); let rx = spawn_reader(reader); @@ -25,7 +26,8 @@ fn tui_starts_and_exits_on_ctrl_c() { #[test] fn combo_run_not_found_is_visible_in_tui() { - let config_dir = require_e2e_config_dir(); + let temp_config = create_minimal_e2e_config(true); + let config_dir = temp_config.path().join("coco"); let temp = TempDir::new().expect("tempdir"); let socket_path = temp.path().join("coco-e2e.sock"); let socket = socket_path.to_string_lossy().to_string(); diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index 6b4944f..cc45c5b 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -116,6 +116,31 @@ offload_combo_reply = true temp } +pub(crate) fn create_minimal_e2e_config(auto_accept_edits: bool) -> TempDir { + let temp = TempDir::new().expect("create e2e temp config dir"); + let config_dir = temp.path().join("coco"); + fs::create_dir_all(config_dir.join("combos")).expect("create temp combo dir"); + + let config_path = config_dir.join("config.toml"); + let config_content = format!( + r#"[ui.notifications] +enabled = false + +[agent] +auto_accept_edits = {auto_accept_edits} + +[[providers]] +name = "minimal-openai" +kind = "openai" +api_key = "test-api-key" +base_url = "http://127.0.0.1:1" +disable_stream = true +"# + ); + fs::write(&config_path, config_content).expect("write minimal e2e config"); + temp +} + #[derive(Debug, Default)] struct MockOpenAiState { request_count: usize, From 9c140dd0794455d4edd64b5e68ec4697f2277ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 16:22:10 +0800 Subject: [PATCH 08/20] test: expand combo interactive e2e and enforce bash confirmation --- .../src/e2e_tests/combo_interactive.rs | 226 ++++++++++++++++-- crates/coco-tui/src/e2e_tests/support.rs | 124 ++++++++-- src/agent/bash_executor.rs | 23 +- src/agent/executor.rs | 30 +-- 4 files changed, 326 insertions(+), 77 deletions(-) diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index 4f6266d..ccb4dbd 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -3,11 +3,20 @@ use std::time::Duration; use vt100::Parser; use super::support::{ - KillOnDrop, MockOpenAiServer, assert_screen_not_contains, create_mock_e2e_config, - send_alt_enter, send_text, shutdown_tui, spawn_reader, spawn_tui, wait_for_screen_contains, + KillOnDrop, MockOpenAiScenario, MockOpenAiServer, assert_screen_not_contains, + create_mock_e2e_config, send_alt_enter, send_text, shutdown_tui, spawn_reader, spawn_tui, + wait_for_screen_contains, }; const COMBO_NAME: &str = "e2e_mock_interactive"; +type ComboHarness = ( + KillOnDrop, + Box, + std::sync::mpsc::Receiver>, + Parser, + Box, + tempfile::TempDir, +); fn mock_combo_script() -> String { format!( @@ -24,18 +33,46 @@ echo "$response" ) } -#[test] -fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { - let mock = MockOpenAiServer::start("E2E_FEEDBACK_TOKEN"); - let temp = create_mock_e2e_config(mock.base_url(), false, COMBO_NAME, &mock_combo_script()); +fn run_combo_with_mock(mock: &MockOpenAiServer) -> ComboHarness { + run_combo_with_mock_and_auto_accept(mock, false) +} + +fn run_combo_with_mock_and_auto_accept( + mock: &MockOpenAiServer, + auto_accept_edits: bool, +) -> ComboHarness { + let temp = create_mock_e2e_config( + mock.base_url(), + auto_accept_edits, + COMBO_NAME, + &mock_combo_script(), + ); let config_dir = temp.path().join("coco"); - let (mut child, reader, mut writer) = spawn_tui( + let (child, reader, writer) = spawn_tui( Some(&config_dir), &["combo", "run", COMBO_NAME, "Long time no see"], ); - let mut guard = KillOnDrop::new(child.clone_killer()); + let guard = KillOnDrop::new(child.clone_killer()); let rx = spawn_reader(reader); - let mut parser = Parser::new(24, 120, 0); + let parser = Parser::new(24, 120, 0); + (guard, writer, rx, parser, child, temp) +} + +fn assert_shutdown_ok( + child: &mut super::support::PtyChild, + writer: &mut dyn std::io::Write, + parser: &Parser, + guard: &mut KillOnDrop, +) { + let status = shutdown_tui(child, writer, parser); + guard.disarm(); + assert!(status.success(), "tui exit failed: {status:?}"); +} + +#[test] +fn combo_interactive_shows_feedback_prompt_before_reply_tool_use() { + let mock = MockOpenAiServer::start("E2E_TOKEN_PROMPT"); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); wait_for_screen_contains( &mut parser, @@ -43,14 +80,12 @@ fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { "Combo: e2e_mock_interactive", Duration::from_secs(30), ); - wait_for_screen_contains( &mut parser, &rx, "Please provide feedback first", Duration::from_secs(20), ); - assert_screen_not_contains( &mut parser, &rx, @@ -58,16 +93,37 @@ fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { Duration::from_secs(2), ); - send_text(&mut writer, "Use user feedback E2E_FEEDBACK_TOKEN"); - send_alt_enter(&mut writer); + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); + assert!( + mock.request_count() >= 1, + "expected at least one LLM call, got {}", + mock.request_count() + ); + assert!( + !mock.saw_feedback_token(), + "unexpectedly observed feedback token before user input" + ); +} + +#[test] +fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { + let mock = MockOpenAiServer::start("E2E_FEEDBACK_TOKEN"); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + wait_for_screen_contains( + &mut parser, + &rx, + "Please provide feedback first", + Duration::from_secs(20), + ); + send_text(&mut *writer, "Use user feedback E2E_FEEDBACK_TOKEN"); + send_alt_enter(&mut *writer); wait_for_screen_contains( &mut parser, &rx, "E2E_FEEDBACK_TOKEN", Duration::from_secs(30), ); - wait_for_screen_contains( &mut parser, &rx, @@ -75,10 +131,7 @@ fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { Duration::from_secs(30), ); - let status = shutdown_tui(&mut *child, &mut writer, &parser); - guard.disarm(); - assert!(status.success(), "tui exit failed: {status:?}"); - + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); assert!( mock.saw_feedback_token(), "mock provider never observed user feedback token in LLM request history" @@ -89,3 +142,140 @@ fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { mock.request_count() ); } + +#[test] +fn combo_interactive_requires_two_feedback_rounds_before_reply_tool_use() { + let mock = + MockOpenAiServer::start_with_scenario(MockOpenAiScenario::RequireFeedbackTokens(vec![ + "E2E_TOKEN_ONE".to_string(), + "E2E_TOKEN_TWO".to_string(), + ])); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Please provide feedback first", + Duration::from_secs(20), + ); + + send_text(&mut *writer, "first round E2E_TOKEN_ONE"); + send_alt_enter(&mut *writer); + wait_for_screen_contains( + &mut parser, + &rx, + "missing: E2E_TOKEN_TWO", + Duration::from_secs(30), + ); + assert_screen_not_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(2), + ); + + send_text(&mut *writer, "second round E2E_TOKEN_TWO"); + send_alt_enter(&mut *writer); + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); + assert!(mock.saw_token("E2E_TOKEN_ONE")); + assert!(mock.saw_token("E2E_TOKEN_TWO")); +} + +#[test] +fn combo_interactive_reply_tool_use_command_contains_required_fields() { + let mock = MockOpenAiServer::start("E2E_CMD_TOKEN"); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Please provide feedback first", + Duration::from_secs(20), + ); + send_text(&mut *writer, "feedback E2E_CMD_TOKEN"); + send_alt_enter(&mut *writer); + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + wait_for_screen_contains( + &mut parser, + &rx, + "coco reply --result='mock polished result'", + Duration::from_secs(30), + ); + wait_for_screen_contains( + &mut parser, + &rx, + "--reason='used user feedback'", + Duration::from_secs(30), + ); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); +} + +#[test] +fn combo_interactive_rate_limit_error_is_visible() { + let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::RateLimited); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains(&mut parser, &rx, "chat failed:", Duration::from_secs(30)); + wait_for_screen_contains(&mut parser, &rx, "429", Duration::from_secs(30)); + assert_screen_not_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(2), + ); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); + assert!(mock.request_count() >= 1); +} + +#[test] +fn combo_interactive_rate_limit_does_not_fake_complete() { + let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::RateLimited); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains(&mut parser, &rx, "chat failed:", Duration::from_secs(30)); + assert_screen_not_contains(&mut parser, &rx, "Completed", Duration::from_secs(2)); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); +} + +#[test] +fn combo_interactive_auto_accept_edits_still_requires_bash_confirmation() { + let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::ImmediateReply); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = + run_combo_with_mock_and_auto_accept(&mock, true); + + wait_for_screen_contains( + &mut parser, + &rx, + "Combo: e2e_mock_interactive", + Duration::from_secs(30), + ); + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + wait_for_screen_contains( + &mut parser, + &rx, + "coco reply --result='mock polished result'", + Duration::from_secs(30), + ); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); +} diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index cc45c5b..ece1efc 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashSet, env, fs, io::{BufRead, BufReader, Read, Write}, net::{TcpListener, TcpStream}, @@ -97,6 +98,8 @@ api_key = "mock-api-key" base_url = "{base_url}" disable_stream = true offload_combo_reply = true +retry_max_attempts = 1 +retry_max_delay_ms = 10 "# ); fs::write(&config_path, config_content).expect("write mock e2e config"); @@ -144,11 +147,19 @@ disable_stream = true #[derive(Debug, Default)] struct MockOpenAiState { request_count: usize, - saw_feedback_token: bool, + seen_tokens: HashSet, +} + +#[derive(Debug, Clone)] +pub(crate) enum MockOpenAiScenario { + RequireFeedbackTokens(Vec), + ImmediateReply, + RateLimited, } pub(crate) struct MockOpenAiServer { base_url: String, + primary_feedback_token: Option, shutdown: Arc, state: Arc>, worker: Option>, @@ -156,6 +167,18 @@ pub(crate) struct MockOpenAiServer { impl MockOpenAiServer { pub(crate) fn start(feedback_token: impl Into) -> Self { + let feedback_token = feedback_token.into(); + Self::start_inner( + Some(feedback_token.clone()), + MockOpenAiScenario::RequireFeedbackTokens(vec![feedback_token]), + ) + } + + pub(crate) fn start_with_scenario(scenario: MockOpenAiScenario) -> Self { + Self::start_inner(None, scenario) + } + + fn start_inner(primary_feedback_token: Option, scenario: MockOpenAiScenario) -> Self { let listener = TcpListener::bind("127.0.0.1:0").expect("bind mock openai listener"); listener .set_nonblocking(true) @@ -164,16 +187,15 @@ impl MockOpenAiServer { let base_url = format!("http://{addr}"); let shutdown = Arc::new(AtomicBool::new(false)); let state = Arc::new(Mutex::new(MockOpenAiState::default())); - let feedback_token = feedback_token.into(); let worker = thread::spawn({ let shutdown = shutdown.clone(); let state = state.clone(); - let feedback_token = feedback_token.clone(); + let scenario = scenario.clone(); move || { while !shutdown.load(Ordering::Relaxed) { match listener.accept() { Ok((mut stream, _)) => { - handle_mock_openai_connection(&mut stream, &feedback_token, &state); + handle_mock_openai_connection(&mut stream, &scenario, &state); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(10)); @@ -185,6 +207,7 @@ impl MockOpenAiServer { }); Self { base_url, + primary_feedback_token, shutdown, state, worker: Some(worker), @@ -200,10 +223,18 @@ impl MockOpenAiServer { } pub(crate) fn saw_feedback_token(&self) -> bool { + let Some(token) = self.primary_feedback_token.as_ref() else { + return false; + }; + self.saw_token(token) + } + + pub(crate) fn saw_token(&self, token: &str) -> bool { self.state .lock() .expect("lock mock state") - .saw_feedback_token + .seen_tokens + .contains(token) } } @@ -220,7 +251,7 @@ impl Drop for MockOpenAiServer { fn handle_mock_openai_connection( stream: &mut TcpStream, - feedback_token: &str, + scenario: &MockOpenAiScenario, state: &Arc>, ) { let Ok((path, body)) = read_http_request(stream) else { @@ -239,9 +270,16 @@ fn handle_mock_openai_connection( return; } }; - let response = build_mock_openai_response(&request, feedback_token, state); - let body = serde_json::to_vec(&response).expect("serialize mock openai response"); - let _ = write_http_response(stream, 200, &body); + match build_mock_openai_response(&request, scenario, state) { + MockHttpResponse::Ok(response) => { + let body = serde_json::to_vec(&response).expect("serialize mock openai response"); + let _ = write_http_response(stream, 200, &body); + } + MockHttpResponse::Error { status, body } => { + let body = serde_json::to_vec(&body).expect("serialize mock openai error"); + let _ = write_http_response(stream, status, &body); + } + } } fn read_http_request(stream: &mut TcpStream) -> Result<(String, Vec), std::io::Error> { @@ -303,11 +341,19 @@ fn write_http_response( Ok(()) } +enum MockHttpResponse { + Ok(serde_json::Value), + Error { + status: u16, + body: serde_json::Value, + }, +} + fn build_mock_openai_response( request: &serde_json::Value, - feedback_token: &str, + scenario: &MockOpenAiScenario, state: &Arc>, -) -> serde_json::Value { +) -> MockHttpResponse { let user_text = request .get("messages") .and_then(|value| value.as_array()) @@ -330,29 +376,59 @@ fn build_mock_openai_response( .unwrap_or_default(); let is_summary_prompt = user_text.contains("Summarize the combo execution for the user."); - let saw_feedback = user_text.contains(feedback_token); { let mut guard = state.lock().expect("lock mock state"); guard.request_count += 1; - guard.saw_feedback_token |= saw_feedback; + if let MockOpenAiScenario::RequireFeedbackTokens(tokens) = scenario { + for token in tokens { + if user_text.contains(token) { + guard.seen_tokens.insert(token.clone()); + } + } + } } if is_summary_prompt { - return mock_text_response( + return MockHttpResponse::Ok(mock_text_response( "- status: success\n- interaction: feedback captured\n- output: reply fields generated", - ); - } - if saw_feedback { - let command = "coco reply --result='mock polished result' --reason='used user feedback'"; - return mock_bash_tool_call_response(command); + )); } - // Slow down no-feedback turns so the UI has time to capture manual input. - thread::sleep(Duration::from_millis(120)); - mock_text_response(&format!( - "Please provide feedback first (include token: {feedback_token})." - )) + match scenario { + MockOpenAiScenario::RateLimited => MockHttpResponse::Error { + status: 429, + body: serde_json::json!({ + "error": { + "message": "mock rate limited", + "type": "rate_limit_error", + "code": "rate_limit" + } + }), + }, + MockOpenAiScenario::ImmediateReply => { + let command = + "coco reply --result='mock polished result' --reason='used user feedback'"; + MockHttpResponse::Ok(mock_bash_tool_call_response(command)) + } + MockOpenAiScenario::RequireFeedbackTokens(tokens) => { + let missing = tokens + .iter() + .filter(|token| !user_text.contains(token.as_str())) + .cloned() + .collect::>(); + if missing.is_empty() { + let command = + "coco reply --result='mock polished result' --reason='used user feedback'"; + return MockHttpResponse::Ok(mock_bash_tool_call_response(command)); + } + thread::sleep(Duration::from_millis(120)); + MockHttpResponse::Ok(mock_text_response(&format!( + "Please provide feedback first (missing: {}).", + missing.join(", ") + ))) + } + } } fn mock_text_response(content: &str) -> serde_json::Value { diff --git a/src/agent/bash_executor.rs b/src/agent/bash_executor.rs index e054776..ffce141 100644 --- a/src/agent/bash_executor.rs +++ b/src/agent/bash_executor.rs @@ -9,13 +9,10 @@ use lazy_static::lazy_static; use tracing::warn; use tree_sitter::{Node, Parser}; -use crate::{ - config::{ - ArgPolicy, BashConfig, BashConfigLayers, FlagPolicy, FlagValuePolicy, FlagValueType, - SafeCommandRule, SafeCommandsMode, build_safe_command_rules_from_entries, - load_safe_command_rules_from_path, parse_safe_command_rules, - }, - tools::BashInput, +use crate::config::{ + ArgPolicy, BashConfig, BashConfigLayers, FlagPolicy, FlagValuePolicy, FlagValueType, + SafeCommandRule, SafeCommandsMode, build_safe_command_rules_from_entries, + load_safe_command_rules_from_path, parse_safe_command_rules, }; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -66,10 +63,6 @@ fn safe_command_rules() -> &'static RwLock> { SAFE_COMMAND_RULES.get_or_init(|| RwLock::new(BUILTIN_SAFE_COMMAND_RULES.clone())) } -pub fn should_bypass_permission(input: &BashInput) -> bool { - is_safe_command(&input.command) -} - pub fn parse_primary_command(command: &str) -> Result { let commands = parse_commands(command).map_err(|err| err.to_reason().to_string())?; if commands.len() != 1 { @@ -253,11 +246,13 @@ pub(super) fn configure_safe_commands( *guard = rules; } +#[cfg(test)] fn is_safe_command(command: &str) -> bool { let rules = safe_command_rules().read().expect("safe command lock"); is_safe_command_with_rules(command, &rules) } +#[cfg(test)] fn is_safe_command_with_rules(command: &str, rules: &[SafeCommandRule]) -> bool { let trimmed = command.trim(); if trimmed.is_empty() { @@ -337,6 +332,7 @@ fn apply_agent_safe_commands_layer( Ok(()) } +#[cfg(test)] fn is_safe_args(args: &[ParsedArg], policy: &ArgPolicy) -> bool { match policy { ArgPolicy::Any { @@ -502,6 +498,7 @@ fn unsafe_args_with_mode( Vec::new() } +#[cfg(test)] fn is_safe_args_with_mode( args: &[ParsedArg], flags: &HashMap, @@ -709,6 +706,7 @@ fn command_chain_token_matches(arg: &str, token: &str) -> bool { token == ".*" || arg == token } +#[cfg(test)] fn consume_long_option( token: &ParsedArg, index: usize, @@ -736,6 +734,7 @@ fn check_long_option( check_option_with_name(name, attached_value, token, index, args, flags, mode) } +#[cfg(test)] fn consume_single_dash_long_option( token: &ParsedArg, index: usize, @@ -766,6 +765,7 @@ fn check_single_dash_long_option( )) } +#[cfg(test)] fn consume_option_with_name( name: &str, attached_value: Option<&str>, @@ -960,6 +960,7 @@ fn check_option_with_name( } } +#[cfg(test)] fn parse_short_options( token: &ParsedArg, index: usize, diff --git a/src/agent/executor.rs b/src/agent/executor.rs index b71c6fe..e24d093 100644 --- a/src/agent/executor.rs +++ b/src/agent/executor.rs @@ -8,7 +8,6 @@ use serde_json::Value; use snafu::prelude::*; use tokio_util::sync::CancellationToken; -use super::bash_executor; use crate::{ AppliedTextEdit, ComboEvent, OutputChunk, TextEdit, error, tools::{ @@ -272,29 +271,12 @@ impl Executor { if let Some(_pcl) = self.tools_pcl.get(name) { // TODO: Implement permission control list validation logic unimplemented!("Permission control list validation not yet implemented") - } else { - // Some tools can execute without explicit permission - let bypass_permission = match (&input, name) { - (Input::Starter(value), BASH_TOOL_NAME) => { - serde_json::from_value::(value.clone()) - .ok() - .map(|input| bash_executor::should_bypass_permission(&input)) - .unwrap_or(false) - } - _ => false, - }; - if !bypass_permission - && !matches!( - name, - STR_REPLACE_TOOL_NAME - | READ_TOOL_NAME - | LIST_TOOL_NAME - | RUN_TASK_TOOL_NAME - ) - { - on_output(Output::AskPermission); - return Ok(ExecuteStatus::Completed); - } + } else if !matches!( + name, + STR_REPLACE_TOOL_NAME | READ_TOOL_NAME | LIST_TOOL_NAME | RUN_TASK_TOOL_NAME + ) { + on_output(Output::AskPermission); + return Ok(ExecuteStatus::Completed); }; } From 34d0531023f8d105f0b0e169d5b4031e3413e8e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 16:44:51 +0800 Subject: [PATCH 09/20] fix: require manual confirmation for combo ask -i coco reply --- crates/coco-tui/src/components/chat.rs | 63 ++++++++++++--- .../coco-tui/src/components/messages/combo.rs | 3 + .../src/e2e_tests/combo_interactive.rs | 1 + crates/coco-tui/src/e2e_tests/support.rs | 6 ++ src/combo/runner.rs | 79 +++++++++++++------ 5 files changed, 120 insertions(+), 32 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index 47e8f11..59424ed 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -95,6 +95,7 @@ pub struct Chat<'a> { manual_combo_commands: HashMap, manual_combo_prompted: HashSet, pending_combo_tool_events: HashMap>, + combo_offload_tool_use_to_combo_id: HashMap, last_usage: Option, retry_status: Option, transcript_scopes: Vec, @@ -342,6 +343,7 @@ impl Chat<'static> { manual_combo_commands: HashMap::new(), manual_combo_prompted: HashSet::new(), pending_combo_tool_events: HashMap::new(), + combo_offload_tool_use_to_combo_id: HashMap::new(), last_usage: None, retry_status: None, transcript_scopes: Vec::new(), @@ -564,10 +566,19 @@ impl Chat<'static> { self.set_combo_thinking_active(false); self.set_processing(); } - ComboEvent::ReplyToolUse { offload: true, .. } => { + ComboEvent::ReplyToolUse { + id, + tool_use, + offload: true, + .. + } => { + self.combo_offload_tool_use_to_combo_id + .insert(tool_use.id.clone(), id.clone()); self.set_processing(); } - ComboEvent::Executed { starter, .. } => { + ComboEvent::Executed { id, starter, .. } => { + self.combo_offload_tool_use_to_combo_id + .retain(|_, combo_id| combo_id != id); self.set_combo_thinking_active(false); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); @@ -576,9 +587,21 @@ impl Chat<'static> { // For ExecuteViaBash: manual_combo_runs handles LLM trigger via ToolResult. // For LLM Bash Tool: Bash Tool completion triggers LLM automatically. } - ComboEvent::Discovered { .. } - | ComboEvent::NotFound { .. } - | ComboEvent::Cancelled { .. } => { + ComboEvent::Discovered { .. } => { + self.set_combo_thinking_active(false); + self.set_ready(); + } + ComboEvent::NotFound { id, .. } => { + self.combo_offload_tool_use_to_combo_id + .retain(|_, combo_id| combo_id != id); + self.set_combo_thinking_active(false); + self.set_ready(); + } + ComboEvent::Cancelled { id, .. } => { + if let Some(id) = id { + self.combo_offload_tool_use_to_combo_id + .retain(|_, combo_id| combo_id != id); + } self.set_combo_thinking_active(false); self.set_ready(); } @@ -590,7 +613,8 @@ impl Chat<'static> { self.set_ready(); global::trigger_schedule_session_save(); } - ComboEvent::ReplyToolResult { .. } => { + ComboEvent::ReplyToolResult { tool_use_id, .. } => { + self.combo_offload_tool_use_to_combo_id.remove(tool_use_id); // Result is handled by Combo component } ComboEvent::Transcript { id, name, messages } => { @@ -1067,7 +1091,9 @@ impl Chat<'static> { } self.handle_combo_event(event); } - ComboEvent::Executed { starter, .. } => { + ComboEvent::Executed { id, starter, .. } => { + self.combo_offload_tool_use_to_combo_id + .retain(|_, combo_id| combo_id != id); self.set_combo_thinking_active(false); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); @@ -2276,7 +2302,17 @@ impl Component for Chat<'static> { self.messages.reset_stream(); self.set_ready(); } - Event::Ask(AskEvent::ToolUsePermission(_)) => { + Event::Ask(AskEvent::ToolUsePermission(id)) => { + if let Some(combo_id) = self.combo_offload_tool_use_to_combo_id.get(id) + && let Some(idx) = self.messages.locate_tool_message(combo_id) + { + handle_component_event!(self, event); + self.update_focus(Focus::Messages); + self.messages.focus(idx); + self.notify_action_required("Tool permission requested"); + global::trigger_schedule_session_save(); + return; + } if let Some(idx) = self.messages.on_tool_event(event) { // Move focus to tool use message when permission is required self.update_focus(Focus::Messages); @@ -2316,6 +2352,11 @@ impl Component for Chat<'static> { let is_manual_combo = self.manual_combo_runs.contains(id); let effective_is_error = if *is_user_cancelled { false } else { *is_error }; self.forward_combo_result_to_session(id, output, effective_is_error); + if self.combo_offload_tool_use_to_combo_id.remove(id).is_some() { + handle_component_event!(self, event); + global::trigger_schedule_session_save(); + return; + } if let Some(idx) = self.messages.on_tool_event(event) && !effective_is_error && self.messages.selected_idx() == Some(idx) @@ -2465,7 +2506,11 @@ impl Component for Chat<'static> { self.set_processing(); let _ = self.messages.on_tool_event(evt); } - Event::Answer(AnswerEvent::ToolOutput { .. }) => { + Event::Answer(AnswerEvent::ToolOutput { id, .. }) => { + if self.combo_offload_tool_use_to_combo_id.contains_key(id) { + handle_component_event!(self, event); + return; + } let _ = self.messages.on_tool_event(event); } _ => { diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index 2109f4d..9320a7b 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -321,7 +321,10 @@ impl Combo { fn push_offload_bash_tool_use(&mut self, tool_use: ToolUse) { self.state.write().view = ComboView::Messages; + let tool_use_id = tool_use.id.clone(); self.messages.push(Message::bot(Tool::new(tool_use).into())); + let ask = Event::Ask(AskEvent::ToolUsePermission(tool_use_id)); + let _ = self.messages.on_tool_event(&ask); } fn forward_output_to_child(&mut self, tool_use_id: &str, chunk: &OutputChunk) -> bool { diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index ccb4dbd..e284892 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -276,6 +276,7 @@ fn combo_interactive_auto_accept_edits_still_requires_bash_confirmation() { "coco reply --result='mock polished result'", Duration::from_secs(30), ); + assert_screen_not_contains(&mut parser, &rx, "Completed", Duration::from_secs(2)); assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); } diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index ece1efc..25a3262 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -635,6 +635,12 @@ pub(crate) fn send_enter(writer: &mut dyn Write) { writer.flush().expect("flush"); } +#[allow(dead_code)] +pub(crate) fn send_escape(writer: &mut dyn Write) { + writer.write_all(&[0x1b]).expect("write escape"); + writer.flush().expect("flush"); +} + #[allow(dead_code)] pub(crate) fn send_alt_enter(writer: &mut dyn Write) { writer.write_all(&[0x1b, 0x0d]).expect("write alt-enter"); diff --git a/src/combo/runner.rs b/src/combo/runner.rs index a406785..4fb7270 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -4,7 +4,10 @@ //! Combos are executable scripts that can perform complex operations //! with recorded tool calls. -use std::{path::PathBuf, sync::Arc}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; use serde_json::{Value, json}; use tokio::sync::Mutex; @@ -1183,20 +1186,9 @@ async fn handle_interactive_combo_reply( } let mut tool_results = Vec::new(); - let mut completed = false; let mut guidance: Option = None; for tool_use in tool_uses { - emit_combo_event( - on_event, - ComboEvent::ReplyToolUse { - name: combo_name.to_string(), - tool_use: tool_use.clone(), - thinking: Vec::new(), - offload: true, - }, - ); - if tool_use.name == BASH_TOOL_NAME { let bash_input: BashInput = serde_json::from_value(tool_use.input.clone()) .map_err(|err| ComboReplyError::InvalidBashInput { @@ -1204,6 +1196,34 @@ async fn handle_interactive_combo_reply( })?; let command = bash_input.command.clone(); let command_kind = classify_offload_command(&command); + let mut emitted_tool_use = tool_use.clone(); + if matches!(command_kind, OffloadCommandKind::Coco) { + let mut bash_input_for_emit = bash_input.clone(); + bash_input_for_emit.command = + command_with_session_socket(&bash_input.command, &session_socket_path); + emitted_tool_use.input = + serde_json::to_value(&bash_input_for_emit).map_err(|err| { + ComboReplyError::InvalidBashInput { + message: err.to_string(), + } + })?; + } + emit_combo_event( + on_event, + ComboEvent::ReplyToolUse { + name: combo_name.to_string(), + tool_use: emitted_tool_use, + thinking: Vec::new(), + offload: true, + }, + ); + + if matches!(command_kind, OffloadCommandKind::Coco) { + // Do not auto-run coco reply in interactive ask mode. + // Wait for user confirmation and execution through the regular ToolUse flow. + return Ok(()); + } + let (output, is_error) = match command_kind { OffloadCommandKind::Unsafe => { let reason = match bash_unsafe_reason(&command) { @@ -1265,21 +1285,25 @@ async fn handle_interactive_combo_reply( )); match command_kind { - OffloadCommandKind::Coco if !is_error => { - completed = true; - break; - } - OffloadCommandKind::Coco => { - guidance = Some(build_offload_reply_guidance(schemas, &command, true)); - } OffloadCommandKind::Unsafe => { guidance = Some(build_offload_reply_guidance(schemas, &command, false)); } OffloadCommandKind::Safe => {} + OffloadCommandKind::Coco => unreachable!("coco reply returns early"), } continue; } + emit_combo_event( + on_event, + ComboEvent::ReplyToolUse { + name: combo_name.to_string(), + tool_use: tool_use.clone(), + thinking: Vec::new(), + offload: true, + }, + ); + let (output, is_error) = execute_interactive_tool_use(agent, &tool_use, cancel_token.clone()).await; emit_combo_event( @@ -1304,10 +1328,6 @@ async fn handle_interactive_combo_reply( .await; } - if completed { - return Ok(()); - } - let prompt = guidance.unwrap_or_else(|| build_interactive_offload_reply_reminder(schemas)); agent .append_message(Message::user(Content::Text(prompt))) @@ -1446,6 +1466,19 @@ fn is_coco_reply_command(command: &str) -> bool { matches!(summary.args.first(), Some(arg) if arg == "reply") } +fn shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\"'\"'")) +} + +fn command_with_session_socket(command: &str, session_socket_path: &Path) -> String { + let socket = session_socket_path.to_string_lossy(); + format!( + "COCO_SESSION_SOCK={} {}", + shell_single_quote(&socket), + command + ) +} + fn is_safe_command(command: &str) -> bool { let trimmed = command.trim(); !trimmed.is_empty() && bash_unsafe_ranges(command).is_empty() From 97dc1927b7d222cc0cd0b1e7feddda9d39cf055f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 17:23:27 +0800 Subject: [PATCH 10/20] fix: wire combo ask -i approval to correct session socket --- crates/coco-tui/src/components/chat.rs | 9 +- .../coco-tui/src/components/messages/combo.rs | 3 + .../src/e2e_tests/combo_interactive.rs | 37 +++++- src/agent/executor.rs | 44 +++++++- src/combo/runner.rs | 24 +--- src/tools/bash.rs | 105 +++++++++++++++++- 6 files changed, 194 insertions(+), 28 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index 59424ed..7739b84 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -574,11 +574,14 @@ impl Chat<'static> { } => { self.combo_offload_tool_use_to_combo_id .insert(tool_use.id.clone(), id.clone()); + if let Some(idx) = self.messages.locate_tool_message(id) { + self.update_focus(Focus::Messages); + self.messages.focus(idx); + } + self.notify_action_required("Tool permission requested"); self.set_processing(); } - ComboEvent::Executed { id, starter, .. } => { - self.combo_offload_tool_use_to_combo_id - .retain(|_, combo_id| combo_id != id); + ComboEvent::Executed { starter, .. } => { self.set_combo_thinking_active(false); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index 9320a7b..a038e8d 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -325,6 +325,9 @@ impl Combo { self.messages.push(Message::bot(Tool::new(tool_use).into())); let ask = Event::Ask(AskEvent::ToolUsePermission(tool_use_id)); let _ = self.messages.on_tool_event(&ask); + if self.messages.select_first_actionable() { + self.is_child_focused = true; + } } fn forward_output_to_child(&mut self, tool_use_id: &str, chunk: &OutputChunk) -> bool { diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index e284892..d8b6d1f 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -4,8 +4,8 @@ use vt100::Parser; use super::support::{ KillOnDrop, MockOpenAiScenario, MockOpenAiServer, assert_screen_not_contains, - create_mock_e2e_config, send_alt_enter, send_text, shutdown_tui, spawn_reader, spawn_tui, - wait_for_screen_contains, + create_mock_e2e_config, send_alt_enter, send_enter, send_text, shutdown_tui, spawn_reader, + spawn_tui, wait_for_screen_contains, }; const COMBO_NAME: &str = "e2e_mock_interactive"; @@ -219,6 +219,12 @@ fn combo_interactive_reply_tool_use_command_contains_required_fields() { "--reason='used user feedback'", Duration::from_secs(30), ); + assert_screen_not_contains( + &mut parser, + &rx, + "COCO_SESSION_SOCK=", + Duration::from_secs(2), + ); assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); } @@ -280,3 +286,30 @@ fn combo_interactive_auto_accept_edits_still_requires_bash_confirmation() { assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); } + +#[test] +fn combo_interactive_approval_executes_coco_reply_and_completes() { + let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::ImmediateReply); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + send_enter(&mut *writer); + wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(30)); + assert_screen_not_contains( + &mut parser, + &rx, + "failed to read COCO_SESSION_SOCK", + Duration::from_secs(2), + ); + child + .clone_killer() + .kill() + .expect("kill tui after assertion"); + let _ = child.try_wait(); + guard.disarm(); +} diff --git a/src/agent/executor.rs b/src/agent/executor.rs index e24d093..61a517c 100644 --- a/src/agent/executor.rs +++ b/src/agent/executor.rs @@ -468,13 +468,18 @@ impl Executor { #[cfg(test)] mod tests { use super::*; - use crate::tools::StrReplaceInput; + use crate::tools::{BashOutput, StrReplaceInput}; use std::collections::HashSet; + use std::sync::OnceLock; + use tokio::sync::Mutex; + + static ENV_LOCK: OnceLock> = OnceLock::new(); fn bash_input_value(command: &str) -> serde_json::Value { serde_json::to_value(BashInput { command: command.to_string(), timeout: 1_000, + env: std::collections::BTreeMap::new(), }) .expect("serialize BashInput") } @@ -559,6 +564,43 @@ mod tests { assert_eq!(updated, "world\n"); } + #[tokio::test] + async fn bash_execution_receives_session_socket_env() { + let _guard = ENV_LOCK.get_or_init(|| Mutex::new(())).lock().await; + let previous = std::env::var_os("COCO_SESSION_SOCK"); + // Safety: test-only mutation guarded by ENV_LOCK and restored before return. + unsafe { + std::env::set_var("COCO_SESSION_SOCK", "/tmp/coco-executor.sock"); + } + + let mut executor = Executor::default(); + let input_value = bash_input_value(r#"printf "%s" "$COCO_SESSION_SOCK""#); + executor.update_pcl( + BASH_TOOL_NAME, + PermissionControl::Once("inject-test".to_string()), + ); + let output = executor + .execute("inject-test", BASH_TOOL_NAME, Input::Starter(input_value)) + .await + .expect("execute bash with injected env"); + + // Safety: test-only mutation guarded by ENV_LOCK. + unsafe { + if let Some(value) = previous { + std::env::set_var("COCO_SESSION_SOCK", value); + } else { + std::env::remove_var("COCO_SESSION_SOCK"); + } + } + + let Output::Success(Final::Json(value)) = output else { + panic!("expected successful bash output"); + }; + let output: BashOutput = + serde_json::from_value(value).expect("deserialize bash output json"); + assert_eq!(output.stdout, "/tmp/coco-executor.sock\n"); + } + #[test] fn allowlist_none_keeps_default_tools() { let mut executor = Executor::default(); diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 4fb7270..37ccf2a 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -4,10 +4,7 @@ //! Combos are executable scripts that can perform complex operations //! with recorded tool calls. -use std::{ - path::{Path, PathBuf}, - sync::Arc, -}; +use std::{path::PathBuf, sync::Arc}; use serde_json::{Value, json}; use tokio::sync::Mutex; @@ -1199,8 +1196,10 @@ async fn handle_interactive_combo_reply( let mut emitted_tool_use = tool_use.clone(); if matches!(command_kind, OffloadCommandKind::Coco) { let mut bash_input_for_emit = bash_input.clone(); - bash_input_for_emit.command = - command_with_session_socket(&bash_input.command, &session_socket_path); + bash_input_for_emit.env.insert( + "COCO_SESSION_SOCK".to_string(), + session_socket_path.to_string_lossy().to_string(), + ); emitted_tool_use.input = serde_json::to_value(&bash_input_for_emit).map_err(|err| { ComboReplyError::InvalidBashInput { @@ -1466,19 +1465,6 @@ fn is_coco_reply_command(command: &str) -> bool { matches!(summary.args.first(), Some(arg) if arg == "reply") } -fn shell_single_quote(value: &str) -> String { - format!("'{}'", value.replace('\'', "'\"'\"'")) -} - -fn command_with_session_socket(command: &str, session_socket_path: &Path) -> String { - let socket = session_socket_path.to_string_lossy(); - format!( - "COCO_SESSION_SOCK={} {}", - shell_single_quote(&socket), - command - ) -} - fn is_safe_command(command: &str) -> bool { let trimmed = command.trim(); !trimmed.is_empty() && bash_unsafe_ranges(command).is_empty() diff --git a/src/tools/bash.rs b/src/tools/bash.rs index b00a804..5e7758f 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -1,4 +1,4 @@ -use std::{sync::OnceLock, time::Duration}; +use std::{collections::BTreeMap, sync::OnceLock, time::Duration}; use async_trait::async_trait; use futures_util::StreamExt; @@ -23,6 +23,8 @@ pub struct BashInput { pub command: String, #[serde(default = "default_timeout_ms")] pub timeout: u64, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub env: BTreeMap, } impl BashInput { @@ -30,6 +32,7 @@ impl BashInput { Self { command, timeout: default_timeout_ms(), + env: BTreeMap::new(), } } } @@ -58,7 +61,21 @@ pub(crate) fn extra_envs_for_bash_input(input: &Input<'_>) -> Vec<(String, Strin let Ok(parsed) = serde_json::from_value::(input.clone()) else { return Vec::new(); }; - extra_envs_for_command(&parsed.command) + let mut envs = extra_envs_for_command(&parsed.command); + for (key, value) in parsed.env { + if value.is_empty() { + continue; + } + if !matches!(key.as_str(), "COCO_SESSION_SOCK" | "COCO_TUI_BIN") { + continue; + } + if let Some(existing) = envs.iter_mut().find(|(k, _)| *k == key) { + existing.1 = value; + } else { + envs.push((key, value)); + } + } + envs } fn extra_envs_for_command(_command: &str) -> Vec<(String, String)> { @@ -139,7 +156,9 @@ async fn run_bash_chunked_raw( where F: FnMut(&OutputChunk) + Send, { - let BashInput { command, timeout } = input; + let BashInput { + command, timeout, .. + } = input; let argv = vec!["bash".to_string(), "-c".to_string(), command]; let envs = match prepare_mcp_envs().await { @@ -313,12 +332,16 @@ pub async fn prepare_mcp_envs() -> Result, String> { #[cfg(test)] mod tests { + use std::sync::{Mutex, OnceLock}; + use serde_json::json; use crate::tools::Output; use super::*; + static ENV_LOCK: OnceLock> = OnceLock::new(); + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn bash_output_split_streams() { let tool = BashTool::default(); @@ -372,4 +395,80 @@ mod tests { assert_eq!(output.exit_code, 255); assert!(output.stderr.contains("Invalid input format")); } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn bash_collects_coco_session_sock_from_env() { + let _guard = ENV_LOCK + .get_or_init(|| Mutex::new(())) + .lock() + .expect("lock env mutex"); + + let prev = std::env::var_os("COCO_SESSION_SOCK"); + // Safety: test-only mutation, serialized by ENV_LOCK and restored before return. + unsafe { + std::env::set_var("COCO_SESSION_SOCK", "/tmp/coco-test.sock"); + } + + let input = Input::Starter(json!({ + "command": "coco reply --result='ok'", + "timeout": 60_000, + })); + + let envs = extra_envs_for_bash_input(&input); + + // Restore env before assertions to avoid leaking state on panic paths below. + // Safety: test-only mutation, serialized by ENV_LOCK. + unsafe { + if let Some(value) = prev { + std::env::set_var("COCO_SESSION_SOCK", value); + } else { + std::env::remove_var("COCO_SESSION_SOCK"); + } + } + + assert!( + envs.iter() + .any(|(k, v)| k == "COCO_SESSION_SOCK" && v == "/tmp/coco-test.sock"), + "COCO_SESSION_SOCK is missing from bash extra envs: {envs:?}" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn bash_input_env_overrides_process_session_sock() { + let _guard = ENV_LOCK + .get_or_init(|| Mutex::new(())) + .lock() + .expect("lock env mutex"); + + let prev = std::env::var_os("COCO_SESSION_SOCK"); + // Safety: test-only mutation, serialized by ENV_LOCK and restored before return. + unsafe { + std::env::set_var("COCO_SESSION_SOCK", "/tmp/coco-process.sock"); + } + + let input = Input::Starter(json!({ + "command": "coco reply --result='ok'", + "timeout": 60_000, + "env": { + "COCO_SESSION_SOCK": "/tmp/coco-tool.sock" + } + })); + + let envs = extra_envs_for_bash_input(&input); + + // Safety: test-only mutation, serialized by ENV_LOCK. + unsafe { + if let Some(value) = prev { + std::env::set_var("COCO_SESSION_SOCK", value); + } else { + std::env::remove_var("COCO_SESSION_SOCK"); + } + } + + assert!( + envs.iter() + .any(|(k, v)| k == "COCO_SESSION_SOCK" && v == "/tmp/coco-tool.sock"), + "expected tool-level COCO_SESSION_SOCK override in envs: {envs:?}" + ); + } } From 78e9c7b04d3eef0a431224d2eef9924efc4f52f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 17:43:45 +0800 Subject: [PATCH 11/20] fix: require combo reply confirmation only in ask -i --- crates/coco-tui/src/components/chat.rs | 19 ++-- .../coco-tui/src/components/messages/combo.rs | 15 +-- .../src/e2e_tests/combo_interactive.rs | 61 +++++++++++-- crates/coco-tui/src/events.rs | 2 + src/combo/runner.rs | 91 +++++++++++-------- src/combo/session.rs | 2 + src/combo/types.rs | 2 + 7 files changed, 134 insertions(+), 58 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index 7739b84..8868f7d 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -570,15 +570,18 @@ impl Chat<'static> { id, tool_use, offload: true, + requires_confirmation, .. } => { - self.combo_offload_tool_use_to_combo_id - .insert(tool_use.id.clone(), id.clone()); - if let Some(idx) = self.messages.locate_tool_message(id) { - self.update_focus(Focus::Messages); - self.messages.focus(idx); + if *requires_confirmation { + self.combo_offload_tool_use_to_combo_id + .insert(tool_use.id.clone(), id.clone()); + if let Some(idx) = self.messages.locate_tool_message(id) { + self.update_focus(Focus::Messages); + self.messages.focus(idx); + } + self.notify_action_required("Tool permission requested"); } - self.notify_action_required("Tool permission requested"); self.set_processing(); } ComboEvent::Executed { starter, .. } => { @@ -840,12 +843,14 @@ impl Chat<'static> { tool_use, thinking, offload, + requires_confirmation, } => ComboEvent::ReplyToolUse { id: id.to_string(), name: name.clone(), tool_use: tool_use.clone(), thinking: thinking.clone(), offload: *offload, + requires_confirmation: *requires_confirmation, }, ComboToolEvent::ReplyToolResult { name, @@ -1001,12 +1006,14 @@ impl Chat<'static> { tool_use, thinking, offload, + requires_confirmation, } => ComboRunEvent::ReplyToolUse { id: id.clone(), name: name.clone(), tool_use: tool_use.clone(), thinking: thinking.clone(), offload: *offload, + requires_confirmation: *requires_confirmation, }, ComboEvent::ReplyToolResult { id, diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index a038e8d..064ce4d 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -319,14 +319,16 @@ impl Combo { true } - fn push_offload_bash_tool_use(&mut self, tool_use: ToolUse) { + fn push_offload_bash_tool_use(&mut self, tool_use: ToolUse, requires_confirmation: bool) { self.state.write().view = ComboView::Messages; let tool_use_id = tool_use.id.clone(); self.messages.push(Message::bot(Tool::new(tool_use).into())); - let ask = Event::Ask(AskEvent::ToolUsePermission(tool_use_id)); - let _ = self.messages.on_tool_event(&ask); - if self.messages.select_first_actionable() { - self.is_child_focused = true; + if requires_confirmation { + let ask = Event::Ask(AskEvent::ToolUsePermission(tool_use_id)); + let _ = self.messages.on_tool_event(&ask); + if self.messages.select_first_actionable() { + self.is_child_focused = true; + } } } @@ -519,6 +521,7 @@ impl Combo { tool_use, thinking, offload, + requires_confirmation, .. } => { if self.matches_id(id) { @@ -528,7 +531,7 @@ impl Combo { self.push_prompt_thinking(block); } if *offload { - self.push_offload_bash_tool_use(tool_use.clone()); + self.push_offload_bash_tool_use(tool_use.clone(), *requires_confirmation); } else { self.push_prompt_reply(tool_use); } diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index d8b6d1f..f5b43f6 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -9,6 +9,7 @@ use super::support::{ }; const COMBO_NAME: &str = "e2e_mock_interactive"; +const COMBO_NAME_NON_INTERACTIVE: &str = "e2e_mock_non_interactive"; type ComboHarness = ( KillOnDrop, Box, @@ -33,6 +34,21 @@ echo "$response" ) } +fn mock_combo_script_non_interactive() -> String { + format!( + r#"#!/usr/bin/env bash +set -euo pipefail + +input="${{*:-Long time no see}}" + +coco metadata name={COMBO_NAME_NON_INTERACTIVE} description="mock non-interactive combo" || exit 0 + +response="$(coco ask --schemas result:result --schemas reason:reason "Process input: ${{input}}")" +echo "$response" +"# + ) +} + fn run_combo_with_mock(mock: &MockOpenAiServer) -> ComboHarness { run_combo_with_mock_and_auto_accept(mock, false) } @@ -41,16 +57,29 @@ fn run_combo_with_mock_and_auto_accept( mock: &MockOpenAiServer, auto_accept_edits: bool, ) -> ComboHarness { - let temp = create_mock_e2e_config( - mock.base_url(), - auto_accept_edits, - COMBO_NAME, - &mock_combo_script(), - ); + run_combo_with_script(mock, auto_accept_edits, COMBO_NAME, &mock_combo_script()) +} + +fn run_combo_non_interactive_with_mock(mock: &MockOpenAiServer) -> ComboHarness { + run_combo_with_script( + mock, + false, + COMBO_NAME_NON_INTERACTIVE, + &mock_combo_script_non_interactive(), + ) +} + +fn run_combo_with_script( + mock: &MockOpenAiServer, + auto_accept_edits: bool, + combo_name: &str, + combo_script: &str, +) -> ComboHarness { + let temp = create_mock_e2e_config(mock.base_url(), auto_accept_edits, combo_name, combo_script); let config_dir = temp.path().join("coco"); let (child, reader, writer) = spawn_tui( Some(&config_dir), - &["combo", "run", COMBO_NAME, "Long time no see"], + &["combo", "run", combo_name, "Long time no see"], ); let guard = KillOnDrop::new(child.clone_killer()); let rx = spawn_reader(reader); @@ -313,3 +342,21 @@ fn combo_interactive_approval_executes_coco_reply_and_completes() { let _ = child.try_wait(); guard.disarm(); } + +#[test] +fn combo_non_interactive_executes_reply_without_confirmation() { + let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::ImmediateReply); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = + run_combo_non_interactive_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Combo: e2e_mock_non_interactive", + Duration::from_secs(30), + ); + // Non-interactive ask should finish without any user approval input. + wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(10)); + + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); +} diff --git a/crates/coco-tui/src/events.rs b/crates/coco-tui/src/events.rs index 081f28b..009ca2a 100644 --- a/crates/coco-tui/src/events.rs +++ b/crates/coco-tui/src/events.rs @@ -169,6 +169,8 @@ pub enum ComboEvent { thinking: Vec, /// Whether this is an offload reply (executed via bash) offload: bool, + /// Whether this offload tool use requires explicit user confirmation. + requires_confirmation: bool, }, /// Result of offload reply bash execution ReplyToolResult { diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 37ccf2a..ec75a14 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -492,6 +492,7 @@ async fn execute_combo( tool_use: reply.tool_use.clone(), thinking: thinking_blocks, offload: false, + requires_confirmation: false, }, ); } @@ -1214,6 +1215,7 @@ async fn handle_interactive_combo_reply( tool_use: emitted_tool_use, thinking: Vec::new(), offload: true, + requires_confirmation: matches!(command_kind, OffloadCommandKind::Coco), }, ); @@ -1300,6 +1302,7 @@ async fn handle_interactive_combo_reply( tool_use: tool_use.clone(), thinking: Vec::new(), offload: true, + requires_confirmation: false, }, ); @@ -1528,45 +1531,54 @@ async fn handle_offload_combo_reply( .append_message(Message::user(Content::Text(directive.to_string()))) .await; - let on_event_stream = on_event.clone(); - let stream_name = combo_name.to_string(); - let chat_response = agent - .chat_stream_with_history(cancel_token.clone(), move |update| match update { - ChatStreamUpdate::Reset => { - emit_combo_event( - &on_event_stream, - ComboEvent::PromptStreamReset { - name: stream_name.clone(), - }, - ); - } - ChatStreamUpdate::Plain { index, text } => { - emit_combo_event( - &on_event_stream, - ComboEvent::PromptStream { - name: stream_name.clone(), - index, - kind: ComboEventStreamKind::Plain, - text, - }, - ); - } - ChatStreamUpdate::Thinking { index, text } => { - emit_combo_event( - &on_event_stream, - ComboEvent::PromptStream { - name: stream_name.clone(), - index, - kind: ComboEventStreamKind::Thinking, - text, - }, - ); - } - }) - .await - .map_err(|e| ComboReplyError::ChatFailed { - message: e.to_string(), - })?; + let chat_response = if agent.disable_stream_for_current_model() { + agent + .chat_with_history() + .await + .map_err(|e| ComboReplyError::ChatFailed { + message: e.to_string(), + })? + } else { + let on_event_stream = on_event.clone(); + let stream_name = combo_name.to_string(); + agent + .chat_stream_with_history(cancel_token.clone(), move |update| match update { + ChatStreamUpdate::Reset => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStreamReset { + name: stream_name.clone(), + }, + ); + } + ChatStreamUpdate::Plain { index, text } => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStream { + name: stream_name.clone(), + index, + kind: ComboEventStreamKind::Plain, + text, + }, + ); + } + ChatStreamUpdate::Thinking { index, text } => { + emit_combo_event( + &on_event_stream, + ComboEvent::PromptStream { + name: stream_name.clone(), + index, + kind: ComboEventStreamKind::Thinking, + text, + }, + ); + } + }) + .await + .map_err(|e| ComboReplyError::ChatFailed { + message: e.to_string(), + })? + }; let blocks = match &chat_response.message.content { Content::Multiple(blocks) => blocks.as_slice(), @@ -1602,6 +1614,7 @@ async fn handle_offload_combo_reply( tool_use: bash_tool_use.clone(), thinking: Vec::new(), offload: true, + requires_confirmation: false, }, ); diff --git a/src/combo/session.rs b/src/combo/session.rs index 8a92453..4d26fdd 100644 --- a/src/combo/session.rs +++ b/src/combo/session.rs @@ -230,6 +230,8 @@ pub enum ComboRunEvent { tool_use: ToolUse, thinking: Vec, offload: bool, + #[serde(default)] + requires_confirmation: bool, }, ReplyToolResult { id: String, diff --git a/src/combo/types.rs b/src/combo/types.rs index 4465026..a586b1f 100644 --- a/src/combo/types.rs +++ b/src/combo/types.rs @@ -126,6 +126,8 @@ pub enum ComboEvent { thinking: Vec, /// Whether this is an offload reply (executed via bash). offload: bool, + /// Whether this tool use requires explicit user confirmation in UI. + requires_confirmation: bool, }, /// Reply tool result for offload. ReplyToolResult { From c6e4344ee6b48e3ceca94c44ee2018cbcc73baad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 18:08:36 +0800 Subject: [PATCH 12/20] fix: cover combo reject-feedback flow in e2e --- .../coco-tui/src/components/messages/combo.rs | 14 +++++++- .../src/e2e_tests/combo_interactive.rs | 25 ++++++++++++++ crates/coco-tui/src/e2e_tests/support.rs | 33 ++++++++++++++++--- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index 064ce4d..ccca113 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -320,10 +320,19 @@ impl Combo { } fn push_offload_bash_tool_use(&mut self, tool_use: ToolUse, requires_confirmation: bool) { - self.state.write().view = ComboView::Messages; + { + let mut state = self.state.write(); + state.view = ComboView::Messages; + state.bash_tool_use = if requires_confirmation { + Some(tool_use.clone()) + } else { + None + }; + } let tool_use_id = tool_use.id.clone(); self.messages.push(Message::bot(Tool::new(tool_use).into())); if requires_confirmation { + self.state.write().starter_state = StarterState::AwaitingPermission; let ask = Event::Ask(AskEvent::ToolUsePermission(tool_use_id)); let _ = self.messages.on_tool_event(&ask); if self.messages.select_first_actionable() { @@ -546,6 +555,7 @@ impl Combo { } => { if self.matches_id(id) { self.forward_result_to_child(tool_use_id, *is_error, output.clone()); + self.state.write().bash_tool_use = None; } } ComboEvent::Executing { @@ -595,6 +605,7 @@ impl Combo { } }; state.starter_state = StarterState::Finalized; + state.bash_tool_use = None; state.display_state.expand(); drop(state); if let Some(message) = error_message { @@ -619,6 +630,7 @@ impl Combo { { let mut state = self.state.write(); state.starter_state = StarterState::Cancelled; + state.bash_tool_use = None; state.display_state.expand(); } } diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index f5b43f6..09088e3 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -343,6 +343,31 @@ fn combo_interactive_approval_executes_coco_reply_and_completes() { guard.disarm(); } +#[test] +fn combo_interactive_reject_then_feedback_allows_second_reply_attempt() { + let mock = MockOpenAiServer::start_with_scenario( + MockOpenAiScenario::RejectThenNeedFeedbackToken("E2E_RETRY_REASON".to_string()), + ); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains(&mut parser, &rx, "rm -rf /", Duration::from_secs(30)); + wait_for_screen_contains(&mut parser, &rx, "Ready", Duration::from_secs(30)); + wait_for_screen_contains( + &mut parser, + &rx, + "Please provide feedback first (missing: E2E_RETRY_REASON).", + Duration::from_secs(30), + ); + assert_screen_not_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(2), + ); + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); + assert!(mock.request_count() >= 2); +} + #[test] fn combo_non_interactive_executes_reply_without_confirmation() { let mock = MockOpenAiServer::start_with_scenario(MockOpenAiScenario::ImmediateReply); diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index 25a3262..2e81ce7 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -154,6 +154,7 @@ struct MockOpenAiState { pub(crate) enum MockOpenAiScenario { RequireFeedbackTokens(Vec), ImmediateReply, + RejectThenNeedFeedbackToken(String), RateLimited, } @@ -377,17 +378,26 @@ fn build_mock_openai_response( let is_summary_prompt = user_text.contains("Summarize the combo execution for the user."); - { + let request_count = { let mut guard = state.lock().expect("lock mock state"); guard.request_count += 1; - if let MockOpenAiScenario::RequireFeedbackTokens(tokens) = scenario { - for token in tokens { + match scenario { + MockOpenAiScenario::RequireFeedbackTokens(tokens) => { + for token in tokens { + if user_text.contains(token) { + guard.seen_tokens.insert(token.clone()); + } + } + } + MockOpenAiScenario::RejectThenNeedFeedbackToken(token) => { if user_text.contains(token) { guard.seen_tokens.insert(token.clone()); } } + MockOpenAiScenario::ImmediateReply | MockOpenAiScenario::RateLimited => {} } - } + guard.request_count + }; if is_summary_prompt { return MockHttpResponse::Ok(mock_text_response( @@ -411,6 +421,21 @@ fn build_mock_openai_response( "coco reply --result='mock polished result' --reason='used user feedback'"; MockHttpResponse::Ok(mock_bash_tool_call_response(command)) } + MockOpenAiScenario::RejectThenNeedFeedbackToken(token) => { + if user_text.contains(token) { + let command = + "coco reply --result='mock polished result' --reason='used user feedback'"; + return MockHttpResponse::Ok(mock_bash_tool_call_response(command)); + } + if request_count == 1 { + return MockHttpResponse::Ok(mock_bash_tool_call_response("rm -rf /")); + } + thread::sleep(Duration::from_millis(500)); + MockHttpResponse::Ok(mock_text_response(&format!( + "Please provide feedback first (missing: {}).", + token + ))) + } MockOpenAiScenario::RequireFeedbackTokens(tokens) => { let missing = tokens .iter() From 2bfa0456497e7f85386b06d14fbd855e1571e964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 19:08:35 +0800 Subject: [PATCH 13/20] fix: keep combo ask -i interactive feedback bound to session --- crates/coco-tui/src/components/chat.rs | 235 ++++++++++++++++-- .../coco-tui/src/components/messages/combo.rs | 2 + .../src/e2e_tests/combo_interactive.rs | 49 +++- crates/coco-tui/src/e2e_tests/support.rs | 18 ++ crates/coco-tui/src/events.rs | 1 + src/combo/runner.rs | 16 ++ src/combo/session.rs | 2 + src/combo/types.rs | 2 + 8 files changed, 306 insertions(+), 19 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index 8868f7d..c95612a 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -96,6 +96,7 @@ pub struct Chat<'a> { manual_combo_prompted: HashSet, pending_combo_tool_events: HashMap>, combo_offload_tool_use_to_combo_id: HashMap, + combo_pending_reply_session_socks: HashMap, last_usage: Option, retry_status: Option, transcript_scopes: Vec, @@ -344,6 +345,7 @@ impl Chat<'static> { manual_combo_prompted: HashSet::new(), pending_combo_tool_events: HashMap::new(), combo_offload_tool_use_to_combo_id: HashMap::new(), + combo_pending_reply_session_socks: HashMap::new(), last_usage: None, retry_status: None, transcript_scopes: Vec::new(), @@ -558,9 +560,19 @@ impl Chat<'static> { | ComboEvent::SummaryDone { .. } => { self.set_processing(); } - ComboEvent::Prompt { thinking, .. } => { + ComboEvent::Prompt { + id, + thinking, + session_sock, + .. + } => { + if let Some(session_sock) = session_sock.as_ref().filter(|value| !value.is_empty()) + { + self.combo_pending_reply_session_socks + .insert(id.clone(), session_sock.clone()); + } self.set_combo_thinking_active(thinking.as_ref().is_some_and(|cfg| cfg.enabled)); - self.set_processing(); + self.set_ready(); } ComboEvent::ReplyToolUse { offload: false, .. } => { self.set_combo_thinking_active(false); @@ -576,6 +588,10 @@ impl Chat<'static> { if *requires_confirmation { self.combo_offload_tool_use_to_combo_id .insert(tool_use.id.clone(), id.clone()); + if let Some(session_sock) = session_sock_from_tool_use(tool_use) { + self.combo_pending_reply_session_socks + .insert(id.clone(), session_sock); + } if let Some(idx) = self.messages.locate_tool_message(id) { self.update_focus(Focus::Messages); self.messages.focus(idx); @@ -584,8 +600,9 @@ impl Chat<'static> { } self.set_processing(); } - ComboEvent::Executed { starter, .. } => { + ComboEvent::Executed { id, starter, .. } => { self.set_combo_thinking_active(false); + self.combo_pending_reply_session_socks.remove(id); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); } @@ -600,6 +617,7 @@ impl Chat<'static> { ComboEvent::NotFound { id, .. } => { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); + self.combo_pending_reply_session_socks.remove(id); self.set_combo_thinking_active(false); self.set_ready(); } @@ -607,6 +625,7 @@ impl Chat<'static> { if let Some(id) = id { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); + self.combo_pending_reply_session_socks.remove(id); } self.set_combo_thinking_active(false); self.set_ready(); @@ -678,7 +697,13 @@ impl Chat<'static> { } fn spawn_chat_task(&mut self, content: ChatContent) { - let cancel_token = self.cancellation_guard.start_token(); + // Keep combo execution alive while collecting feedback for interactive `coco reply`. + // Starting a new token here would cancel the pending combo run and drop its session socket. + let cancel_token = if self.combo_pending_reply_session_socks.is_empty() { + self.cancellation_guard.start_token() + } else { + self.cancellation_guard.token() + }; tokio::task::spawn(task_chat(self.agent.clone(), content, cancel_token)); } @@ -784,11 +809,13 @@ impl Chat<'static> { name, prompt, thinking, + session_sock, } => ComboEvent::Prompt { id: id.to_string(), name: name.clone(), prompt: prompt.clone(), thinking: thinking.clone(), + session_sock: session_sock.clone(), }, ComboToolEvent::PromptStream { name, @@ -943,11 +970,13 @@ impl Chat<'static> { name, prompt, thinking, + session_sock, } => ComboRunEvent::Prompt { id: id.clone(), name: name.clone(), prompt: prompt.clone(), thinking: thinking.clone(), + session_sock: session_sock.clone(), }, ComboEvent::PromptStream { id, @@ -1104,6 +1133,7 @@ impl Chat<'static> { ComboEvent::Executed { id, starter, .. } => { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); + self.combo_pending_reply_session_socks.remove(id); self.set_combo_thinking_active(false); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); @@ -1911,6 +1941,101 @@ impl Chat<'static> { None } + fn pending_combo_id_for_session_sock(&self, session_sock: &str) -> Option { + self.combo_pending_reply_session_socks + .iter() + .find_map(|(combo_id, pending_sock)| { + if pending_sock == session_sock { + Some(combo_id.clone()) + } else { + None + } + }) + } + + fn lone_pending_combo_reply_session(&self) -> Option<(String, String)> { + if self.combo_pending_reply_session_socks.len() != 1 { + return None; + } + self.combo_pending_reply_session_socks + .iter() + .next() + .map(|(combo_id, session_sock)| (combo_id.clone(), session_sock.clone())) + } + + fn maybe_route_pending_combo_reply_tool_use( + &self, + tool_use: &ToolUse, + ) -> Option<(String, ToolUse)> { + if tool_use.name != BASH_TOOL_NAME { + return None; + } + let bash_input = serde_json::from_value::(tool_use.input.clone()).ok()?; + if !is_coco_reply_command(&bash_input.command) { + return None; + } + if let Some(session_sock) = bash_input + .env + .get("COCO_SESSION_SOCK") + .filter(|session_sock| !session_sock.is_empty()) + { + let combo_id = self.pending_combo_id_for_session_sock(session_sock)?; + return Some((combo_id, tool_use.clone())); + } + let (combo_id, session_sock) = self.lone_pending_combo_reply_session()?; + let tool_use = inject_session_sock_into_tool_use(tool_use, &session_sock)?; + Some((combo_id, tool_use)) + } + + fn patch_tool_use_with_pending_combo_session_sock(&self, tool_use: &ToolUse) -> ToolUse { + if tool_use.name != BASH_TOOL_NAME { + return tool_use.clone(); + } + let Ok(input) = serde_json::from_value::(tool_use.input.clone()) else { + return tool_use.clone(); + }; + if !is_coco_reply_command(&input.command) { + return tool_use.clone(); + } + if session_sock_from_tool_use(tool_use).is_some() { + return tool_use.clone(); + } + + let session_sock = + if let Some(combo_id) = self.combo_offload_tool_use_to_combo_id.get(&tool_use.id) { + self.combo_pending_reply_session_socks + .get(combo_id) + .cloned() + } else { + self.lone_pending_combo_reply_session() + .map(|(_, session_sock)| session_sock) + }; + if let Some(session_sock) = session_sock + && let Some(patched) = inject_session_sock_into_tool_use(tool_use, &session_sock) + { + return patched; + } + tool_use.clone() + } + + fn push_combo_offload_reply_tool_use(&mut self, combo_id: String, tool_use: ToolUse) { + let combo_name = self + .combo_name_from_transcript(&combo_id) + .unwrap_or_else(|| combo_id.clone()); + let combo_event = ComboEvent::ReplyToolUse { + id: combo_id, + name: combo_name, + tool_use, + thinking: Vec::new(), + offload: true, + requires_confirmation: true, + }; + self.handle_combo_event(&combo_event); + let wrapped = Event::Combo(combo_event); + let wrapped_ref = &wrapped; + handle_component_event!(self, wrapped_ref); + } + fn ensure_transcript_focus(&mut self) { if self.transcript.selected_idx().is_none() && !self.transcript.is_empty() { self.transcript.focus(0); @@ -2235,10 +2360,18 @@ impl Component for Chat<'static> { let mut new_messages = Vec::with_capacity(msgs.len()); let mut combo_tool_ids = Vec::new(); for msg in msgs.iter().cloned() { - let message = match msg { - BotMessage::Plain(text) => Message::bot(Plain::new(text).into()), + match msg { + BotMessage::Plain(text) => { + new_messages.push(Message::bot(Plain::new(text).into())) + } BotMessage::ToolUse(tool_use) => { - if tool_use.name == RUN_COMBO_TOOL_NAME { + if let Some((combo_id, patched_tool_use)) = + self.maybe_route_pending_combo_reply_tool_use(&tool_use) + { + self.push_combo_offload_reply_tool_use(combo_id, patched_tool_use); + continue; + } + let message = if tool_use.name == RUN_COMBO_TOOL_NAME { if let Ok(input) = serde_json::from_value::(tool_use.input.clone()) { @@ -2265,14 +2398,16 @@ impl Component for Chat<'static> { } } else { Message::bot(Tool::new(tool_use.to_owned()).into()) - } + }; + new_messages.push(message); + } + BotMessage::System(message) => { + new_messages.push(Message::system(Plain::new(message).into())) } - BotMessage::System(message) => Message::system(Plain::new(message).into()), BotMessage::Thinking(thinking) => { - Message::bot(Thinking::new(thinking).into()) + new_messages.push(Message::bot(Thinking::new(thinking).into())) } - }; - new_messages.push(message); + } } self.messages.extend(new_messages.into_iter()); for id in combo_tool_ids { @@ -2811,20 +2946,33 @@ impl Component for Chat<'static> { }, Action::Tool(action) => match action { ToolAction::Grant(tool_use) => { - self.agent.grant_once(&tool_use.id, &tool_use.name); - self.spawn_tool_use(tool_use); + let patched_tool_use = + self.patch_tool_use_with_pending_combo_session_sock(tool_use); + self.agent + .grant_once(&patched_tool_use.id, &patched_tool_use.name); + self.spawn_tool_use(&patched_tool_use); } ToolAction::GrantSession(tool_use) => { - self.agent.grant_session(tool_use); - self.spawn_tool_use(tool_use); + let patched_tool_use = + self.patch_tool_use_with_pending_combo_session_sock(tool_use); + self.agent.grant_session(&patched_tool_use); + self.spawn_tool_use(&patched_tool_use); } ToolAction::Cancel(tool_use) => { + let cancelled_combo_id = + self.combo_offload_tool_use_to_combo_id.remove(&tool_use.id); + let cancelled_combo_offload = cancelled_combo_id.is_some(); // Move focus back to Input when tool use is cancelled. if let Some(idx) = self.messages.locate_tool_message(&tool_use.id) && self.messages.selected_idx() == Some(idx) { self.update_focus(Focus::Input); self.messages.blur(); + } else if cancelled_combo_offload { + // Combo offload reply tool use is nested in Combo content, so it is not + // discoverable via locate_tool_message. Restore input focus explicitly. + self.update_focus(Focus::Input); + self.messages.blur(); } // Await the next user message to avoid the LLM reacting without further user // instructions @@ -3103,10 +3251,61 @@ fn is_combo_command_token(token: &str) -> bool { .is_some_and(|name| name == "coco") } +fn is_coco_reply_command(command: &str) -> bool { + let tokens: Vec<&str> = command.split_whitespace().collect(); + for (idx, token) in tokens.iter().enumerate() { + if is_combo_command_token(token) && tokens.get(idx + 1) == Some(&"reply") { + return true; + } + } + false +} + +fn is_coco_reply_tool_use(tool_use: &ToolUse) -> bool { + if tool_use.name != BASH_TOOL_NAME { + return false; + } + let Ok(input) = serde_json::from_value::(tool_use.input.clone()) else { + return false; + }; + is_coco_reply_command(&input.command) +} + fn trim_combo_token(token: &str) -> String { token.trim_matches('"').trim_matches('\'').to_string() } +fn session_sock_from_tool_use(tool_use: &ToolUse) -> Option { + if tool_use.name != BASH_TOOL_NAME { + return None; + } + let input = serde_json::from_value::(tool_use.input.clone()).ok()?; + input + .env + .get("COCO_SESSION_SOCK") + .filter(|value| !value.is_empty()) + .cloned() +} + +fn inject_session_sock_into_tool_use(tool_use: &ToolUse, session_sock: &str) -> Option { + if tool_use.name != BASH_TOOL_NAME { + return None; + } + let mut input = serde_json::from_value::(tool_use.input.clone()).ok()?; + let has_session_sock = input + .env + .get("COCO_SESSION_SOCK") + .is_some_and(|value| !value.is_empty()); + if !has_session_sock { + input + .env + .insert("COCO_SESSION_SOCK".to_string(), session_sock.to_string()); + } + let mut patched = tool_use.clone(); + patched.input = serde_json::to_value(&input).ok()?; + Some(patched) +} + fn session_summary_prompt() -> String { format!( "Write a short session summary for a session switcher list. Return a single line (max {SESSION_SUMMARY_MAX_LEN} chars), plain text only, no quotes. Focus on the user's goal and progress. If there is no meaningful content, return an empty string." @@ -3504,7 +3703,9 @@ async fn handle_chat_response( } ChatContent::Multiple(blocks) => { to_execute.extend(blocks.iter().filter_map(|b| { - if let code_combo::Block::ToolUse(tool_use) = b { + if let code_combo::Block::ToolUse(tool_use) = b + && !is_coco_reply_tool_use(tool_use) + { Some(tool_use.clone()) } else { None diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index ccca113..64d6cac 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -1405,6 +1405,7 @@ mod tests { name: TEST_NAME.to_string(), prompt: "line1".to_string(), thinking: None, + session_sock: None, })); combo.handle_event(&Event::Combo(ComboEvent::Executed { id: TEST_ID.to_string(), @@ -1453,6 +1454,7 @@ mod tests { name: TEST_NAME.to_string(), prompt: "line1".to_string(), thinking: None, + session_sock: None, })); combo.handle_event(&Event::Combo(ComboEvent::Executed { id: TEST_ID.to_string(), diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index 09088e3..532167d 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -4,8 +4,8 @@ use vt100::Parser; use super::support::{ KillOnDrop, MockOpenAiScenario, MockOpenAiServer, assert_screen_not_contains, - create_mock_e2e_config, send_alt_enter, send_enter, send_text, shutdown_tui, spawn_reader, - spawn_tui, wait_for_screen_contains, + create_mock_e2e_config, send_alt_enter, send_enter, send_escape, send_text, shutdown_tui, + spawn_reader, spawn_tui, wait_for_screen_contains, }; const COMBO_NAME: &str = "e2e_mock_interactive"; @@ -343,6 +343,51 @@ fn combo_interactive_approval_executes_coco_reply_and_completes() { guard.disarm(); } +#[test] +fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { + const FEEDBACK_TOKEN: &str = "E2E_REJECT_FEEDBACK"; + let mock = MockOpenAiServer::start_with_scenario( + MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(FEEDBACK_TOKEN.to_string()), + ); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + send_escape(&mut *writer); + wait_for_screen_contains(&mut parser, &rx, "Cancelled", Duration::from_secs(30)); + + // Ensure focus returns to input before submitting feedback. + send_escape(&mut *writer); + send_text(&mut *writer, "i"); + send_text(&mut *writer, "retry with feedback E2E_REJECT_FEEDBACK"); + send_alt_enter(&mut *writer); + + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + send_enter(&mut *writer); + wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(30)); + + assert!( + mock.saw_token(FEEDBACK_TOKEN), + "expected feedback token in model request after cancellation" + ); + + child + .clone_killer() + .kill() + .expect("kill tui after assertion"); + let _ = child.try_wait(); + guard.disarm(); +} + #[test] fn combo_interactive_reject_then_feedback_allows_second_reply_attempt() { let mock = MockOpenAiServer::start_with_scenario( diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index 2e81ce7..c5aa571 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -154,6 +154,7 @@ struct MockOpenAiState { pub(crate) enum MockOpenAiScenario { RequireFeedbackTokens(Vec), ImmediateReply, + ImmediateReplyThenRequireFeedbackToken(String), RejectThenNeedFeedbackToken(String), RateLimited, } @@ -394,6 +395,11 @@ fn build_mock_openai_response( guard.seen_tokens.insert(token.clone()); } } + MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(token) => { + if user_text.contains(token) { + guard.seen_tokens.insert(token.clone()); + } + } MockOpenAiScenario::ImmediateReply | MockOpenAiScenario::RateLimited => {} } guard.request_count @@ -421,6 +427,18 @@ fn build_mock_openai_response( "coco reply --result='mock polished result' --reason='used user feedback'"; MockHttpResponse::Ok(mock_bash_tool_call_response(command)) } + MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(token) => { + if request_count == 1 || user_text.contains(token) { + let command = + "coco reply --result='mock polished result' --reason='used user feedback'"; + return MockHttpResponse::Ok(mock_bash_tool_call_response(command)); + } + thread::sleep(Duration::from_millis(120)); + MockHttpResponse::Ok(mock_text_response(&format!( + "Please provide feedback first (missing: {}).", + token + ))) + } MockOpenAiScenario::RejectThenNeedFeedbackToken(token) => { if user_text.contains(token) { let command = diff --git a/crates/coco-tui/src/events.rs b/crates/coco-tui/src/events.rs index 009ca2a..579e6a9 100644 --- a/crates/coco-tui/src/events.rs +++ b/crates/coco-tui/src/events.rs @@ -132,6 +132,7 @@ pub enum ComboEvent { name: String, prompt: String, thinking: Option, + session_sock: Option, }, PromptStream { id: String, diff --git a/src/combo/runner.rs b/src/combo/runner.rs index ec75a14..7256912 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -344,6 +344,7 @@ async fn execute_combo( name: combo_name.clone(), prompt: prompt.clone(), thinking: None, + session_sock: None, }); summary_parts.push(format!("[Prompt] {}", prompt)); } @@ -361,6 +362,7 @@ async fn execute_combo( name: combo_name.clone(), prompt: prompt.clone(), thinking: thinking.clone(), + session_sock: Some(session_socket_path.to_string_lossy().to_string()), }); summary_parts.push(format!("[Prompt] {}", prompt)); @@ -1175,6 +1177,20 @@ async fn handle_interactive_combo_reply( let tool_uses = extract_tool_uses(&response.message); if tool_uses.is_empty() { + if disable_stream { + let response_text = extract_text_response(&response.message); + if !response_text.trim().is_empty() { + emit_combo_event( + on_event, + ComboEvent::Prompt { + name: combo_name.to_string(), + prompt: response_text, + thinking: None, + session_sock: Some(session_socket_path.to_string_lossy().to_string()), + }, + ); + } + } agent .append_message(Message::user(Content::Text( build_interactive_offload_reply_reminder(schemas), diff --git a/src/combo/session.rs b/src/combo/session.rs index 4d26fdd..26b7c49 100644 --- a/src/combo/session.rs +++ b/src/combo/session.rs @@ -194,6 +194,8 @@ pub enum ComboRunEvent { name: String, prompt: String, thinking: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + session_sock: Option, }, PromptStream { id: String, diff --git a/src/combo/types.rs b/src/combo/types.rs index a586b1f..4549e7f 100644 --- a/src/combo/types.rs +++ b/src/combo/types.rs @@ -74,6 +74,8 @@ pub enum ComboEvent { prompt: String, /// Optional thinking config. thinking: Option, + /// Session socket path for interactive reply handoff. + session_sock: Option, }, /// Prompt stream update for combo reply. PromptStream { From 19ac5fc1a3ff39f804912596f124cf304eb251a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 19:30:04 +0800 Subject: [PATCH 14/20] fix: keep combo alive after rejecting ask -i reply --- crates/coco-tui/src/components/messages/combo.rs | 4 +++- crates/coco-tui/src/e2e_tests/combo_interactive.rs | 10 +++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index 64d6cac..c8d76e8 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -1141,7 +1141,9 @@ impl Component for Combo { .unwrap(); } (KeyModifiers::NONE, KeyCode::Esc) => { - self.state.write().starter_state = StarterState::Cancelled; + // Reject only current reply tool use; combo should stay alive for feedback. + self.state.write().starter_state = StarterState::Executing; + self.state.write().bash_tool_use = None; global::action_tx() .send(ToolAction::Cancel(tool_use).into()) .unwrap(); diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index 532167d..ea9f6e0 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -359,10 +359,14 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { ); send_escape(&mut *writer); wait_for_screen_contains(&mut parser, &rx, "Cancelled", Duration::from_secs(30)); + assert_screen_not_contains( + &mut parser, + &rx, + "Combo execution was cancelled", + Duration::from_secs(2), + ); - // Ensure focus returns to input before submitting feedback. - send_escape(&mut *writer); - send_text(&mut *writer, "i"); + // Submit feedback directly after cancellation. send_text(&mut *writer, "retry with feedback E2E_REJECT_FEEDBACK"); send_alt_enter(&mut *writer); From 9d4753149b30a055a11c77b88079179c51d38509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 19:33:40 +0800 Subject: [PATCH 15/20] test: print full screen snapshots in combo interactive reject-feedback e2e --- .../src/e2e_tests/combo_interactive.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index ea9f6e0..8c10477 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -359,6 +359,10 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { ); send_escape(&mut *writer); wait_for_screen_contains(&mut parser, &rx, "Cancelled", Duration::from_secs(30)); + println!( + "=== screen after reject ===\n{}\n=== end ===", + parser.screen().contents() + ); assert_screen_not_contains( &mut parser, &rx, @@ -369,6 +373,16 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { // Submit feedback directly after cancellation. send_text(&mut *writer, "retry with feedback E2E_REJECT_FEEDBACK"); send_alt_enter(&mut *writer); + wait_for_screen_contains( + &mut parser, + &rx, + "retry with feedback E2E_REJECT_FEEDBACK", + Duration::from_secs(30), + ); + println!( + "=== screen after feedback ===\n{}\n=== end ===", + parser.screen().contents() + ); wait_for_screen_contains( &mut parser, @@ -378,6 +392,10 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { ); send_enter(&mut *writer); wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(30)); + println!( + "=== screen after complete ===\n{}\n=== end ===", + parser.screen().contents() + ); assert!( mock.saw_token(FEEDBACK_TOKEN), From f15cc8cfea123ac9def474e2d3610e5a12311f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 23:02:40 +0800 Subject: [PATCH 16/20] feat: support combo ask -i feedback loop and isolated e2e history --- crates/coco-tui/src/components/chat.rs | 187 +++++++++++++++++- .../src/e2e_tests/combo_interactive.rs | 57 ++++-- crates/coco-tui/src/e2e_tests/support.rs | 140 +++++++++++-- src/combo/runner.rs | 67 ++++++- 4 files changed, 403 insertions(+), 48 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index c95612a..3b3947e 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -7,9 +7,9 @@ use code_combo::tools::{ use code_combo::{ Agent, Block as ChatBlock, ChatResponse, ChatStreamUpdate, ComboRunEvent, ComboRunResult, ComboStreamKind as ComboRunStreamKind, Config, Content as ChatContent, Message as ChatMessage, - NotificationRule, NotificationWhen, Output, RetryAttempt, RetryNotifier, RetryUpdate, - RuntimeOverrides, Starter, StopReason, TextEdit, ToolUse, UsageStats, discover_starters, - load_runtime_overrides, save_runtime_overrides, + NotificationRule, NotificationWhen, Output, PromptPayload, RetryAttempt, RetryNotifier, + RetryUpdate, RuntimeOverrides, SessionSocketClient, Starter, StopReason, TextEdit, ToolUse, + UsageStats, discover_starters, load_runtime_overrides, save_runtime_overrides, }; /// Holds information about a collected tool result from concurrent execution. @@ -19,6 +19,17 @@ struct CollectedToolResult { is_error: bool, content: code_combo::Content, } + +#[derive(Debug, Clone, PartialEq, Eq)] +enum PendingUserAction { + ToolUse { + tool_use_id: String, + combo_id: Option, + }, + ComboFeedback { + combo_id: String, + }, +} use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use ratatui::{ Frame, @@ -97,6 +108,7 @@ pub struct Chat<'a> { pending_combo_tool_events: HashMap>, combo_offload_tool_use_to_combo_id: HashMap, combo_pending_reply_session_socks: HashMap, + pending_user_actions: Vec, last_usage: Option, retry_status: Option, transcript_scopes: Vec, @@ -346,6 +358,7 @@ impl Chat<'static> { pending_combo_tool_events: HashMap::new(), combo_offload_tool_use_to_combo_id: HashMap::new(), combo_pending_reply_session_socks: HashMap::new(), + pending_user_actions: Vec::new(), last_usage: None, retry_status: None, transcript_scopes: Vec::new(), @@ -570,6 +583,7 @@ impl Chat<'static> { { self.combo_pending_reply_session_socks .insert(id.clone(), session_sock.clone()); + self.set_pending_combo_feedback_action(id.clone()); } self.set_combo_thinking_active(thinking.as_ref().is_some_and(|cfg| cfg.enabled)); self.set_ready(); @@ -592,6 +606,8 @@ impl Chat<'static> { self.combo_pending_reply_session_socks .insert(id.clone(), session_sock); } + self.clear_pending_combo_feedback_action(id); + self.add_pending_tool_use_action(tool_use.id.clone(), Some(id.clone())); if let Some(idx) = self.messages.locate_tool_message(id) { self.update_focus(Focus::Messages); self.messages.focus(idx); @@ -603,6 +619,7 @@ impl Chat<'static> { ComboEvent::Executed { id, starter, .. } => { self.set_combo_thinking_active(false); self.combo_pending_reply_session_socks.remove(id); + self.clear_pending_actions_for_combo(id); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); } @@ -618,6 +635,7 @@ impl Chat<'static> { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); self.combo_pending_reply_session_socks.remove(id); + self.clear_pending_actions_for_combo(id); self.set_combo_thinking_active(false); self.set_ready(); } @@ -626,6 +644,7 @@ impl Chat<'static> { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); self.combo_pending_reply_session_socks.remove(id); + self.clear_pending_actions_for_combo(id); } self.set_combo_thinking_active(false); self.set_ready(); @@ -640,6 +659,7 @@ impl Chat<'static> { } ComboEvent::ReplyToolResult { tool_use_id, .. } => { self.combo_offload_tool_use_to_combo_id.remove(tool_use_id); + self.remove_pending_tool_use_action(tool_use_id); // Result is handled by Combo component } ComboEvent::Transcript { id, name, messages } => { @@ -1134,6 +1154,7 @@ impl Chat<'static> { self.combo_offload_tool_use_to_combo_id .retain(|_, combo_id| combo_id != id); self.combo_pending_reply_session_socks.remove(id); + self.clear_pending_actions_for_combo(id); self.set_combo_thinking_active(false); if let Err(err) = starter.combo.as_ref() { warn!(?err, "Failed to execute starter"); @@ -1374,6 +1395,9 @@ impl Chat<'static> { return; } debug!(?value, "submiting"); + if self.try_submit_combo_feedback(&value) { + return; + } self.messages.collapse_thinking(); self.messages .push(Message::user(Plain::new(value.clone()).into())); @@ -1963,6 +1987,147 @@ impl Chat<'static> { .map(|(combo_id, session_sock)| (combo_id.clone(), session_sock.clone())) } + fn add_pending_tool_use_action(&mut self, tool_use_id: String, combo_id: Option) { + self.pending_user_actions.retain(|action| { + !matches!( + action, + PendingUserAction::ToolUse { + tool_use_id: id, + .. + } if id == &tool_use_id + ) + }); + self.pending_user_actions.push(PendingUserAction::ToolUse { + tool_use_id, + combo_id, + }); + } + + fn remove_pending_tool_use_action(&mut self, tool_use_id: &str) -> Option> { + let mut removed = None; + self.pending_user_actions.retain(|action| { + let PendingUserAction::ToolUse { + tool_use_id: id, + combo_id, + } = action + else { + return true; + }; + if id != tool_use_id { + return true; + } + removed = Some(combo_id.clone()); + false + }); + removed + } + + fn set_pending_combo_feedback_action(&mut self, combo_id: String) { + self.pending_user_actions.retain(|action| { + !matches!( + action, + PendingUserAction::ComboFeedback { combo_id: id } if id == &combo_id + ) + }); + self.pending_user_actions + .push(PendingUserAction::ComboFeedback { combo_id }); + } + + fn clear_pending_combo_feedback_action(&mut self, combo_id: &str) { + self.pending_user_actions.retain(|action| { + !matches!( + action, + PendingUserAction::ComboFeedback { combo_id: id } if id == combo_id + ) + }); + } + + fn clear_pending_actions_for_combo(&mut self, combo_id: &str) { + self.pending_user_actions.retain(|action| match action { + PendingUserAction::ToolUse { + combo_id: Some(id), .. + } => id != combo_id, + PendingUserAction::ToolUse { combo_id: None, .. } => true, + PendingUserAction::ComboFeedback { combo_id: id } => id != combo_id, + }); + } + + fn try_submit_combo_feedback(&mut self, feedback: &str) -> bool { + let pending_feedback_combo_ids = self + .pending_user_actions + .iter() + .filter_map(|action| { + if let PendingUserAction::ComboFeedback { combo_id } = action { + Some(combo_id.clone()) + } else { + None + } + }) + .collect::>(); + + if pending_feedback_combo_ids.is_empty() { + return false; + } + + if pending_feedback_combo_ids.len() != 1 { + self.messages.push(Message::system( + Plain::new("multiple combo feedback actions are pending".to_string()).into(), + )); + self.set_ready(); + return true; + } + + let combo_id = pending_feedback_combo_ids[0].clone(); + let Some(session_sock) = self + .combo_pending_reply_session_socks + .get(&combo_id) + .cloned() + else { + self.messages.push(Message::system( + Plain::new("combo feedback session is unavailable".to_string()).into(), + )); + self.set_ready(); + return true; + }; + + let prompt = feedback.trim().to_string(); + if prompt.is_empty() { + return true; + } + + let payload = PromptPayload { + prompt, + reply: false, + interactive: false, + schemas: Vec::new(), + thinking: None, + }; + let session_sock_for_send = session_sock.clone(); + let result = tokio::task::block_in_place(|| { + tokio::runtime::Handle::current().block_on(async move { + let client = SessionSocketClient::connect(&session_sock_for_send).await?; + client.send_prompt(payload).await + }) + }); + + match result { + Ok(_) => { + self.set_processing(); + } + Err(err) => { + warn!( + ?err, + combo_id, session_sock, "failed to forward combo feedback to session socket" + ); + self.messages.push(Message::system( + Plain::new("failed to forward combo feedback".to_string()).into(), + )); + self.set_ready(); + } + } + true + } + fn maybe_route_pending_combo_reply_tool_use( &self, tool_use: &ToolUse, @@ -2948,6 +3113,7 @@ impl Component for Chat<'static> { ToolAction::Grant(tool_use) => { let patched_tool_use = self.patch_tool_use_with_pending_combo_session_sock(tool_use); + self.remove_pending_tool_use_action(&patched_tool_use.id); self.agent .grant_once(&patched_tool_use.id, &patched_tool_use.name); self.spawn_tool_use(&patched_tool_use); @@ -2955,12 +3121,17 @@ impl Component for Chat<'static> { ToolAction::GrantSession(tool_use) => { let patched_tool_use = self.patch_tool_use_with_pending_combo_session_sock(tool_use); + self.remove_pending_tool_use_action(&patched_tool_use.id); self.agent.grant_session(&patched_tool_use); self.spawn_tool_use(&patched_tool_use); } ToolAction::Cancel(tool_use) => { - let cancelled_combo_id = - self.combo_offload_tool_use_to_combo_id.remove(&tool_use.id); + let pending_combo_id = + self.remove_pending_tool_use_action(&tool_use.id).flatten(); + let cancelled_combo_id = self + .combo_offload_tool_use_to_combo_id + .remove(&tool_use.id) + .or(pending_combo_id); let cancelled_combo_offload = cancelled_combo_id.is_some(); // Move focus back to Input when tool use is cancelled. if let Some(idx) = self.messages.locate_tool_message(&tool_use.id) @@ -2974,6 +3145,12 @@ impl Component for Chat<'static> { self.update_focus(Focus::Input); self.messages.blur(); } + if let Some(combo_id) = cancelled_combo_id { + self.set_pending_combo_feedback_action(combo_id); + self.set_ready(); + global::trigger_schedule_session_save(); + return; + } // Await the next user message to avoid the LLM reacting without further user // instructions self.state diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index 8c10477..bbc8640 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -359,10 +359,6 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { ); send_escape(&mut *writer); wait_for_screen_contains(&mut parser, &rx, "Cancelled", Duration::from_secs(30)); - println!( - "=== screen after reject ===\n{}\n=== end ===", - parser.screen().contents() - ); assert_screen_not_contains( &mut parser, &rx, @@ -370,19 +366,23 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { Duration::from_secs(2), ); - // Submit feedback directly after cancellation. - send_text(&mut *writer, "retry with feedback E2E_REJECT_FEEDBACK"); - send_alt_enter(&mut *writer); - wait_for_screen_contains( - &mut parser, - &rx, - "retry with feedback E2E_REJECT_FEEDBACK", - Duration::from_secs(30), + let feedback_phrase = "retry with feedback E2E_REJECT_FEEDBACK"; + let combo_requests_before_submit = mock.combo_request_user_texts(); + assert!( + !combo_requests_before_submit.is_empty(), + "expected combo interactive requests before feedback submit" ); - println!( - "=== screen after feedback ===\n{}\n=== end ===", - parser.screen().contents() + assert!( + combo_requests_before_submit + .iter() + .all(|text| !text.contains(feedback_phrase)), + "feedback should not appear in combo history before submit: {combo_requests_before_submit:#?}" ); + let non_combo_last_user_texts_before = mock.non_combo_request_last_user_texts(); + + send_text(&mut *writer, feedback_phrase); + wait_for_screen_contains(&mut parser, &rx, feedback_phrase, Duration::from_secs(30)); + send_alt_enter(&mut *writer); wait_for_screen_contains( &mut parser, @@ -392,16 +392,31 @@ fn combo_interactive_coco_reply_reject_then_feedback_can_complete() { ); send_enter(&mut *writer); wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(30)); - println!( - "=== screen after complete ===\n{}\n=== end ===", - parser.screen().contents() - ); assert!( mock.saw_token(FEEDBACK_TOKEN), - "expected feedback token in model request after cancellation" + "expected feedback token in combo interactive request after cancellation" + ); + let combo_requests_after_submit = mock.combo_request_user_texts(); + assert!( + combo_requests_after_submit.len() > combo_requests_before_submit.len(), + "expected additional combo request after feedback submit, before={combo_requests_before_submit:#?}, after={combo_requests_after_submit:#?}" + ); + assert!( + combo_requests_after_submit + .iter() + .skip(combo_requests_before_submit.len()) + .any(|text| text.contains(feedback_phrase)), + "expected feedback to be merged into combo history, got: {combo_requests_after_submit:#?}" + ); + let non_combo_last_user_texts_after = mock.non_combo_request_last_user_texts(); + assert!( + non_combo_last_user_texts_after + .iter() + .skip(non_combo_last_user_texts_before.len()) + .all(|text| !text.contains(feedback_phrase)), + "feedback should not be sent to non-combo history, got: {non_combo_last_user_texts_after:#?}" ); - child .clone_killer() .kill() diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index c5aa571..1b5f054 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -147,7 +147,13 @@ disable_stream = true #[derive(Debug, Default)] struct MockOpenAiState { request_count: usize, + combo_request_count: usize, seen_tokens: HashSet, + request_user_texts: Vec, + request_last_user_texts: Vec, + combo_request_user_texts: Vec, + combo_request_last_user_texts: Vec, + non_combo_request_last_user_texts: Vec, } #[derive(Debug, Clone)] @@ -238,6 +244,49 @@ impl MockOpenAiServer { .seen_tokens .contains(token) } + + #[allow(dead_code)] + pub(crate) fn request_user_texts(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .request_user_texts + .clone() + } + + #[allow(dead_code)] + pub(crate) fn request_last_user_texts(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .request_last_user_texts + .clone() + } + + pub(crate) fn combo_request_user_texts(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .combo_request_user_texts + .clone() + } + + #[allow(dead_code)] + pub(crate) fn combo_request_last_user_texts(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .combo_request_last_user_texts + .clone() + } + + pub(crate) fn non_combo_request_last_user_texts(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .non_combo_request_last_user_texts + .clone() + } } impl Drop for MockOpenAiServer { @@ -356,7 +405,7 @@ fn build_mock_openai_response( scenario: &MockOpenAiScenario, state: &Arc>, ) -> MockHttpResponse { - let user_text = request + let user_messages = request .get("messages") .and_then(|value| value.as_array()) .map(|messages| { @@ -367,42 +416,57 @@ fn build_mock_openai_response( if role != "user" { return None; } - message + let text = message .get("content") - .and_then(|value| value.as_str()) - .map(str::to_string) + .map(extract_text_content) + .unwrap_or_default(); + if text.is_empty() { None } else { Some(text) } }) .collect::>() - .join("\n") }) .unwrap_or_default(); + let user_text = user_messages.join("\n"); + let last_user_text = user_messages.last().cloned().unwrap_or_default(); let is_summary_prompt = user_text.contains("Summarize the combo execution for the user."); - - let request_count = { + let is_combo_request = is_combo_interactive_history(&user_text); + let (_request_count, combo_request_count) = { let mut guard = state.lock().expect("lock mock state"); guard.request_count += 1; + guard.request_user_texts.push(user_text.clone()); + guard.request_last_user_texts.push(last_user_text.clone()); + if is_combo_request { + guard.combo_request_count += 1; + guard.combo_request_user_texts.push(user_text.clone()); + guard + .combo_request_last_user_texts + .push(last_user_text.clone()); + } else { + guard + .non_combo_request_last_user_texts + .push(last_user_text.clone()); + } match scenario { MockOpenAiScenario::RequireFeedbackTokens(tokens) => { for token in tokens { - if user_text.contains(token) { + if is_combo_request && user_text.contains(token) { guard.seen_tokens.insert(token.clone()); } } } MockOpenAiScenario::RejectThenNeedFeedbackToken(token) => { - if user_text.contains(token) { + if is_combo_request && user_text.contains(token) { guard.seen_tokens.insert(token.clone()); } } MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(token) => { - if user_text.contains(token) { + if is_combo_request && user_text.contains(token) { guard.seen_tokens.insert(token.clone()); } } MockOpenAiScenario::ImmediateReply | MockOpenAiScenario::RateLimited => {} } - guard.request_count + (guard.request_count, guard.combo_request_count) }; if is_summary_prompt { @@ -428,7 +492,12 @@ fn build_mock_openai_response( MockHttpResponse::Ok(mock_bash_tool_call_response(command)) } MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(token) => { - if request_count == 1 || user_text.contains(token) { + if !is_combo_request { + return MockHttpResponse::Ok(mock_text_response( + "Please provide feedback in combo interactive flow.", + )); + } + if combo_request_count == 1 || user_text.contains(token) { let command = "coco reply --result='mock polished result' --reason='used user feedback'"; return MockHttpResponse::Ok(mock_bash_tool_call_response(command)); @@ -440,12 +509,17 @@ fn build_mock_openai_response( ))) } MockOpenAiScenario::RejectThenNeedFeedbackToken(token) => { + if !is_combo_request { + return MockHttpResponse::Ok(mock_text_response( + "Please provide feedback in combo interactive flow.", + )); + } if user_text.contains(token) { let command = "coco reply --result='mock polished result' --reason='used user feedback'"; return MockHttpResponse::Ok(mock_bash_tool_call_response(command)); } - if request_count == 1 { + if combo_request_count == 1 { return MockHttpResponse::Ok(mock_bash_tool_call_response("rm -rf /")); } thread::sleep(Duration::from_millis(500)); @@ -455,6 +529,11 @@ fn build_mock_openai_response( ))) } MockOpenAiScenario::RequireFeedbackTokens(tokens) => { + if !is_combo_request { + return MockHttpResponse::Ok(mock_text_response( + "Please provide feedback in combo interactive flow.", + )); + } let missing = tokens .iter() .filter(|token| !user_text.contains(token.as_str())) @@ -474,6 +553,41 @@ fn build_mock_openai_response( } } +fn extract_text_content(content: &serde_json::Value) -> String { + if let Some(text) = content.as_str() { + return text.to_string(); + } + let Some(parts) = content.as_array() else { + return String::new(); + }; + parts + .iter() + .filter_map(|part| { + part.get("text") + .and_then(|value| value.as_str()) + .or_else(|| part.get("input_text").and_then(|value| value.as_str())) + .or_else(|| { + part.get("type") + .and_then(|value| value.as_str()) + .and_then(|kind| match kind { + "text" | "input_text" => { + part.get("value").and_then(|value| value.as_str()) + } + _ => None, + }) + }) + .map(str::to_string) + }) + .collect::>() + .join("\n") +} + +fn is_combo_interactive_history(user_text: &str) -> bool { + user_text.contains("You can interact in multiple rounds and call tools as needed.") + || user_text.contains("Continue if needed. When ready, finish with: coco reply") + || user_text.contains("Process input:") +} + fn mock_text_response(content: &str) -> serde_json::Value { serde_json::json!({ "choices": [{ diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 7256912..42a99fe 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -30,9 +30,10 @@ use crate::tools::{ pub type ExecuteResult = Result; use crate::{ - Agent, Block, ChatStreamUpdate, Config, Content, Message, OutputChunk, PromptSchema, - SessionEnv, Starter, StarterCommand, StarterError, StarterEvent, ToolUse, bash_unsafe_ranges, - bash_unsafe_reason, discover_starters, exec::StreamKind, parse_primary_command, workspace_dir, + Agent, Block, ChatStreamUpdate, Config, Content, Message, OutputChunk, PromptResponseSender, + PromptSchema, SessionEnv, Starter, StarterCommand, StarterError, StarterEvent, ToolUse, + bash_unsafe_ranges, bash_unsafe_reason, discover_starters, exec::StreamKind, + parse_primary_command, workspace_dir, }; // Import types from combo module @@ -250,6 +251,8 @@ async fn execute_combo( let mut tool_failed = false; let mut starter_failed = false; let mut cancelled = false; + let mut pending_interactive_schemas: Option> = None; + let mut pending_interactive_responder: Option = None; let command_line = format_combo_run_command(&combo_name, &args); // Emit executing event @@ -347,6 +350,32 @@ async fn execute_combo( session_sock: None, }); summary_parts.push(format!("[Prompt] {}", prompt)); + + if let Some(schemas) = pending_interactive_schemas.clone() { + let result = handle_interactive_combo_reply( + &mut reply_agent, + &schemas, + &combo_name, + cancel_token.clone(), + session_socket_path.clone(), + &on_event_for_emit, + false, + ) + .await; + if let Err(err) = result { + let message = err.to_string(); + emit_combo_event( + &on_event_for_emit, + ComboEvent::ReplyToolError { + message: message.clone(), + }, + ); + if let Some(responder) = pending_interactive_responder.take() { + let _ = responder.send(Err(message)); + } + pending_interactive_schemas = None; + } + } } StarterEvent::PromptRequest { prompt, @@ -367,6 +396,8 @@ async fn execute_combo( summary_parts.push(format!("[Prompt] {}", prompt)); if interactive { + pending_interactive_schemas = Some(schemas.clone()); + pending_interactive_responder = Some(responder.clone()); let result = handle_interactive_combo_reply( &mut reply_agent, &schemas, @@ -374,6 +405,7 @@ async fn execute_combo( cancel_token.clone(), session_socket_path.clone(), &on_event_for_emit, + true, ) .await; if let Err(err) = result { @@ -385,8 +417,12 @@ async fn execute_combo( }, ); let _ = responder.send(Err(message)); + pending_interactive_schemas = None; + pending_interactive_responder = None; } } else if reply_agent.offload_combo_reply() { + pending_interactive_schemas = None; + pending_interactive_responder = None; let result = handle_offload_combo_reply_with_retry( &mut reply_agent, &schemas, @@ -407,6 +443,8 @@ async fn execute_combo( let _ = responder.send(Err(message)); } } else { + pending_interactive_schemas = None; + pending_interactive_responder = None; let disable_stream = reply_agent.disable_stream_for_current_model(); let mut streamed_thinking = false; @@ -517,13 +555,19 @@ async fn execute_combo( } StarterEvent::Finished { exit_code: code } => { exit_code = code; + pending_interactive_schemas = None; + pending_interactive_responder = None; } StarterEvent::Cancelled => { cancelled = true; + pending_interactive_schemas = None; + pending_interactive_responder = None; } StarterEvent::Failed { reason } => { starter_failed = true; summary_parts.push(format!("[Failed] {}", reason)); + pending_interactive_schemas = None; + pending_interactive_responder = None; } } } @@ -1110,15 +1154,18 @@ async fn handle_interactive_combo_reply( cancel_token: CancellationToken, session_socket_path: PathBuf, on_event: &ComboEventCallback, + seed_directive: bool, ) -> Result<(), ComboReplyError> { if cancel_token.is_cancelled() { return Err(ComboReplyError::Cancelled); } - agent - .append_message(Message::user(Content::Text( - build_interactive_offload_reply_directive(schemas), - ))) - .await; + if seed_directive { + agent + .append_message(Message::user(Content::Text( + build_interactive_offload_reply_directive(schemas), + ))) + .await; + } let max_turns = 50usize; for _ in 0..max_turns { @@ -1196,7 +1243,9 @@ async fn handle_interactive_combo_reply( build_interactive_offload_reply_reminder(schemas), ))) .await; - continue; + // Pause and wait for user feedback from combo session. + // The next feedback prompt will re-enter interactive handling. + return Ok(()); } let mut tool_results = Vec::new(); From 6be5dc0aaf57740d1eebcee059883fb2c850feec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 6 Feb 2026 23:28:51 +0800 Subject: [PATCH 17/20] fix: require explicit combo reply session sock --- crates/coco-tui/src/components/chat.rs | 81 ++--------------------- src/combo/runner.rs | 91 ++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 80 deletions(-) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index 3b3947e..a1c5f51 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -1977,16 +1977,6 @@ impl Chat<'static> { }) } - fn lone_pending_combo_reply_session(&self) -> Option<(String, String)> { - if self.combo_pending_reply_session_socks.len() != 1 { - return None; - } - self.combo_pending_reply_session_socks - .iter() - .next() - .map(|(combo_id, session_sock)| (combo_id.clone(), session_sock.clone())) - } - fn add_pending_tool_use_action(&mut self, tool_use_id: String, combo_id: Option) { self.pending_user_actions.retain(|action| { !matches!( @@ -2147,40 +2137,7 @@ impl Chat<'static> { let combo_id = self.pending_combo_id_for_session_sock(session_sock)?; return Some((combo_id, tool_use.clone())); } - let (combo_id, session_sock) = self.lone_pending_combo_reply_session()?; - let tool_use = inject_session_sock_into_tool_use(tool_use, &session_sock)?; - Some((combo_id, tool_use)) - } - - fn patch_tool_use_with_pending_combo_session_sock(&self, tool_use: &ToolUse) -> ToolUse { - if tool_use.name != BASH_TOOL_NAME { - return tool_use.clone(); - } - let Ok(input) = serde_json::from_value::(tool_use.input.clone()) else { - return tool_use.clone(); - }; - if !is_coco_reply_command(&input.command) { - return tool_use.clone(); - } - if session_sock_from_tool_use(tool_use).is_some() { - return tool_use.clone(); - } - - let session_sock = - if let Some(combo_id) = self.combo_offload_tool_use_to_combo_id.get(&tool_use.id) { - self.combo_pending_reply_session_socks - .get(combo_id) - .cloned() - } else { - self.lone_pending_combo_reply_session() - .map(|(_, session_sock)| session_sock) - }; - if let Some(session_sock) = session_sock - && let Some(patched) = inject_session_sock_into_tool_use(tool_use, &session_sock) - { - return patched; - } - tool_use.clone() + None } fn push_combo_offload_reply_tool_use(&mut self, combo_id: String, tool_use: ToolUse) { @@ -3111,19 +3068,14 @@ impl Component for Chat<'static> { }, Action::Tool(action) => match action { ToolAction::Grant(tool_use) => { - let patched_tool_use = - self.patch_tool_use_with_pending_combo_session_sock(tool_use); - self.remove_pending_tool_use_action(&patched_tool_use.id); - self.agent - .grant_once(&patched_tool_use.id, &patched_tool_use.name); - self.spawn_tool_use(&patched_tool_use); + self.remove_pending_tool_use_action(&tool_use.id); + self.agent.grant_once(&tool_use.id, &tool_use.name); + self.spawn_tool_use(tool_use); } ToolAction::GrantSession(tool_use) => { - let patched_tool_use = - self.patch_tool_use_with_pending_combo_session_sock(tool_use); - self.remove_pending_tool_use_action(&patched_tool_use.id); - self.agent.grant_session(&patched_tool_use); - self.spawn_tool_use(&patched_tool_use); + self.remove_pending_tool_use_action(&tool_use.id); + self.agent.grant_session(tool_use); + self.spawn_tool_use(tool_use); } ToolAction::Cancel(tool_use) => { let pending_combo_id = @@ -3464,25 +3416,6 @@ fn session_sock_from_tool_use(tool_use: &ToolUse) -> Option { .cloned() } -fn inject_session_sock_into_tool_use(tool_use: &ToolUse, session_sock: &str) -> Option { - if tool_use.name != BASH_TOOL_NAME { - return None; - } - let mut input = serde_json::from_value::(tool_use.input.clone()).ok()?; - let has_session_sock = input - .env - .get("COCO_SESSION_SOCK") - .is_some_and(|value| !value.is_empty()); - if !has_session_sock { - input - .env - .insert("COCO_SESSION_SOCK".to_string(), session_sock.to_string()); - } - let mut patched = tool_use.clone(); - patched.input = serde_json::to_value(&input).ok()?; - Some(patched) -} - fn session_summary_prompt() -> String { format!( "Write a short session summary for a session switcher list. Return a single line (max {SESSION_SUMMARY_MAX_LEN} chars), plain text only, no quotes. Focus on the user's goal and progress. If there is no meaningful content, return an empty string." diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 42a99fe..69b610b 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -1523,14 +1523,75 @@ fn is_coco_command_name(name: &str) -> bool { } fn is_coco_reply_command(command: &str) -> bool { - let summary = match parse_primary_command(command) { - Ok(summary) => summary, - Err(_) => return false, - }; - if !is_coco_command_name(&summary.name) { + if let Ok(summary) = parse_primary_command(command) + && is_coco_command_name(&summary.name) + { + return matches!(summary.args.first(), Some(arg) if arg == "reply"); + } + contains_coco_reply_invocation(command) +} + +fn contains_coco_reply_invocation(command: &str) -> bool { + let tokens = command + .split_whitespace() + .map(normalize_token) + .filter(|token| !token.is_empty()) + .collect::>(); + if tokens.len() < 2 { return false; } - matches!(summary.args.first(), Some(arg) if arg == "reply") + for i in 0..(tokens.len() - 1) { + if !is_coco_command_name(tokens[i].as_str()) { + continue; + } + if tokens[i + 1] != "reply" { + continue; + } + if i == 0 { + return true; + } + if tokens[..i] + .iter() + .all(|token| is_env_assignment_token(token)) + { + return true; + } + if i >= 2 && is_shell_command_name(tokens[0].as_str()) { + let flag_index = (1..i) + .rev() + .find(|index| matches!(tokens[*index].as_str(), "-c" | "-lc")); + if let Some(flag_index) = flag_index + && tokens[(flag_index + 1)..i] + .iter() + .all(|token| is_env_assignment_token(token)) + { + return true; + } + } + } + false +} + +fn normalize_token(token: &str) -> String { + token + .trim_matches(|c| matches!(c, '\'' | '"' | ';')) + .to_string() +} + +fn is_shell_command_name(token: &str) -> bool { + matches!(token.rsplit('/').next(), Some("sh" | "bash" | "zsh")) +} + +fn is_env_assignment_token(token: &str) -> bool { + let Some((name, _)) = token.split_once('=') else { + return false; + }; + let mut chars = name.chars(); + match chars.next() { + Some(c) if c == '_' || c.is_ascii_alphabetic() => {} + _ => return false, + } + chars.all(|c| c == '_' || c.is_ascii_alphanumeric()) } fn is_safe_command(command: &str) -> bool { @@ -1923,8 +1984,20 @@ mod tests { assert!(is_coco_reply_command( "/usr/local/bin/coco reply --message='hello world'" )); + assert!(is_coco_reply_command( + "COCO_SESSION_SOCK='/tmp/coco.sock' coco reply --message='hello world'" + )); + assert!(is_coco_reply_command( + "/run/current-system/sw/bin/zsh -lc \"coco reply --message='hello world'\"" + )); + assert!(is_coco_reply_command( + "bash -lc \"COCO_SESSION_SOCK=/tmp/coco.sock coco reply --message='hello world'\"" + )); assert!(!is_coco_reply_command("coco ask hello")); assert!(!is_coco_reply_command("echo coco reply --message=hello")); + assert!(!is_coco_reply_command( + "/run/current-system/sw/bin/zsh -lc \"echo coco reply --message=hello\"" + )); } #[test] @@ -1933,6 +2006,12 @@ mod tests { classify_offload_command("coco reply --message=done --status=ok"), OffloadCommandKind::Coco )); + assert!(matches!( + classify_offload_command( + "/run/current-system/sw/bin/zsh -lc \"coco reply --message=done --status=ok\"" + ), + OffloadCommandKind::Coco + )); assert!(matches!( classify_offload_command("ls -la"), OffloadCommandKind::Safe From 2c8b99c006aee76f39d7c9e2c58bd582976e3982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Sat, 7 Feb 2026 00:12:05 +0800 Subject: [PATCH 18/20] feat: auto nudge interactive combo reply before feedback --- .../src/e2e_tests/combo_interactive.rs | 56 ++++++++++++++++++- crates/coco-tui/src/e2e_tests/support.rs | 33 ++++++++++- src/combo/runner.rs | 23 ++++++-- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/crates/coco-tui/src/e2e_tests/combo_interactive.rs b/crates/coco-tui/src/e2e_tests/combo_interactive.rs index bbc8640..d37c3b5 100644 --- a/crates/coco-tui/src/e2e_tests/combo_interactive.rs +++ b/crates/coco-tui/src/e2e_tests/combo_interactive.rs @@ -1,4 +1,7 @@ -use std::time::Duration; +use std::{ + thread, + time::{Duration, Instant}, +}; use vt100::Parser; @@ -98,6 +101,23 @@ fn assert_shutdown_ok( assert!(status.success(), "tui exit failed: {status:?}"); } +fn wait_for_request_count(mock: &MockOpenAiServer, min: usize, timeout: Duration) { + let start = Instant::now(); + while start.elapsed() < timeout { + if mock.request_count() >= min { + return; + } + thread::sleep(Duration::from_millis(50)); + } + panic!( + "timed out waiting for mock request count >= {min}, current={}, paths={:#?}, all_user_texts={:#?}, combo_user_texts={:#?}", + mock.request_count(), + mock.request_paths(), + mock.request_user_texts(), + mock.combo_request_user_texts() + ); +} + #[test] fn combo_interactive_shows_feedback_prompt_before_reply_tool_use() { let mock = MockOpenAiServer::start("E2E_TOKEN_PROMPT"); @@ -121,11 +141,12 @@ fn combo_interactive_shows_feedback_prompt_before_reply_tool_use() { "Bash Awaiting confirmation", Duration::from_secs(2), ); + wait_for_request_count(&mock, 3, Duration::from_secs(5)); assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); assert!( - mock.request_count() >= 1, - "expected at least one LLM call, got {}", + mock.request_count() >= 3, + "expected auto implicit nudges before waiting feedback, got {} requests", mock.request_count() ); assert!( @@ -134,6 +155,35 @@ fn combo_interactive_shows_feedback_prompt_before_reply_tool_use() { ); } +#[test] +fn combo_interactive_auto_nudge_can_reach_reply_without_manual_feedback() { + let mock = + MockOpenAiServer::start_with_scenario(MockOpenAiScenario::TextThenReplyAfterImplicitNudge); + let (mut guard, mut writer, rx, mut parser, mut child, _temp) = run_combo_with_mock(&mock); + + wait_for_screen_contains( + &mut parser, + &rx, + "Combo: e2e_mock_interactive", + Duration::from_secs(30), + ); + wait_for_screen_contains( + &mut parser, + &rx, + "Bash Awaiting confirmation", + Duration::from_secs(30), + ); + assert!( + mock.request_count() >= 2, + "expected at least two model requests for auto-nudge flow, got {}", + mock.request_count() + ); + + send_enter(&mut *writer); + wait_for_screen_contains(&mut parser, &rx, "Completed", Duration::from_secs(30)); + assert_shutdown_ok(&mut *child, &mut *writer, &parser, &mut guard); +} + #[test] fn combo_interactive_allows_feedback_before_coco_reply_tool_use() { let mock = MockOpenAiServer::start("E2E_FEEDBACK_TOKEN"); diff --git a/crates/coco-tui/src/e2e_tests/support.rs b/crates/coco-tui/src/e2e_tests/support.rs index 1b5f054..f8864d6 100644 --- a/crates/coco-tui/src/e2e_tests/support.rs +++ b/crates/coco-tui/src/e2e_tests/support.rs @@ -149,6 +149,7 @@ struct MockOpenAiState { request_count: usize, combo_request_count: usize, seen_tokens: HashSet, + request_paths: Vec, request_user_texts: Vec, request_last_user_texts: Vec, combo_request_user_texts: Vec, @@ -160,6 +161,7 @@ struct MockOpenAiState { pub(crate) enum MockOpenAiScenario { RequireFeedbackTokens(Vec), ImmediateReply, + TextThenReplyAfterImplicitNudge, ImmediateReplyThenRequireFeedbackToken(String), RejectThenNeedFeedbackToken(String), RateLimited, @@ -230,6 +232,14 @@ impl MockOpenAiServer { self.state.lock().expect("lock mock state").request_count } + pub(crate) fn request_paths(&self) -> Vec { + self.state + .lock() + .expect("lock mock state") + .request_paths + .clone() + } + pub(crate) fn saw_feedback_token(&self) -> bool { let Some(token) = self.primary_feedback_token.as_ref() else { return false; @@ -309,6 +319,10 @@ fn handle_mock_openai_connection( let _ = write_http_response(stream, 400, br#"{"error":"bad request"}"#); return; }; + { + let mut guard = state.lock().expect("lock mock state"); + guard.request_paths.push(path.clone()); + } if path != "/v1/chat/completions" && path != "/chat/completions" { let _ = write_http_response(stream, 404, br#"{"error":"not found"}"#); return; @@ -464,7 +478,9 @@ fn build_mock_openai_response( guard.seen_tokens.insert(token.clone()); } } - MockOpenAiScenario::ImmediateReply | MockOpenAiScenario::RateLimited => {} + MockOpenAiScenario::ImmediateReply + | MockOpenAiScenario::TextThenReplyAfterImplicitNudge + | MockOpenAiScenario::RateLimited => {} } (guard.request_count, guard.combo_request_count) }; @@ -491,6 +507,21 @@ fn build_mock_openai_response( "coco reply --result='mock polished result' --reason='used user feedback'"; MockHttpResponse::Ok(mock_bash_tool_call_response(command)) } + MockOpenAiScenario::TextThenReplyAfterImplicitNudge => { + if !is_combo_request { + return MockHttpResponse::Ok(mock_text_response( + "Please provide feedback in combo interactive flow.", + )); + } + if combo_request_count == 1 { + return MockHttpResponse::Ok(mock_text_response( + "Still checking. I need one more confirmation round before reply.", + )); + } + let command = + "coco reply --result='mock polished result' --reason='used user feedback'"; + MockHttpResponse::Ok(mock_bash_tool_call_response(command)) + } MockOpenAiScenario::ImmediateReplyThenRequireFeedbackToken(token) => { if !is_combo_request { return MockHttpResponse::Ok(mock_text_response( diff --git a/src/combo/runner.rs b/src/combo/runner.rs index 69b610b..0540092 100644 --- a/src/combo/runner.rs +++ b/src/combo/runner.rs @@ -1080,11 +1080,14 @@ fn build_interactive_offload_reply_reminder(schemas: &[PromptSchema]) -> String .map(|schema| format!("--{}=...", schema.name)) .collect(); format!( - "Continue if needed. When ready, finish with: coco reply {}", + "{INTERACTIVE_REPLY_REMINDER_PREFIX} {}", field_args.join(" ") ) } +const INTERACTIVE_REPLY_REMINDER_PREFIX: &str = + "Continue if needed. When ready, finish with: coco reply"; + async fn execute_interactive_tool_use( agent: &mut Agent, tool_use: &ToolUse, @@ -1156,6 +1159,8 @@ async fn handle_interactive_combo_reply( on_event: &ComboEventCallback, seed_directive: bool, ) -> Result<(), ComboReplyError> { + const AUTO_IMPLICIT_NUDGE_LIMIT: usize = 2; + if cancel_token.is_cancelled() { return Err(ComboReplyError::Cancelled); } @@ -1168,6 +1173,7 @@ async fn handle_interactive_combo_reply( } let max_turns = 50usize; + let mut empty_tool_use_turns = 0usize; for _ in 0..max_turns { if cancel_token.is_cancelled() { return Err(ComboReplyError::Cancelled); @@ -1224,9 +1230,12 @@ async fn handle_interactive_combo_reply( let tool_uses = extract_tool_uses(&response.message); if tool_uses.is_empty() { + empty_tool_use_turns += 1; + let reminder = build_interactive_offload_reply_reminder(schemas); + let should_pause_for_feedback = empty_tool_use_turns > AUTO_IMPLICIT_NUDGE_LIMIT; if disable_stream { let response_text = extract_text_response(&response.message); - if !response_text.trim().is_empty() { + if should_pause_for_feedback && !response_text.trim().is_empty() { emit_combo_event( on_event, ComboEvent::Prompt { @@ -1239,14 +1248,16 @@ async fn handle_interactive_combo_reply( } } agent - .append_message(Message::user(Content::Text( - build_interactive_offload_reply_reminder(schemas), - ))) + .append_message(Message::user(Content::Text(reminder))) .await; - // Pause and wait for user feedback from combo session. + if !should_pause_for_feedback { + continue; + } + // Pause and wait for user feedback from combo session after bounded auto nudges. // The next feedback prompt will re-enter interactive handling. return Ok(()); } + empty_tool_use_turns = 0; let mut tool_results = Vec::new(); let mut guidance: Option = None; From 1507f24dc35c9e0ac07704b1a4a4bfa29322a870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Sat, 7 Feb 2026 00:54:17 +0800 Subject: [PATCH 19/20] feat: auto focus next pending user action after tool approval/cancellation --- crates/coco-tui/src/components/chat.rs | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/coco-tui/src/components/chat.rs b/crates/coco-tui/src/components/chat.rs index a1c5f51..f5097c6 100644 --- a/crates/coco-tui/src/components/chat.rs +++ b/crates/coco-tui/src/components/chat.rs @@ -2042,6 +2042,36 @@ impl Chat<'static> { }); } + fn focus_next_pending_user_action(&mut self) -> bool { + for action in self.pending_user_actions.clone() { + match action { + PendingUserAction::ToolUse { + tool_use_id, + combo_id, + } => { + if let Some(combo_id) = combo_id + && let Some(idx) = self.messages.locate_tool_message(&combo_id) + { + self.update_focus(Focus::Messages); + self.messages.focus(idx); + return true; + } + if let Some(idx) = self.messages.locate_tool_message(&tool_use_id) { + self.update_focus(Focus::Messages); + self.messages.focus(idx); + return true; + } + } + PendingUserAction::ComboFeedback { .. } => { + self.update_focus(Focus::Input); + self.messages.blur(); + return true; + } + } + } + false + } + fn try_submit_combo_feedback(&mut self, feedback: &str) -> bool { let pending_feedback_combo_ids = self .pending_user_actions @@ -3071,11 +3101,13 @@ impl Component for Chat<'static> { self.remove_pending_tool_use_action(&tool_use.id); self.agent.grant_once(&tool_use.id, &tool_use.name); self.spawn_tool_use(tool_use); + self.focus_next_pending_user_action(); } ToolAction::GrantSession(tool_use) => { self.remove_pending_tool_use_action(&tool_use.id); self.agent.grant_session(tool_use); self.spawn_tool_use(tool_use); + self.focus_next_pending_user_action(); } ToolAction::Cancel(tool_use) => { let pending_combo_id = @@ -3100,6 +3132,7 @@ impl Component for Chat<'static> { if let Some(combo_id) = cancelled_combo_id { self.set_pending_combo_feedback_action(combo_id); self.set_ready(); + self.focus_next_pending_user_action(); global::trigger_schedule_session_save(); return; } @@ -3115,6 +3148,7 @@ impl Component for Chat<'static> { }); // Set chat status to Ready after cancellation self.set_ready(); + self.focus_next_pending_user_action(); // Trigger session save after tool cancellation global::trigger_schedule_session_save(); } From e8744ab39d1611438c4b46c684ca5b82ceab4cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Sat, 7 Feb 2026 02:02:25 +0800 Subject: [PATCH 20/20] refactor: use set_bash_env API for test session socket injection and allow Backspace to clear child focus --- .../coco-tui/src/components/messages/combo.rs | 2 +- src/agent/executor.rs | 21 +------------------ 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/crates/coco-tui/src/components/messages/combo.rs b/crates/coco-tui/src/components/messages/combo.rs index c8d76e8..f19b636 100644 --- a/crates/coco-tui/src/components/messages/combo.rs +++ b/crates/coco-tui/src/components/messages/combo.rs @@ -1156,7 +1156,7 @@ impl Component for Combo { if self.is_child_focused { match (key.modifiers, key.code) { - (KeyModifiers::NONE, KeyCode::Esc) => self.clear_child_focus(), + (KeyModifiers::NONE, KeyCode::Esc | KeyCode::Backspace) => self.clear_child_focus(), (KeyModifiers::NONE, KeyCode::Char('r')) => { self.messages.toggle_thinking_for_focus(); } diff --git a/src/agent/executor.rs b/src/agent/executor.rs index 61a517c..37b16b2 100644 --- a/src/agent/executor.rs +++ b/src/agent/executor.rs @@ -470,10 +470,6 @@ mod tests { use super::*; use crate::tools::{BashOutput, StrReplaceInput}; use std::collections::HashSet; - use std::sync::OnceLock; - use tokio::sync::Mutex; - - static ENV_LOCK: OnceLock> = OnceLock::new(); fn bash_input_value(command: &str) -> serde_json::Value { serde_json::to_value(BashInput { @@ -566,14 +562,8 @@ mod tests { #[tokio::test] async fn bash_execution_receives_session_socket_env() { - let _guard = ENV_LOCK.get_or_init(|| Mutex::new(())).lock().await; - let previous = std::env::var_os("COCO_SESSION_SOCK"); - // Safety: test-only mutation guarded by ENV_LOCK and restored before return. - unsafe { - std::env::set_var("COCO_SESSION_SOCK", "/tmp/coco-executor.sock"); - } - let mut executor = Executor::default(); + executor.set_bash_env("COCO_SESSION_SOCK", "/tmp/coco-executor.sock"); let input_value = bash_input_value(r#"printf "%s" "$COCO_SESSION_SOCK""#); executor.update_pcl( BASH_TOOL_NAME, @@ -584,15 +574,6 @@ mod tests { .await .expect("execute bash with injected env"); - // Safety: test-only mutation guarded by ENV_LOCK. - unsafe { - if let Some(value) = previous { - std::env::set_var("COCO_SESSION_SOCK", value); - } else { - std::env::remove_var("COCO_SESSION_SOCK"); - } - } - let Output::Success(Final::Json(value)) = output else { panic!("expected successful bash output"); };