diff --git a/crates/jcode-app-core/src/server/client_lifecycle.rs b/crates/jcode-app-core/src/server/client_lifecycle.rs index e450e8c61d..9ea3fa9913 100644 --- a/crates/jcode-app-core/src/server/client_lifecycle.rs +++ b/crates/jcode-app-core/src/server/client_lifecycle.rs @@ -99,6 +99,13 @@ fn initial_subscribe_working_dir(request: &Request) -> std::result::Result Vec<(String, String)> { + match request { + Request::Subscribe { terminal_env, .. } => terminal_env.clone(), + _ => Vec::new(), + } +} + struct ProcessingMessage { id: u64, content: String, @@ -468,6 +475,7 @@ pub(super) async fn handle_client( return Ok(()); } }; + let mut active_terminal_env = subscribe_terminal_env(&initial_request); // Per-client state let mut client_is_processing = false; @@ -493,11 +501,15 @@ pub(super) async fn handle_client( // Create a new session for this client let t0 = std::time::Instant::now(); - let mut new_agent = Agent::new_with_initial_working_dir( - Arc::clone(&provider), - registry.clone(), - Some(&initial_working_dir), - ); + let mut new_agent = + crate::hooks::with_client_terminal_env(active_terminal_env.clone(), async { + Agent::new_with_initial_working_dir( + Arc::clone(&provider), + registry.clone(), + Some(&initial_working_dir), + ) + }) + .await; let agent_new_ms = t0.elapsed().as_millis(); new_agent.set_memory_enabled(crate::config::config().features.memory); @@ -525,7 +537,7 @@ pub(super) async fn handle_client( last_seen: connected_at, is_processing: false, current_tool_name: None, - terminal_env: Vec::new(), + terminal_env: active_terminal_env.clone(), disconnect_tx: disconnect_tx.clone(), }, ); @@ -1132,6 +1144,7 @@ pub(super) async fn handle_client( &agent, &client_event_tx, &processing_done_tx, + active_terminal_env.clone(), &SwarmStatusRefs { members: &swarm_members, swarms_by_id: &swarms_by_id, @@ -1209,28 +1222,31 @@ pub(super) async fn handle_client( ) { continue; } - handle_clear_session( - id, - client_selfdev, - &mut client_session_id, - &client_connection_id, - &agent, - &provider, - ®istry, - &sessions, - &shutdown_signals, - &soft_interrupt_queues, - &client_connections, - &swarm_members, - &swarms_by_id, - &file_touch, - &channel_subscriptions, - &channel_subscriptions_by_session, - &swarm_plans, - &event_history, - &event_counter, - &swarm_event_tx, - &client_event_tx, + crate::hooks::with_client_terminal_env( + active_terminal_env.clone(), + handle_clear_session( + id, + client_selfdev, + &mut client_session_id, + &client_connection_id, + &agent, + &provider, + ®istry, + &sessions, + &shutdown_signals, + &soft_interrupt_queues, + &client_connections, + &swarm_members, + &swarms_by_id, + &file_touch, + &channel_subscriptions, + &channel_subscriptions_by_session, + &swarm_plans, + &event_history, + &event_counter, + &swarm_event_tx, + &client_event_tx, + ), ) .await; session_control = refresh_session_control_handle( @@ -1404,57 +1420,60 @@ pub(super) async fn handle_client( }); continue; } + // An empty snapshot is authoritative too. It must clear all + // terminal vars inherited by the daemon rather than retaining + // a prior pane's values. + active_terminal_env = terminal_env; current_client_instance_id = client_instance_id.clone(); { let mut connections = client_connections.write().await; if let Some(info) = connections.get_mut(&client_connection_id) { info.client_instance_id = client_instance_id.clone(); - // Record the client's terminal env so spawn/focus hooks - // target the client's terminal, not the server's stale - // startup env (#405). Only overwrite when the client sent - // something, so reconnects without env don't clobber it. - if !terminal_env.is_empty() { - info.terminal_env = terminal_env.clone(); - } + // Record the authoritative snapshot even when it is + // empty so stale pane variables are not retained. + info.terminal_env = active_terminal_env.clone(); } } if let Some(target_session_id) = target_session_id { if crate::session::session_exists(&target_session_id) { let pre_resume_session_id = client_session_id.clone(); - agent = handle_resume_session( - id, - target_session_id.clone(), - subscribe_working_dir.as_deref(), - client_instance_id.as_deref(), - client_has_local_history, - allow_session_takeover, - &mut client_selfdev, - &mut client_session_id, - &client_connection_id, - &agent, - &provider, - ®istry, - &sessions, - &shutdown_signals, - &soft_interrupt_queues, - &client_connections, - &client_debug_state, - &swarm_members, - &swarms_by_id, - &file_touch, - &channel_subscriptions, - &channel_subscriptions_by_session, - &swarm_plans, - &swarm_coordinators, - &client_count, - &writer, - &server_name, - &server_icon, - &client_event_tx, - &mcp_pool, - &event_history, - &event_counter, - &swarm_event_tx, + agent = crate::hooks::with_client_terminal_env( + active_terminal_env.clone(), + handle_resume_session( + id, + target_session_id.clone(), + subscribe_working_dir.as_deref(), + client_instance_id.as_deref(), + client_has_local_history, + allow_session_takeover, + &mut client_selfdev, + &mut client_session_id, + &client_connection_id, + &agent, + &provider, + ®istry, + &sessions, + &shutdown_signals, + &soft_interrupt_queues, + &client_connections, + &client_debug_state, + &swarm_members, + &swarms_by_id, + &file_touch, + &channel_subscriptions, + &channel_subscriptions_by_session, + &swarm_plans, + &swarm_coordinators, + &client_count, + &writer, + &server_name, + &server_icon, + &client_event_tx, + &mcp_pool, + &event_history, + &event_counter, + &swarm_event_tx, + ), ) .await?; session_control = refresh_session_control_handle( @@ -2753,28 +2772,31 @@ pub(super) async fn handle_client( } } - cleanup_client_connection( - &sessions, - &client_session_id, - client_is_processing, - &mut processing_task, - event_handle, - &swarm_members, - &swarms_by_id, - &swarm_coordinators, - &swarm_plans, - &file_touch, - &channel_subscriptions, - &channel_subscriptions_by_session, - &client_debug_state, - &client_debug_id, - &client_connections, - &client_connection_id, - &shutdown_signals, - &soft_interrupt_queues, - &event_history, - &event_counter, - &swarm_event_tx, + crate::hooks::with_client_terminal_env( + active_terminal_env, + cleanup_client_connection( + &sessions, + &client_session_id, + client_is_processing, + &mut processing_task, + event_handle, + &swarm_members, + &swarms_by_id, + &swarm_coordinators, + &swarm_plans, + &file_touch, + &channel_subscriptions, + &channel_subscriptions_by_session, + &client_debug_state, + &client_debug_id, + &client_connections, + &client_connection_id, + &shutdown_signals, + &soft_interrupt_queues, + &event_history, + &event_counter, + &swarm_event_tx, + ), ) .await?; Ok(()) @@ -2818,6 +2840,7 @@ async fn start_processing_message( agent: &Arc>, client_event_tx: &mpsc::UnboundedSender, processing_done_tx: &mpsc::UnboundedSender<(u64, Result<()>, Option)>, + client_terminal_env: Vec<(String, String)>, swarm: &SwarmStatusRefs<'_>, ) { let ProcessingMessage { @@ -2888,32 +2911,34 @@ async fn start_processing_message( crate::logging::info(&format!("Processing message id={} spawning task", id)); *state.task = Some(tokio::spawn(async move { let event_tx = tx.clone(); - let result = match std::panic::AssertUnwindSafe(process_message_streaming_mpsc( - agent, - &content, - images, - system_reminder, - event_tx, - )) - .catch_unwind() - .await - { - Ok(result) => result, - Err(panic_payload) => { - let msg = if let Some(text) = panic_payload.downcast_ref::<&str>() { - text.to_string() - } else if let Some(text) = panic_payload.downcast_ref::() { - text.clone() - } else { - "unknown panic".to_string() - }; - crate::logging::error(&format!( - "Processing task PANICKED for message id={}: {}", - id, msg - )); - Err(anyhow::anyhow!("Processing task panicked: {}", msg)) - } - }; + let result = + match std::panic::AssertUnwindSafe(process_message_streaming_mpsc_with_terminal_env( + agent, + &content, + images, + system_reminder, + client_terminal_env, + event_tx, + )) + .catch_unwind() + .await + { + Ok(result) => result, + Err(panic_payload) => { + let msg = if let Some(text) = panic_payload.downcast_ref::<&str>() { + text.to_string() + } else if let Some(text) = panic_payload.downcast_ref::() { + text.clone() + } else { + "unknown panic".to_string() + }; + crate::logging::error(&format!( + "Processing task PANICKED for message id={}: {}", + id, msg + )); + Err(anyhow::anyhow!("Processing task panicked: {}", msg)) + } + }; match &result { Ok(()) => crate::logging::info(&format!( "Processing task completed OK for message id={}", @@ -3255,6 +3280,24 @@ pub(super) async fn process_message_streaming_mpsc( result } +/// Process a client-originated turn with that client's terminal identity. The +/// task-local scope flows through turn, pre-tool, and post-tool hooks without +/// mutating process-global environment shared by other client tasks. +async fn process_message_streaming_mpsc_with_terminal_env( + agent: Arc>, + content: &str, + images: Vec<(String, String)>, + system_reminder: Option, + client_terminal_env: Vec<(String, String)>, + event_tx: tokio::sync::mpsc::UnboundedSender, +) -> Result<()> { + crate::hooks::with_client_terminal_env( + client_terminal_env, + process_message_streaming_mpsc(agent, content, images, system_reminder, event_tx), + ) + .await +} + #[cfg(test)] #[path = "client_lifecycle_tests.rs"] mod tests; diff --git a/crates/jcode-app-core/src/server/client_lifecycle_tests.rs b/crates/jcode-app-core/src/server/client_lifecycle_tests.rs index 0afa2177d1..1893c61156 100644 --- a/crates/jcode-app-core/src/server/client_lifecycle_tests.rs +++ b/crates/jcode-app-core/src/server/client_lifecycle_tests.rs @@ -906,6 +906,7 @@ fn reload_starting_rejects_new_turn_without_spawning_processing_task() { &agent, &client_event_tx, &processing_done_tx, + Vec::new(), &SwarmStatusRefs { members: &swarm_members, swarms_by_id: &swarms_by_id, @@ -1005,6 +1006,7 @@ async fn client_initiated_turn_fans_out_stream_and_terminal_events_to_live_attac &agent, &origin_tx, &processing_done_tx, + Vec::new(), &SwarmStatusRefs { members: &swarm_members, swarms_by_id: &swarms_by_id, @@ -1128,6 +1130,7 @@ fn accepted_reload_recovery_continuation_marks_intent_delivered() -> anyhow::Res &agent, &client_event_tx, &processing_done_tx, + Vec::new(), &SwarmStatusRefs { members: &swarm_members, swarms_by_id: &swarms_by_id, @@ -1226,6 +1229,7 @@ fn reload_starting_rejects_new_turns_for_multiple_sessions() { &agent, &client_event_tx, &processing_done_tx, + Vec::new(), &SwarmStatusRefs { members: &swarm_members, swarms_by_id: &swarms_by_id, diff --git a/crates/jcode-base/src/config/env_overrides.rs b/crates/jcode-base/src/config/env_overrides.rs index e8aed3da38..e8f9e52928 100644 --- a/crates/jcode-base/src/config/env_overrides.rs +++ b/crates/jcode-base/src/config/env_overrides.rs @@ -431,22 +431,34 @@ impl Config { } // Lifecycle hooks. Empty env values disable config-file hooks. - fn hook_env_override(slot: &mut Option, key: &str) { + fn hook_env_override(hooks: &mut HooksConfig, event: &str, key: &str) { if let Ok(v) = std::env::var(key) { let trimmed = v.trim(); - *slot = if trimmed.is_empty() { - None + let commands = if trimmed.is_empty() { + Vec::new() + } else if trimmed.starts_with('[') { + // Environment variables remain backward-compatible plain + // command strings. A TOML/JSON-style string array opts in + // to multiple direct command invocations. + #[derive(serde::Deserialize)] + struct Commands { + commands: Vec, + } + toml::from_str::(&format!("commands = {trimmed}")) + .map(|parsed| parsed.commands) + .unwrap_or_else(|_| vec![trimmed.to_string()]) } else { - Some(trimmed.to_string()) + vec![trimmed.to_string()] }; + hooks.set_commands(event, commands); } } - hook_env_override(&mut self.hooks.turn_start, "JCODE_HOOK_TURN_START"); - hook_env_override(&mut self.hooks.turn_end, "JCODE_HOOK_TURN_END"); - hook_env_override(&mut self.hooks.session_start, "JCODE_HOOK_SESSION_START"); - hook_env_override(&mut self.hooks.session_end, "JCODE_HOOK_SESSION_END"); - hook_env_override(&mut self.hooks.pre_tool, "JCODE_HOOK_PRE_TOOL"); - hook_env_override(&mut self.hooks.post_tool, "JCODE_HOOK_POST_TOOL"); + hook_env_override(&mut self.hooks, "turn_start", "JCODE_HOOK_TURN_START"); + hook_env_override(&mut self.hooks, "turn_end", "JCODE_HOOK_TURN_END"); + hook_env_override(&mut self.hooks, "session_start", "JCODE_HOOK_SESSION_START"); + hook_env_override(&mut self.hooks, "session_end", "JCODE_HOOK_SESSION_END"); + hook_env_override(&mut self.hooks, "pre_tool", "JCODE_HOOK_PRE_TOOL"); + hook_env_override(&mut self.hooks, "post_tool", "JCODE_HOOK_POST_TOOL"); if let Ok(v) = std::env::var("JCODE_HOOK_PRE_TOOL_TIMEOUT_MS") { if let Ok(parsed) = v.trim().parse::() { self.hooks.pre_tool_timeout_ms = parsed; diff --git a/crates/jcode-base/src/config_tests.rs b/crates/jcode-base/src/config_tests.rs index 4d7fa184b2..023effd0e7 100644 --- a/crates/jcode-base/src/config_tests.rs +++ b/crates/jcode-base/src/config_tests.rs @@ -244,7 +244,26 @@ fn hooks_config_defaults_and_parses_from_toml() { assert_eq!(cfg.hooks.turn_start.as_deref(), Some("notify-start")); assert_eq!(cfg.hooks.turn_end.as_deref(), Some("notify-turn")); assert_eq!(cfg.hooks.pre_tool.as_deref(), Some("~/bin/policy")); + assert_eq!(cfg.hooks.commands("turn_end"), vec!["notify-turn"]); assert_eq!(cfg.hooks.pre_tool_timeout_ms, 1500); + + let cfg: Config = toml::from_str( + "[hooks]\nturn_end = [\"notify-one --direct\", \"notify-two 'quoted arg'\"]\npre_tool = [\"policy-a\", \"policy-b\"]\n", + ) + .expect("hook command arrays should parse"); + assert_eq!( + cfg.hooks.commands("turn_end"), + vec!["notify-one --direct", "notify-two 'quoted arg'"] + ); + assert_eq!(cfg.hooks.turn_end.as_deref(), Some("notify-one --direct")); + assert_eq!(cfg.hooks.commands("pre_tool"), vec!["policy-a", "policy-b"]); + + let serialized = toml::to_string(&cfg).expect("hook command arrays should serialize"); + let round_trip: Config = toml::from_str(&serialized).expect("serialized hooks should parse"); + assert_eq!( + round_trip.hooks.commands("turn_end"), + cfg.hooks.commands("turn_end") + ); } #[test] @@ -260,6 +279,17 @@ fn test_env_override_lifecycle_hooks() { assert_eq!(cfg.hooks.turn_end.as_deref(), Some("my-notifier --fast")); assert_eq!(cfg.hooks.pre_tool_timeout_ms, 250); + crate::env::set_var( + "JCODE_HOOK_TURN_END", + r#"["notify-one --direct", "notify-two 'quoted arg'"]"#, + ); + let mut cfg = Config::default(); + cfg.apply_env_overrides(); + assert_eq!( + cfg.hooks.commands("turn_end"), + vec!["notify-one --direct", "notify-two 'quoted arg'"] + ); + // Empty env value disables a config-file hook. crate::env::set_var("JCODE_HOOK_TURN_END", " "); let mut cfg = Config::default(); diff --git a/crates/jcode-base/src/hooks.rs b/crates/jcode-base/src/hooks.rs index 34d0471ac4..e90bbb21c9 100644 --- a/crates/jcode-base/src/hooks.rs +++ b/crates/jcode-base/src/hooks.rs @@ -23,6 +23,13 @@ use std::path::PathBuf; +tokio::task_local! { + /// Terminal identity for the client whose request is currently executing. + /// Tokio task-local storage keeps concurrent server clients isolated even + /// though they share one daemon process and process environment. + static CLIENT_TERMINAL_ENV: Vec<(String, String)>; +} + /// Maximum bytes of JSON payload exported via `JCODE_HOOK_PAYLOAD`. const PAYLOAD_ENV_LIMIT: usize = 16 * 1024; /// Maximum bytes of tool input JSON exported to the pre_tool gate. @@ -78,28 +85,43 @@ impl HookEvent { /// The configured command for `event`, if any. pub fn hook_command(event: &str) -> Option { + hook_commands(event).into_iter().next() +} + +/// Every configured command for `event`, in declaration order. +pub fn hook_commands(event: &str) -> Vec { if hooks_suppressed() { - return None; + return Vec::new(); } let hooks = &crate::config::config().hooks; - let raw = match event { - "turn_start" => hooks.turn_start.as_deref(), - "turn_end" => hooks.turn_end.as_deref(), - "session_start" => hooks.session_start.as_deref(), - "session_end" => hooks.session_end.as_deref(), - "pre_tool" => hooks.pre_tool.as_deref(), - "post_tool" => hooks.post_tool.as_deref(), - _ => None, - }; - raw.map(str::trim) + hooks + .commands(event) + .into_iter() + .map(str::trim) .filter(|command| !command.is_empty()) .map(str::to_string) + .collect() } /// Whether a hook is configured for `event`. Cheap; used by hot paths to /// skip payload construction entirely when no hook is set. pub fn hook_configured(event: &str) -> bool { - hook_command(event).is_some() + !hook_commands(event).is_empty() +} + +/// Run a future with an authoritative client terminal environment. Nested hook +/// calls inherit this value, while concurrent client futures remain isolated. +pub async fn with_client_terminal_env(env: Vec<(String, String)>, future: F) -> F::Output +where + F: std::future::Future, +{ + CLIENT_TERMINAL_ENV.scope(env, future).await +} + +fn current_client_terminal_env() -> Vec<(String, String)> { + CLIENT_TERMINAL_ENV + .try_with(Clone::clone) + .unwrap_or_default() } /// True when running inside a hook process (recursion guard). @@ -183,6 +205,7 @@ fn build_hook_process( { cmd.current_dir(cwd); } + crate::terminal_launch::apply_client_terminal_env(&mut cmd, ¤t_client_terminal_env()); apply_event_env(&mut cmd, event); Ok(cmd) } @@ -192,28 +215,31 @@ fn build_hook_process( /// Detached and fire-and-forget: failures are logged, never propagated, and /// the hook process cannot block the agent. pub fn dispatch_observer(event: HookEvent) { - let Some(command_line) = hook_command(event.event) else { + let command_lines = hook_commands(event.event); + if command_lines.is_empty() { return; - }; + } let event_name = event.event; - match build_hook_process(&command_line, &event) { - Ok(mut cmd) => { - cmd.stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()); - match crate::platform::spawn_detached(&mut cmd) { - Ok(_) => crate::logging::debug(&format!( - "Hook '{event_name}' dispatched to '{command_line}' (session={:?})", - event.session_id - )), - Err(error) => crate::logging::warn(&format!( - "Hook '{event_name}' command '{command_line}' failed to start: {error}" - )), + for command_line in command_lines { + match build_hook_process(&command_line, &event) { + Ok(mut cmd) => { + cmd.stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()); + match crate::platform::spawn_detached(&mut cmd) { + Ok(_) => crate::logging::debug(&format!( + "Hook '{event_name}' dispatched to '{command_line}' (session={:?})", + event.session_id + )), + Err(error) => crate::logging::warn(&format!( + "Hook '{event_name}' command '{command_line}' failed to start: {error}" + )), + } } + Err(error) => crate::logging::warn(&format!( + "Hook '{event_name}' command '{command_line}' is invalid: {error}" + )), } - Err(error) => crate::logging::warn(&format!( - "Hook '{event_name}' command '{command_line}' is invalid: {error}" - )), } } @@ -231,9 +257,10 @@ pub async fn run_pre_tool_gate( tool_name: &str, tool_input_json: &str, ) -> GateDecision { - let Some(command_line) = hook_command("pre_tool") else { + let command_lines = hook_commands("pre_tool"); + if command_lines.is_empty() { return GateDecision::Allow; - }; + } let mut event = HookEvent::new("pre_tool") .session_id(session_id) @@ -246,11 +273,34 @@ pub async fn run_pre_tool_gate( event = event.cwd(cwd); } - let std_cmd = match build_hook_process(&command_line, &event) { + for command_line in command_lines { + let decision = run_pre_tool_command( + &command_line, + &event, + session_id, + tool_name, + tool_input_json, + ) + .await; + if matches!(decision, GateDecision::Block { .. }) { + return decision; + } + } + GateDecision::Allow +} + +async fn run_pre_tool_command( + command_line: &str, + event: &HookEvent, + session_id: &str, + tool_name: &str, + tool_input_json: &str, +) -> GateDecision { + let std_cmd = match build_hook_process(command_line, event) { Ok(cmd) => cmd, Err(error) => { crate::logging::warn(&format!( - "Hook 'pre_tool' command '{command_line}' is invalid: {error} (allowing tool call)" + "Hook 'pre_tool' command '{command_line}' is invalid: {error} (continuing)" )); return GateDecision::Allow; } @@ -266,7 +316,7 @@ pub async fn run_pre_tool_gate( Ok(child) => child, Err(error) => { crate::logging::warn(&format!( - "Hook 'pre_tool' command '{command_line}' failed to start: {error} (allowing tool call)" + "Hook 'pre_tool' command '{command_line}' failed to start: {error} (continuing)" )); return GateDecision::Allow; } @@ -285,13 +335,13 @@ pub async fn run_pre_tool_gate( Ok(Ok(output)) => output, Ok(Err(error)) => { crate::logging::warn(&format!( - "Hook 'pre_tool' command '{command_line}' failed: {error} (allowing tool call)" + "Hook 'pre_tool' command '{command_line}' failed: {error} (continuing)" )); return GateDecision::Allow; } Err(_elapsed) => { crate::logging::warn(&format!( - "Hook 'pre_tool' command '{command_line}' timed out after {}ms (allowing tool call)", + "Hook 'pre_tool' command '{command_line}' timed out after {}ms (continuing)", timeout.as_millis() )); return GateDecision::Allow; @@ -315,7 +365,7 @@ pub async fn run_pre_tool_gate( } other => { crate::logging::warn(&format!( - "Hook 'pre_tool' command '{command_line}' exited with {other:?} (expected 0=allow or 2=block; allowing tool call)" + "Hook 'pre_tool' command '{command_line}' exited with {other:?} (expected 0=allow or 2=block; continuing)" )); GateDecision::Allow } @@ -422,6 +472,46 @@ mod tests { } } + #[cfg(unix)] + #[tokio::test] + async fn pre_tool_gate_runs_command_array_until_one_blocks() { + let _guard = crate::storage::lock_test_env(); + let temp = tempfile::TempDir::new().expect("temp dir"); + let marker = temp.path().join("first-ran.txt"); + let allow = write_executable_script( + temp.path(), + "first-allow.sh", + &format!( + "#!/bin/sh\nprintf ran > {}\nexit 0\n", + crate::terminal_launch::sh_escape(&marker.to_string_lossy()) + ), + ); + let block = write_executable_script( + temp.path(), + "second-block.sh", + "#!/bin/sh\necho 'blocked by second policy' >&2\nexit 2\n", + ); + let commands = serde_json::to_string(&vec![ + allow.to_string_lossy().into_owned(), + block.to_string_lossy().into_owned(), + ]) + .expect("serialize hook command array"); + let _env = gate_test_config(&commands, 5000); + + let decision = run_pre_tool_gate("ses_multi", None, "bash", "{}").await; + + assert_eq!( + decision, + GateDecision::Block { + reason: "blocked by second policy".to_string() + } + ); + assert_eq!( + std::fs::read_to_string(marker).expect("first policy should execute"), + "ran" + ); + } + #[cfg(unix)] #[tokio::test] async fn pre_tool_gate_fails_open_on_timeout_and_odd_exits() { @@ -514,4 +604,107 @@ mod tests { } assert_eq!(recorded, "turn_end|ses_obs|ok|1"); } + + #[cfg(unix)] + #[test] + fn observer_dispatch_runs_every_configured_command_directly() { + let _guard = crate::storage::lock_test_env(); + let temp = tempfile::TempDir::new().expect("temp dir"); + let first_record = temp.path().join("first.txt"); + let second_record = temp.path().join("second.txt"); + let first = write_executable_script( + temp.path(), + "first.sh", + &format!( + "#!/bin/sh\nprintf first > {}\n", + crate::terminal_launch::sh_escape(&first_record.to_string_lossy()) + ), + ); + let second = write_executable_script( + temp.path(), + "second.sh", + &format!( + "#!/bin/sh\nprintf second > {}\n", + crate::terminal_launch::sh_escape(&second_record.to_string_lossy()) + ), + ); + let prev = std::env::var_os("JCODE_HOOK_TURN_START"); + let commands = serde_json::to_string(&vec![ + first.to_string_lossy().into_owned(), + second.to_string_lossy().into_owned(), + ]) + .expect("serialize hook command array"); + crate::env::set_var("JCODE_HOOK_TURN_START", commands); + + dispatch_observer(HookEvent::new("turn_start").session_id("ses_multi")); + + for _ in 0..100 { + if first_record.exists() && second_record.exists() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + match prev { + Some(value) => crate::env::set_var("JCODE_HOOK_TURN_START", value), + None => crate::env::remove_var("JCODE_HOOK_TURN_START"), + } + assert_eq!(std::fs::read_to_string(first_record).unwrap(), "first"); + assert_eq!(std::fs::read_to_string(second_record).unwrap(), "second"); + } + + #[cfg(unix)] + #[tokio::test] + async fn hook_process_replaces_daemon_terminal_env_with_client_snapshot() { + let _guard = crate::storage::lock_test_env(); + let temp = tempfile::TempDir::new().expect("temp dir"); + let script = write_executable_script( + temp.path(), + "env.sh", + "#!/bin/sh\nprintf '%s|%s|%s|%s' \"$TMUX_PANE\" \"$HERDR_PANE_ID\" \"$JCODE_CLIENT_TMUX_PANE\" \"$JCODE_CLIENT_HERDR_PANE_ID\"\n", + ); + let previous_tmux = std::env::var_os("TMUX_PANE"); + let previous_herdr = std::env::var_os("HERDR_PANE_ID"); + crate::env::set_var("TMUX_PANE", "daemon-pane"); + crate::env::set_var("HERDR_PANE_ID", "daemon-herdr"); + + let run_for_pane = |tmux: &'static str, herdr: &'static str| { + let script = script.clone(); + with_client_terminal_env( + vec![ + ("TMUX_PANE".to_string(), tmux.to_string()), + ("HERDR_PANE_ID".to_string(), herdr.to_string()), + ], + async move { + tokio::task::yield_now().await; + build_hook_process(&script.to_string_lossy(), &HookEvent::new("turn_start")) + .expect("hook command") + .output() + .expect("run hook") + }, + ) + }; + let (first_output, second_output) = tokio::join!( + run_for_pane("client-pane-a", "herdr-pane-a"), + run_for_pane("client-pane-b", "herdr-pane-b") + ); + + match previous_tmux { + Some(value) => crate::env::set_var("TMUX_PANE", value), + None => crate::env::remove_var("TMUX_PANE"), + } + match previous_herdr { + Some(value) => crate::env::set_var("HERDR_PANE_ID", value), + None => crate::env::remove_var("HERDR_PANE_ID"), + } + assert!(first_output.status.success()); + assert!(second_output.status.success()); + assert_eq!( + String::from_utf8_lossy(&first_output.stdout), + "client-pane-a|herdr-pane-a|client-pane-a|herdr-pane-a" + ); + assert_eq!( + String::from_utf8_lossy(&second_output.stdout), + "client-pane-b|herdr-pane-b|client-pane-b|herdr-pane-b" + ); + } } diff --git a/crates/jcode-base/src/terminal_launch.rs b/crates/jcode-base/src/terminal_launch.rs index ef9bf3616d..f2c09dbc38 100644 --- a/crates/jcode-base/src/terminal_launch.rs +++ b/crates/jcode-base/src/terminal_launch.rs @@ -1,8 +1,8 @@ use anyhow::Result; pub use jcode_terminal_launch::{ - SpawnAttempt, TerminalCommand, build_hook_spawn_command, detected_resume_terminal, expand_home, - parse_hook_command, resume_terminal_candidates, sh_escape, shell_command, - snapshot_client_terminal_env, spawn_command_in_new_terminal_with, + SpawnAttempt, TerminalCommand, apply_client_terminal_env, build_hook_spawn_command, + detected_resume_terminal, expand_home, parse_hook_command, resume_terminal_candidates, + sh_escape, shell_command, snapshot_client_terminal_env, spawn_command_in_new_terminal_with, }; use std::path::Path; diff --git a/crates/jcode-config-types/src/lib.rs b/crates/jcode-config-types/src/lib.rs index 902ca0f87c..04dccb9e9f 100644 --- a/crates/jcode-config-types/src/lib.rs +++ b/crates/jcode-config-types/src/lib.rs @@ -766,8 +766,7 @@ pub struct TerminalConfig { /// failures only logged. `pre_tool` is a gate: jcode waits for it and exit /// code 2 blocks the tool call (stderr becomes the error shown to the model); /// exit 0 allows; anything else fails open. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(default)] +#[derive(Debug, Clone)] pub struct HooksConfig { /// Runs when an agent turn begins (after the user message is added and /// before the model starts generating). Fires before the first `pre_tool`, @@ -798,6 +797,173 @@ pub struct HooksConfig { /// Max milliseconds to wait for the pre_tool gate before failing open /// (default: 5000). Env override: JCODE_HOOK_PRE_TOOL_TIMEOUT_MS. pub pre_tool_timeout_ms: u64, + // `Option` above is the original public API. Keep it populated with + // the first configured command while retaining an explicitly configured + // TOML/env array here. This lets existing callers keep reading or assigning + // the legacy fields without preventing native one-process-per-command + // execution. + turn_start_commands: Option>, + turn_end_commands: Option>, + session_start_commands: Option>, + session_end_commands: Option>, + pre_tool_commands: Option>, + post_tool_commands: Option>, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +enum HookCommandValue { + One(String), + Many(Vec), +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(default)] +struct HooksConfigWire { + turn_start: Option, + turn_end: Option, + session_start: Option, + session_end: Option, + pre_tool: Option, + post_tool: Option, + pre_tool_timeout_ms: u64, +} + +impl Default for HooksConfigWire { + fn default() -> Self { + Self { + turn_start: None, + turn_end: None, + session_start: None, + session_end: None, + pre_tool: None, + post_tool: None, + pre_tool_timeout_ms: 5000, + } + } +} + +impl HooksConfig { + fn split_value(value: Option) -> (Option, Option>) { + match value { + Some(HookCommandValue::One(command)) => (Some(command), None), + Some(HookCommandValue::Many(commands)) => (commands.first().cloned(), Some(commands)), + None => (None, None), + } + } + + fn command_value<'a>( + legacy: &'a Option, + commands: &'a Option>, + ) -> Option> { + match commands { + Some(commands) => Some(HookCommandValueRef::Many(commands)), + None => legacy.as_deref().map(HookCommandValueRef::One), + } + } + + /// Return every configured command for a lifecycle event in declaration + /// order. Legacy string fields are treated as a one-command list. + pub fn commands(&self, event: &str) -> Vec<&str> { + let (legacy, commands) = match event { + "turn_start" => (&self.turn_start, &self.turn_start_commands), + "turn_end" => (&self.turn_end, &self.turn_end_commands), + "session_start" => (&self.session_start, &self.session_start_commands), + "session_end" => (&self.session_end, &self.session_end_commands), + "pre_tool" => (&self.pre_tool, &self.pre_tool_commands), + "post_tool" => (&self.post_tool, &self.post_tool_commands), + _ => return Vec::new(), + }; + match commands { + Some(commands) => commands.iter().map(String::as_str).collect(), + None => legacy.iter().map(String::as_str).collect(), + } + } + + /// Replace an event's commands. A zero-length list disables the hook and a + /// one-length list is stored through the legacy `Option` field. + pub fn set_commands(&mut self, event: &str, commands: Vec) { + let (legacy, all) = match event { + "turn_start" => (&mut self.turn_start, &mut self.turn_start_commands), + "turn_end" => (&mut self.turn_end, &mut self.turn_end_commands), + "session_start" => (&mut self.session_start, &mut self.session_start_commands), + "session_end" => (&mut self.session_end, &mut self.session_end_commands), + "pre_tool" => (&mut self.pre_tool, &mut self.pre_tool_commands), + "post_tool" => (&mut self.post_tool, &mut self.post_tool_commands), + _ => return, + }; + *legacy = commands.first().cloned(); + *all = (commands.len() > 1).then_some(commands); + } +} + +impl<'de> Deserialize<'de> for HooksConfig { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let wire = HooksConfigWire::deserialize(deserializer)?; + let (turn_start, turn_start_commands) = Self::split_value(wire.turn_start); + let (turn_end, turn_end_commands) = Self::split_value(wire.turn_end); + let (session_start, session_start_commands) = Self::split_value(wire.session_start); + let (session_end, session_end_commands) = Self::split_value(wire.session_end); + let (pre_tool, pre_tool_commands) = Self::split_value(wire.pre_tool); + let (post_tool, post_tool_commands) = Self::split_value(wire.post_tool); + Ok(Self { + turn_start, + turn_end, + session_start, + session_end, + pre_tool, + post_tool, + pre_tool_timeout_ms: wire.pre_tool_timeout_ms, + turn_start_commands, + turn_end_commands, + session_start_commands, + session_end_commands, + pre_tool_commands, + post_tool_commands, + }) + } +} + +#[derive(Serialize)] +#[serde(untagged)] +enum HookCommandValueRef<'a> { + One(&'a str), + Many(&'a Vec), +} + +impl Serialize for HooksConfig { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + use serde::ser::SerializeStruct; + + let mut state = serializer.serialize_struct("HooksConfig", 7)?; + if let Some(value) = Self::command_value(&self.turn_start, &self.turn_start_commands) { + state.serialize_field("turn_start", &value)?; + } + if let Some(value) = Self::command_value(&self.turn_end, &self.turn_end_commands) { + state.serialize_field("turn_end", &value)?; + } + if let Some(value) = Self::command_value(&self.session_start, &self.session_start_commands) + { + state.serialize_field("session_start", &value)?; + } + if let Some(value) = Self::command_value(&self.session_end, &self.session_end_commands) { + state.serialize_field("session_end", &value)?; + } + if let Some(value) = Self::command_value(&self.pre_tool, &self.pre_tool_commands) { + state.serialize_field("pre_tool", &value)?; + } + if let Some(value) = Self::command_value(&self.post_tool, &self.post_tool_commands) { + state.serialize_field("post_tool", &value)?; + } + state.serialize_field("pre_tool_timeout_ms", &self.pre_tool_timeout_ms)?; + state.end() + } } impl Default for HooksConfig { @@ -810,6 +976,12 @@ impl Default for HooksConfig { pre_tool: None, post_tool: None, pre_tool_timeout_ms: 5000, + turn_start_commands: None, + turn_end_commands: None, + session_start_commands: None, + session_end_commands: None, + pre_tool_commands: None, + post_tool_commands: None, } } } diff --git a/crates/jcode-terminal-launch/src/lib.rs b/crates/jcode-terminal-launch/src/lib.rs index 511d2408f1..ab6f61ab98 100644 --- a/crates/jcode-terminal-launch/src/lib.rs +++ b/crates/jcode-terminal-launch/src/lib.rs @@ -146,6 +146,27 @@ pub fn snapshot_client_terminal_env() -> Vec<(String, String)> { .collect() } +/// Replace every terminal-identifying variable inherited by `command` with an +/// authoritative client snapshot. +/// +/// Long-running servers may carry values from whichever pane launched them. +/// Removing the complete allowlist first is important: simply overlaying the +/// client's set leaves unrelated daemon values behind when the client does not +/// define that variable. `JCODE_CLIENT_*` aliases follow the same rule. +pub fn apply_client_terminal_env(command: &mut Command, client_env: &[(String, String)]) { + for key in CLIENT_TERMINAL_ENV_VARS { + command.env_remove(key); + command.env_remove(format!("JCODE_CLIENT_{key}")); + } + + for (key, value) in client_env { + if CLIENT_TERMINAL_ENV_VARS.contains(&key.as_str()) { + command.env(key, value); + command.env(format!("JCODE_CLIENT_{key}"), value); + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct SpawnAttempt { pub terminal: String,