From e364e23eb5c6160a998de8809fcf5d6664bc9338 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Mon, 9 Mar 2026 21:49:49 -0500 Subject: [PATCH 1/3] feat: Make triggers opt-in and fix inject submission Triggers are no longer auto-bound to the first idle trigger on spawn. Instead, users explicitly bind via `--trigger ` on spawn or `pu trigger assign ` post-hoc. Also switches inject_text from raw write_to_fd to write_chunked_submit, fixing the race where pasted text + Enter was swallowed by the TUI. Co-Authored-By: Claude Opus 4.6 --- crates/pu-cli/src/commands/spawn.rs | 2 + crates/pu-cli/src/commands/trigger.rs | 22 ++++ crates/pu-cli/src/main.rs | 21 +++- crates/pu-cli/src/output.rs | 12 +++ crates/pu-core/src/protocol.rs | 97 +++++++++++++++++ crates/pu-engine/src/engine.rs | 143 ++++++++++++++++++++++---- crates/pu-engine/src/test_helpers.rs | 1 + 7 files changed, 276 insertions(+), 22 deletions(-) diff --git a/crates/pu-cli/src/commands/spawn.rs b/crates/pu-cli/src/commands/spawn.rs index 1752bfd..0c9bc49 100644 --- a/crates/pu-cli/src/commands/spawn.rs +++ b/crates/pu-cli/src/commands/spawn.rs @@ -25,6 +25,7 @@ pub async fn run( agent_args: Option, plan_mode: bool, no_trigger: bool, + trigger: Option, json: bool, ) -> Result<(), CliError> { daemon_ctrl::ensure_daemon(socket).await?; @@ -73,6 +74,7 @@ pub async fn run( extra_args, plan_mode, no_trigger, + trigger, }, ) .await?; diff --git a/crates/pu-cli/src/commands/trigger.rs b/crates/pu-cli/src/commands/trigger.rs index f089aeb..096ab1a 100644 --- a/crates/pu-cli/src/commands/trigger.rs +++ b/crates/pu-cli/src/commands/trigger.rs @@ -111,3 +111,25 @@ pub async fn run_delete( output::print_response(&resp, json)?; Ok(()) } + +pub async fn run_assign( + socket: &Path, + agent_id: &str, + trigger_name: &str, + json: bool, +) -> Result<(), CliError> { + daemon_ctrl::ensure_daemon(socket).await?; + let project_root = commands::cwd_string()?; + let resp = client::send_request( + socket, + &Request::AssignTrigger { + project_root, + agent_id: agent_id.to_string(), + trigger_name: trigger_name.to_string(), + }, + ) + .await?; + let resp = output::check_response(resp, json)?; + output::print_response(&resp, json)?; + Ok(()) +} diff --git a/crates/pu-cli/src/main.rs b/crates/pu-cli/src/main.rs index 23221ac..a4abb02 100644 --- a/crates/pu-cli/src/main.rs +++ b/crates/pu-cli/src/main.rs @@ -65,6 +65,9 @@ enum Commands { /// Disable event triggers for this agent #[arg(long)] no_trigger: bool, + /// Bind an idle trigger to this agent (name of trigger in .pu/triggers/) + #[arg(long)] + trigger: Option, /// Output as JSON #[arg(long)] json: bool, @@ -575,6 +578,16 @@ enum TriggerAction { #[arg(long)] json: bool, }, + /// Assign a trigger to an idle agent + Assign { + /// Agent ID + agent_id: String, + /// Trigger name + trigger_name: String, + /// Output as JSON + #[arg(long)] + json: bool, + }, } #[tokio::main] @@ -608,11 +621,12 @@ async fn main() { agent_args, plan, no_trigger, + trigger, json, } => { commands::spawn::run( &socket, prompt, agent, name, base, root, worktree, template, file, command, vars, - no_auto, agent_args, plan, no_trigger, json, + no_auto, agent_args, plan, no_trigger, trigger, json, ) .await } @@ -771,6 +785,11 @@ async fn main() { TriggerAction::Delete { name, scope, json } => { commands::trigger::run_delete(&socket, &name, &scope, json).await } + TriggerAction::Assign { + agent_id, + trigger_name, + json, + } => commands::trigger::run_assign(&socket, &agent_id, &trigger_name, json).await, }, Commands::Gate { event, diff --git a/crates/pu-cli/src/output.rs b/crates/pu-cli/src/output.rs index 4ecdd1f..b527f98 100644 --- a/crates/pu-cli/src/output.rs +++ b/crates/pu-cli/src/output.rs @@ -253,6 +253,18 @@ pub fn print_response(response: &Response, json_mode: bool) -> Result<(), CliErr Response::RenameResult { agent_id, name } => { println!("Renamed agent {} to {}", agent_id.bold(), name.green()); } + Response::AssignTriggerResult { + agent_id, + trigger_name, + sequence_len, + } => { + println!( + "Assigned trigger {} to agent {} ({} steps)", + trigger_name.green(), + agent_id.bold(), + sequence_len + ); + } Response::CreateWorktreeResult { worktree_id } => { println!("Created worktree {}", worktree_id.bold()); } diff --git a/crates/pu-core/src/protocol.rs b/crates/pu-core/src/protocol.rs index a3f84df..9ad07ad 100644 --- a/crates/pu-core/src/protocol.rs +++ b/crates/pu-core/src/protocol.rs @@ -66,6 +66,8 @@ pub enum Request { plan_mode: bool, #[serde(default)] no_trigger: bool, + #[serde(default)] + trigger: Option, }, Status { project_root: String, @@ -127,6 +129,11 @@ pub enum Request { agent_id: String, name: String, }, + AssignTrigger { + project_root: String, + agent_id: String, + trigger_name: String, + }, CreateWorktree { project_root: String, #[serde(default)] @@ -598,6 +605,11 @@ pub enum Response { agent_id: String, name: String, }, + AssignTriggerResult { + agent_id: String, + trigger_name: String, + sequence_len: u32, + }, CreateWorktreeResult { worktree_id: String, }, @@ -836,6 +848,7 @@ mod tests { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }; let json = serde_json::to_string(&req).unwrap(); let parsed: Request = serde_json::from_str(&json).unwrap(); @@ -886,6 +899,7 @@ mod tests { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }; let json = serde_json::to_string(&req).unwrap(); let parsed: Request = serde_json::from_str(&json).unwrap(); @@ -920,6 +934,7 @@ mod tests { extra_args: vec!["--model".into(), "opus".into()], plan_mode: false, no_trigger: false, + trigger: None, }; let json = serde_json::to_string(&req).unwrap(); let parsed: Request = serde_json::from_str(&json).unwrap(); @@ -946,6 +961,7 @@ mod tests { extra_args: vec![], plan_mode: true, no_trigger: false, + trigger: None, }; let json = serde_json::to_string(&req).unwrap(); let parsed: Request = serde_json::from_str(&json).unwrap(); @@ -955,6 +971,64 @@ mod tests { } } + #[test] + fn given_spawn_request_without_trigger_should_default_to_none() { + let json = r#"{"type":"spawn","project_root":"/test","prompt":"fix bug"}"#; + let req: Request = serde_json::from_str(json).unwrap(); + match req { + Request::Spawn { trigger, .. } => assert!(trigger.is_none()), + _ => panic!("expected Spawn"), + } + } + + #[test] + fn given_spawn_request_with_trigger_should_round_trip() { + let req = Request::Spawn { + project_root: "/test".into(), + prompt: "fix".into(), + agent: "claude".into(), + name: None, + base: None, + root: true, + worktree: None, + command: None, + no_auto: false, + extra_args: vec![], + plan_mode: false, + no_trigger: false, + trigger: Some("review-bot".into()), + }; + let json = serde_json::to_string(&req).unwrap(); + let parsed: Request = serde_json::from_str(&json).unwrap(); + match parsed { + Request::Spawn { trigger, .. } => assert_eq!(trigger.unwrap(), "review-bot"), + _ => panic!("expected Spawn"), + } + } + + #[test] + fn given_assign_trigger_request_should_round_trip() { + let req = Request::AssignTrigger { + project_root: "/test".into(), + agent_id: "ag-abc123".into(), + trigger_name: "review-bot".into(), + }; + let json = serde_json::to_string(&req).unwrap(); + let parsed: Request = serde_json::from_str(&json).unwrap(); + match parsed { + Request::AssignTrigger { + project_root, + agent_id, + trigger_name, + } => { + assert_eq!(project_root, "/test"); + assert_eq!(agent_id, "ag-abc123"); + assert_eq!(trigger_name, "review-bot"); + } + _ => panic!("expected AssignTrigger"), + } + } + #[test] fn given_status_request_with_agent_id_should_round_trip() { let req = Request::Status { @@ -1528,6 +1602,29 @@ mod tests { } } + #[test] + fn given_assign_trigger_result_should_round_trip() { + let resp = Response::AssignTriggerResult { + agent_id: "ag-abc".into(), + trigger_name: "review-bot".into(), + sequence_len: 3, + }; + let json = serde_json::to_string(&resp).unwrap(); + let parsed: Response = serde_json::from_str(&json).unwrap(); + match parsed { + Response::AssignTriggerResult { + agent_id, + trigger_name, + sequence_len, + } => { + assert_eq!(agent_id, "ag-abc"); + assert_eq!(trigger_name, "review-bot"); + assert_eq!(sequence_len, 3); + } + _ => panic!("expected AssignTriggerResult"), + } + } + // --- DeleteWorktree round-trips --- #[test] diff --git a/crates/pu-engine/src/engine.rs b/crates/pu-engine/src/engine.rs index 11759a2..48175e2 100644 --- a/crates/pu-engine/src/engine.rs +++ b/crates/pu-engine/src/engine.rs @@ -43,6 +43,8 @@ struct SpawnParams { extra_args: Vec, plan_mode: bool, no_trigger: bool, + /// Name of trigger to bind (from --trigger flag) + trigger: Option, } struct SaveTriggerParams { @@ -241,6 +243,14 @@ impl Engine { agent_id, name, } => self.handle_rename(&project_root, &agent_id, &name).await, + Request::AssignTrigger { + project_root, + agent_id, + trigger_name, + } => { + self.handle_assign_trigger(&project_root, &agent_id, &trigger_name) + .await + } Request::GetConfig { project_root } => self.handle_get_config(&project_root).await, Request::UpdateAgentConfig { project_root, @@ -269,6 +279,7 @@ impl Engine { extra_args, plan_mode, no_trigger, + trigger, } => { self.handle_spawn(SpawnParams { project_root, @@ -283,6 +294,7 @@ impl Engine { extra_args, plan_mode, no_trigger, + trigger, }) .await } @@ -836,6 +848,7 @@ impl Engine { extra_args, plan_mode, no_trigger, + trigger: trigger_param, } = params; let root_path = Path::new(&project_root); @@ -1150,34 +1163,40 @@ impl Engine { // tries to attach — the session must already be in the map. self.sessions.lock().await.insert(agent_id.clone(), handle); - // Find the first idle trigger and bind this agent to it + // Bind trigger if explicitly specified via --trigger let (trigger_name, trigger_total) = if no_trigger { (None, None) - } else { + } else if let Some(ref name) = trigger_param { let pr = project_root.to_string(); + let name_clone = name.clone(); let found = tokio::task::spawn_blocking(move || { let triggers = pu_core::trigger_def::triggers_for_event( Path::new(&pr), &pu_core::trigger_def::TriggerEvent::AgentIdle, ); - if triggers.len() > 1 { - tracing::warn!( - "multiple agent_idle triggers found ({}), using first: {}", - triggers.len(), - triggers[0].name - ); - } - triggers.into_iter().next().map(|t| { - let len = t.sequence.len() as u32; - (t.name, len) - }) + triggers + .into_iter() + .find(|t| t.name == name_clone) + .map(|t| { + let len = t.sequence.len() as u32; + (t.name, len) + }) }) .await .unwrap_or(None); match found { - Some((name, total)) if total > 0 => (Some(name), Some(total)), - _ => (None, None), + Some((tname, total)) if total > 0 => (Some(tname), Some(total)), + Some(_) => { + tracing::warn!("trigger '{name}' has empty sequence, not binding"); + (None, None) + } + None => { + tracing::warn!("trigger '{name}' not found, not binding"); + (None, None) + } } + } else { + (None, None) }; // Update manifest @@ -1545,6 +1564,83 @@ impl Engine { } } + async fn handle_assign_trigger( + &self, + project_root: &str, + agent_id: &str, + trigger_name: &str, + ) -> Response { + let pr = project_root.to_string(); + let tn = trigger_name.to_string(); + + let trigger = tokio::task::spawn_blocking(move || { + pu_core::trigger_def::triggers_for_event( + Path::new(&pr), + &pu_core::trigger_def::TriggerEvent::AgentIdle, + ) + .into_iter() + .find(|t| t.name == tn) + }) + .await; + + let trigger = match trigger { + Ok(Some(t)) => t, + Ok(None) => { + return Response::Error { + code: "NOT_FOUND".into(), + message: format!("trigger '{}' not found", trigger_name), + }; + } + Err(e) => { + return Response::Error { + code: "INTERNAL_ERROR".into(), + message: format!("trigger lookup failed: {e}"), + }; + } + }; + + let sequence_len = trigger.sequence.len() as u32; + if sequence_len == 0 { + return Response::Error { + code: "INVALID_TRIGGER".into(), + message: format!("trigger '{}' has empty sequence", trigger_name), + }; + } + + let pr2 = project_root.to_string(); + let aid2 = agent_id.to_string(); + let tn2 = trigger_name.to_string(); + let result = tokio::task::spawn_blocking(move || { + manifest::update_manifest(Path::new(&pr2), |mut m| { + if let Some(agent) = m.find_agent_mut(&aid2) { + agent.trigger_name = Some(tn2.clone()); + agent.trigger_state = Some(pu_core::types::TriggerState::Active); + agent.trigger_seq_index = Some(0); + agent.trigger_total = Some(sequence_len); + agent.gate_attempts = Some(0); + } + m + }) + }) + .await; + + match result { + Ok(Ok(_)) => { + self.notify_status_change(project_root).await; + Response::AssignTriggerResult { + agent_id: agent_id.to_string(), + trigger_name: trigger_name.to_string(), + sequence_len, + } + } + Ok(Err(e)) => Self::error_response(&e), + Err(e) => Response::Error { + code: "INTERNAL_ERROR".into(), + message: format!("assign trigger task failed: {e}"), + }, + } + } + async fn handle_suspend(&self, project_root: &str, target: SuspendTarget) -> Response { let m = match self.read_manifest_async(project_root).await { Ok(m) => m, @@ -2926,6 +3022,7 @@ impl Engine { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await; @@ -2970,6 +3067,7 @@ impl Engine { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await; if let Response::SpawnResult { agent_id, .. } = resp { @@ -3718,8 +3816,7 @@ impl Engine { if let Some(ref inject_text) = action.inject { let resolved = pu_core::trigger_def::substitute_variables(inject_text, &trigger.variables); - let text_with_newline = format!("{resolved}\n"); - match self.inject_text(agent_id, &text_with_newline).await { + match self.inject_text(agent_id, &resolved).await { Ok(true) => {} // success, proceed to advance Ok(false) => { tracing::warn!(agent_id, "inject_text: session not found, marking failed"); @@ -3760,9 +3857,8 @@ impl Engine { } } - /// Inject text into an agent's PTY. Clones the fd under the lock, then drops - /// the lock before the potentially-blocking write. Returns `Ok(true)` on - /// success, `Ok(false)` if the session was not found. + /// Inject text into an agent's PTY using chunked typing + Enter submission. + /// Returns `Ok(true)` on success, `Ok(false)` if the session was not found. async fn inject_text(&self, agent_id: &str, text: &str) -> Result { let fd = { let sessions = self.sessions.lock().await; @@ -3770,7 +3866,9 @@ impl Engine { }; match fd { Some(fd) => { - self.pty_host.write_to_fd(&fd, text.as_bytes()).await?; + self.pty_host + .write_chunked_submit(&fd, text.as_bytes()) + .await?; Ok(true) } None => Ok(false), @@ -3853,6 +3951,7 @@ impl Engine { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await } else { @@ -3884,6 +3983,7 @@ impl Engine { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await } @@ -4367,6 +4467,7 @@ mod tests { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await; diff --git a/crates/pu-engine/src/test_helpers.rs b/crates/pu-engine/src/test_helpers.rs index 9925643..cb2d614 100644 --- a/crates/pu-engine/src/test_helpers.rs +++ b/crates/pu-engine/src/test_helpers.rs @@ -35,6 +35,7 @@ pub(crate) async fn init_and_spawn() -> (Engine, String, TempDir) { extra_args: vec![], plan_mode: false, no_trigger: false, + trigger: None, }) .await; From 0566c4bed37e662cbe48598e808cd75714e5a81d Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Mon, 9 Mar 2026 21:56:54 -0500 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20Address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20clippy,=20arg=20conflict,=20agent=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Inline format args in handle_assign_trigger (clippy uninlined_format_args) - Add conflicts_with between --trigger and --no-trigger flags - Validate agent exists before assigning trigger (return NOT_FOUND error instead of silent no-op) Co-Authored-By: Claude Opus 4.6 --- crates/pu-cli/src/main.rs | 2 +- crates/pu-engine/src/engine.rs | 29 +++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/pu-cli/src/main.rs b/crates/pu-cli/src/main.rs index a4abb02..5e78e88 100644 --- a/crates/pu-cli/src/main.rs +++ b/crates/pu-cli/src/main.rs @@ -66,7 +66,7 @@ enum Commands { #[arg(long)] no_trigger: bool, /// Bind an idle trigger to this agent (name of trigger in .pu/triggers/) - #[arg(long)] + #[arg(long, conflicts_with = "no_trigger")] trigger: Option, /// Output as JSON #[arg(long)] diff --git a/crates/pu-engine/src/engine.rs b/crates/pu-engine/src/engine.rs index 48175e2..72caada 100644 --- a/crates/pu-engine/src/engine.rs +++ b/crates/pu-engine/src/engine.rs @@ -1588,7 +1588,7 @@ impl Engine { Ok(None) => { return Response::Error { code: "NOT_FOUND".into(), - message: format!("trigger '{}' not found", trigger_name), + message: format!("trigger '{trigger_name}' not found"), }; } Err(e) => { @@ -1603,10 +1603,35 @@ impl Engine { if sequence_len == 0 { return Response::Error { code: "INVALID_TRIGGER".into(), - message: format!("trigger '{}' has empty sequence", trigger_name), + message: format!("trigger '{trigger_name}' has empty sequence"), }; } + // Verify the agent exists in the manifest before assigning + let pr_check = project_root.to_string(); + let aid_check = agent_id.to_string(); + let agent_exists = tokio::task::spawn_blocking(move || { + manifest::read_manifest(Path::new(&pr_check)) + .map(|m| m.find_agent(&aid_check).is_some()) + }) + .await; + + match agent_exists { + Ok(Ok(false)) | Ok(Err(_)) => { + return Response::Error { + code: "NOT_FOUND".into(), + message: format!("agent '{agent_id}' not found"), + }; + } + Err(e) => { + return Response::Error { + code: "INTERNAL_ERROR".into(), + message: format!("agent lookup failed: {e}"), + }; + } + Ok(Ok(true)) => {} // proceed + } + let pr2 = project_root.to_string(); let aid2 = agent_id.to_string(); let tn2 = trigger_name.to_string(); From f700027a21b09071b2cb31c7f6622f552db75dc6 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Mon, 9 Mar 2026 22:02:29 -0500 Subject: [PATCH 3/3] fix: Address CodeRabbit review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump PROTOCOL_VERSION 4 → 5 for AssignTrigger variants - Add AssignTrigger to project registration match - Make --trigger on spawn return error for invalid triggers Co-Authored-By: Claude Opus 4.6 --- crates/pu-core/src/protocol.rs | 4 ++-- crates/pu-engine/src/engine.rs | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/pu-core/src/protocol.rs b/crates/pu-core/src/protocol.rs index 9ad07ad..8046f09 100644 --- a/crates/pu-core/src/protocol.rs +++ b/crates/pu-core/src/protocol.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use crate::types::{AgentStatus, WorktreeEntry}; -pub const PROTOCOL_VERSION: u32 = 4; +pub const PROTOCOL_VERSION: u32 = 5; /// Serde helper: encode `Vec` as hex in JSON for binary PTY data. mod hex_bytes { @@ -1336,7 +1336,7 @@ mod tests { #[test] fn given_protocol_version_should_be_current() { - assert_eq!(PROTOCOL_VERSION, 4); + assert_eq!(PROTOCOL_VERSION, 5); } // --- GridCommand round-trips --- diff --git a/crates/pu-engine/src/engine.rs b/crates/pu-engine/src/engine.rs index 72caada..c908f3a 100644 --- a/crates/pu-engine/src/engine.rs +++ b/crates/pu-engine/src/engine.rs @@ -229,7 +229,8 @@ impl Engine { | Request::Diff { project_root, .. } | Request::GetConfig { project_root } | Request::UpdateAgentConfig { project_root, .. } - | Request::Pulse { project_root, .. } => { + | Request::Pulse { project_root, .. } + | Request::AssignTrigger { project_root, .. } => { self.register_project(project_root); } _ => {} @@ -1187,12 +1188,16 @@ impl Engine { match found { Some((tname, total)) if total > 0 => (Some(tname), Some(total)), Some(_) => { - tracing::warn!("trigger '{name}' has empty sequence, not binding"); - (None, None) + return Response::Error { + code: "INVALID_TRIGGER".into(), + message: format!("trigger '{name}' has empty sequence"), + }; } None => { - tracing::warn!("trigger '{name}' not found, not binding"); - (None, None) + return Response::Error { + code: "NOT_FOUND".into(), + message: format!("trigger '{name}' not found"), + }; } } } else {