From 467e2cbfff0a8679a6a2c80d42054fb56655ea75 Mon Sep 17 00:00:00 2001 From: ningjingkun Date: Thu, 28 May 2026 20:58:26 +0800 Subject: [PATCH] feat(hooks): allow message_submit to transform submitted text --- crates/tui/src/commands/hooks.rs | 2 +- crates/tui/src/hooks.rs | 549 +++++++++++++++++++++++++++++- crates/tui/src/tui/ui.rs | 26 +- crates/tui/src/tui/ui/tests.rs | 108 ++++++ docs/CONFIGURATION.md | 49 +++ docs/rfcs/1364-hooks-lifecycle.md | 276 +++++++++++++++ web/app/[locale]/docs/page.tsx | 12 +- 7 files changed, 1009 insertions(+), 13 deletions(-) create mode 100644 docs/rfcs/1364-hooks-lifecycle.md diff --git a/crates/tui/src/commands/hooks.rs b/crates/tui/src/commands/hooks.rs index fbf7d760..d9589c6c 100644 --- a/crates/tui/src/commands/hooks.rs +++ b/crates/tui/src/commands/hooks.rs @@ -45,7 +45,7 @@ fn events() -> CommandResult { (HookEvent::SessionEnd, "fires once on graceful shutdown"), ( HookEvent::MessageSubmit, - "fires when the user submits a turn (before model dispatch)", + "fires before model dispatch; can transform or block submitted text", ), ( HookEvent::ToolCallBefore, diff --git a/crates/tui/src/hooks.rs b/crates/tui/src/hooks.rs index 2fbc5f55..685fea76 100644 --- a/crates/tui/src/hooks.rs +++ b/crates/tui/src/hooks.rs @@ -14,8 +14,9 @@ #[allow(unused_imports)] use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use serde_json::json; use std::collections::HashMap; -use std::io::Read; +use std::io::{Read, Write}; use std::path::PathBuf; use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; @@ -423,6 +424,24 @@ pub struct HookResult { pub error: Option, } +/// Result of running mutable `message_submit` hooks. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MessageSubmitOutcome { + /// No hook changed the submitted text. + Unchanged, + /// One or more hooks replaced the submitted text. + Replaced(String), + /// A hook intentionally blocked the submission. + Blocked { reason: String }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum MessageSubmitStdout { + Unchanged, + Replaced(String), + Invalid(String), +} + /// Executor for running hooks #[derive(Debug, Clone)] pub struct HookExecutor { @@ -500,6 +519,100 @@ impl HookExecutor { self.config.enabled && self.config.hooks.iter().any(|h| h.event == event) } + /// Run configured `message_submit` hooks as a mutable submit pipeline. + /// + /// This is deliberately separate from [`Self::execute`]: most hook events + /// are observer-only, while `message_submit` has a narrow stdout JSON + /// contract that can replace or block the submitted text. + pub fn execute_message_submit_transform( + &self, + context: &HookContext, + original_text: &str, + ) -> MessageSubmitOutcome { + if !self.config.enabled { + return MessageSubmitOutcome::Unchanged; + } + + let hooks = self.config.hooks_for_event(HookEvent::MessageSubmit); + if hooks.is_empty() { + return MessageSubmitOutcome::Unchanged; + } + + let mut current_text = original_text.to_string(); + + for hook in hooks { + let hook_context = context.clone().with_message(¤t_text); + if !self.matches_condition(hook, &hook_context) { + continue; + } + + let env_vars = hook_context.to_env_vars(); + if hook.background { + let _ = self.execute_background(hook, &env_vars); + continue; + } + + let payload = message_submit_payload(&hook_context, ¤t_text); + let result = self.execute_sync_with_stdin(hook, &env_vars, &payload); + + if result.exit_code == Some(2) { + return MessageSubmitOutcome::Blocked { + reason: message_submit_block_reason( + &result, + "message_submit hook blocked submission", + ), + }; + } + + if !result.success { + let label = result.name.as_deref().unwrap_or("(unnamed)"); + tracing::warn!( + target: "hooks", + hook = label, + event = "message_submit", + exit_code = ?result.exit_code, + duration_ms = result.duration.as_millis() as u64, + error = result.error.as_deref().unwrap_or(""), + stderr_head = %result.stderr.lines().next().unwrap_or(""), + "message_submit hook failed" + ); + + if hook.continue_on_error { + continue; + } + + return MessageSubmitOutcome::Blocked { + reason: message_submit_block_reason( + &result, + "message_submit hook failed and blocked submission", + ), + }; + } + + match parse_message_submit_stdout(&result.stdout) { + MessageSubmitStdout::Unchanged => {} + MessageSubmitStdout::Replaced(text) => { + current_text = text; + } + MessageSubmitStdout::Invalid(reason) => { + tracing::warn!( + target: "hooks", + hook = result.name.as_deref().unwrap_or("(unnamed)"), + event = "message_submit", + reason = %reason, + "ignored invalid message_submit hook stdout" + ); + } + } + } + + if current_text == original_text { + MessageSubmitOutcome::Unchanged + } else { + MessageSubmitOutcome::Replaced(current_text) + } + } + /// Run every `ShellEnv` hook for this context and merge their stdout /// (`KEY=VALUE\n` lines) into a single env-var map. Used by the /// `exec_shell` tool to inject ephemeral credentials, per-skill PATH @@ -659,6 +772,28 @@ impl HookExecutor { /// Execute a hook synchronously fn execute_sync(&self, hook: &Hook, env_vars: &HashMap) -> HookResult { + self.execute_sync_inner(hook, env_vars, None) + } + + /// Execute a hook synchronously with a structured JSON stdin payload. + /// + /// Used by mutable `message_submit` hooks. Existing observer hooks keep the + /// stdin-less [`Self::execute_sync`] path so their behavior is unchanged. + fn execute_sync_with_stdin( + &self, + hook: &Hook, + env_vars: &HashMap, + stdin_json: &serde_json::Value, + ) -> HookResult { + self.execute_sync_inner(hook, env_vars, Some(stdin_json)) + } + + fn execute_sync_inner( + &self, + hook: &Hook, + env_vars: &HashMap, + stdin_json: Option<&serde_json::Value>, + ) -> HookResult { let started = Instant::now(); let working_dir = self .config @@ -672,13 +807,32 @@ impl HookExecutor { .unwrap_or(hook.timeout_secs); let timeout = Duration::from_secs(timeout_secs); - let mut child = match Self::build_shell_command(&hook.command) + let stdin_bytes = match stdin_json.map(serde_json::to_vec).transpose() { + Ok(bytes) => bytes, + Err(e) => { + return HookResult { + name: hook.name.clone(), + success: false, + exit_code: None, + stdout: String::new(), + stderr: String::new(), + duration: started.elapsed(), + error: Some(format!("Failed to encode hook stdin: {e}")), + }; + } + }; + + let mut command = Self::build_shell_command(&hook.command); + command .current_dir(&working_dir) .envs(env_vars) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - { + .stderr(Stdio::piped()); + if stdin_bytes.is_some() { + command.stdin(Stdio::piped()); + } + + let mut child = match command.spawn() { Ok(child) => child, Err(e) => { return HookResult { @@ -693,6 +847,12 @@ impl HookExecutor { } }; + if let (Some(bytes), Some(mut stdin)) = (stdin_bytes, child.stdin.take()) { + let _ = stdin.write_all(&bytes); + let _ = stdin.write_all(b"\n"); + let _ = stdin.flush(); + } + fn read_pipe(mut pipe: impl Read) -> String { let mut buf = String::new(); let _ = pipe.read_to_string(&mut buf); @@ -768,6 +928,84 @@ impl HookExecutor { } } +fn message_submit_payload(context: &HookContext, text: &str) -> serde_json::Value { + json!({ + "event": HookEvent::MessageSubmit.as_str(), + "text": text, + "session_id": context.session_id.as_deref(), + "workspace": context.workspace.as_ref().map(|path| path.display().to_string()), + "mode": context.mode.as_deref(), + "model": context.model.as_deref(), + "total_tokens": context.total_tokens, + }) +} + +fn parse_message_submit_stdout(stdout: &str) -> MessageSubmitStdout { + let trimmed = stdout.trim(); + if trimmed.is_empty() { + return MessageSubmitStdout::Unchanged; + } + + let value: serde_json::Value = match serde_json::from_str(trimmed) { + Ok(value) => value, + Err(e) => return MessageSubmitStdout::Invalid(format!("invalid JSON: {e}")), + }; + + let Some(object) = value.as_object() else { + return MessageSubmitStdout::Invalid("stdout JSON must be an object".to_string()); + }; + + match object.get("text") { + Some(serde_json::Value::String(text)) => MessageSubmitStdout::Replaced(text.clone()), + Some(_) => MessageSubmitStdout::Invalid("stdout `text` field must be a string".to_string()), + None => MessageSubmitStdout::Unchanged, + } +} + +fn message_submit_block_reason(result: &HookResult, fallback: &str) -> String { + if let Some(reason) = message_submit_stdout_reason(&result.stdout) { + return reason; + } + if let Some(reason) = first_non_empty_line(&result.stderr) { + return reason; + } + if let Some(reason) = first_non_empty_line(&result.stdout) { + return reason; + } + if let Some(reason) = result.error.as_deref().and_then(first_non_empty_line) { + return reason; + } + fallback.to_string() +} + +fn message_submit_stdout_reason(stdout: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(stdout.trim()).ok()?; + value + .get("reason") + .and_then(serde_json::Value::as_str) + .map(truncate_hook_message) +} + +fn first_non_empty_line(text: &str) -> Option { + text.lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(truncate_hook_message) +} + +fn truncate_hook_message(message: &str) -> String { + const MAX_CHARS: usize = 240; + let mut out = String::new(); + for (idx, ch) in message.chars().enumerate() { + if idx >= MAX_CHARS { + out.push('…'); + return out; + } + out.push(ch); + } + out +} + /// Parse `KEY=VALUE\n` lines from a `shell_env` hook's stdout into a map. /// /// Tolerated: blank lines, leading whitespace, `#` comment lines (ignored), @@ -850,6 +1088,58 @@ NOEQUAL line dropped assert!(parsed.is_empty()); } + #[test] + fn parse_message_submit_stdout_replaces_text() { + assert_eq!( + super::parse_message_submit_stdout(r#"{"text":"changed"}"#), + MessageSubmitStdout::Replaced("changed".to_string()) + ); + } + + #[test] + fn parse_message_submit_stdout_empty_is_unchanged() { + assert_eq!( + super::parse_message_submit_stdout(" \n\t "), + MessageSubmitStdout::Unchanged + ); + } + + #[test] + fn parse_message_submit_stdout_without_text_is_unchanged() { + assert_eq!( + super::parse_message_submit_stdout(r#"{"reason":"only used for blocks"}"#), + MessageSubmitStdout::Unchanged + ); + } + + #[test] + fn parse_message_submit_stdout_rejects_malformed_json() { + assert!(matches!( + super::parse_message_submit_stdout("not json"), + MessageSubmitStdout::Invalid(_) + )); + } + + #[test] + fn parse_message_submit_stdout_rejects_non_string_text() { + assert!(matches!( + super::parse_message_submit_stdout(r#"{"text":123}"#), + MessageSubmitStdout::Invalid(_) + )); + } + + #[test] + fn parse_message_submit_stdout_rejects_non_object_json() { + assert!(matches!( + super::parse_message_submit_stdout(r#"["not", "an", "object"]"#), + MessageSubmitStdout::Invalid(_) + )); + assert!(matches!( + super::parse_message_submit_stdout(r#""not an object""#), + MessageSubmitStdout::Invalid(_) + )); + } + #[test] fn test_hook_event_as_str() { assert_eq!(HookEvent::SessionStart.as_str(), "session_start"); @@ -995,6 +1285,255 @@ NOEQUAL line dropped assert_eq!(executor.session_id().len(), 13); // "sess_" + 8 chars } + #[cfg(not(windows))] + fn write_hook_script(dir: &tempfile::TempDir, name: &str, content: &str) -> String { + let path = dir.path().join(name); + std::fs::write(&path, content).expect("write hook script"); + format!("sh {}", path.display()) + } + + #[cfg(not(windows))] + fn submit_context(dir: &tempfile::TempDir) -> HookContext { + HookContext::new() + .with_session_id("sess_test") + .with_workspace(dir.path().to_path_buf()) + .with_mode("agent") + .with_model("deepseek-test") + .with_tokens(42) + } + + #[cfg(not(windows))] + #[test] + fn message_submit_transform_applies_hooks_in_order() { + let dir = tempfile::tempdir().expect("tempdir"); + let first = write_hook_script( + &dir, + "first.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"first"}' +"#, + ); + let second = write_hook_script( + &dir, + "second.sh", + r#"#!/bin/sh +payload=$(cat) +case "$payload" in + *'"text":"first"'*) printf '%s\n' '{"text":"first second"}' ;; + *) printf '%s\n' '{"text":"wrong"}' ;; +esac +"#, + ); + let config = HooksConfig { + enabled: true, + hooks: vec![ + Hook::new(HookEvent::MessageSubmit, &first), + Hook::new(HookEvent::MessageSubmit, &second), + ], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Replaced("first second".to_string()) + ); + } + + #[cfg(not(windows))] + #[test] + fn message_submit_transform_exit_two_blocks_submission() { + let dir = tempfile::tempdir().expect("tempdir"); + let command = write_hook_script( + &dir, + "block.sh", + r#"#!/bin/sh +printf '%s\n' '{"reason":"policy blocked this prompt"}' +exit 2 +"#, + ); + let config = HooksConfig { + enabled: true, + hooks: vec![Hook::new(HookEvent::MessageSubmit, &command)], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Blocked { + reason: "policy blocked this prompt".to_string() + } + ); + } + + #[cfg(not(windows))] + #[test] + fn background_message_submit_hook_is_observer_only() { + let dir = tempfile::tempdir().expect("tempdir"); + let command = write_hook_script( + &dir, + "background.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"ignored"}' +"#, + ); + let config = HooksConfig { + enabled: true, + hooks: vec![Hook::new(HookEvent::MessageSubmit, &command).background()], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Unchanged + ); + } + + #[test] + fn message_submit_transform_without_configured_hooks_is_unchanged() { + let executor = HookExecutor::new(HooksConfig::default(), PathBuf::from(".")); + + assert_eq!( + executor.execute_message_submit_transform(&HookContext::new(), "original"), + MessageSubmitOutcome::Unchanged + ); + } + + #[cfg(not(windows))] + #[test] + fn message_submit_transform_skips_non_matching_condition() { + let dir = tempfile::tempdir().expect("tempdir"); + let command = write_hook_script( + &dir, + "replace.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"should not apply"}' +"#, + ); + let hook = + Hook::new(HookEvent::MessageSubmit, &command).with_condition(HookCondition::Mode { + mode: "plan".into(), + }); + let config = HooksConfig { + enabled: true, + hooks: vec![hook], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Unchanged + ); + } + + #[cfg(not(windows))] + #[test] + fn message_submit_continue_on_error_true_keeps_text_and_runs_later_hooks() { + let dir = tempfile::tempdir().expect("tempdir"); + let failing = write_hook_script( + &dir, + "fail_continue.sh", + r#"#!/bin/sh +printf '%s\n' 'soft failure' >&2 +exit 9 +"#, + ); + let replacing = write_hook_script( + &dir, + "replace_after_failure.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"recovered"}' +"#, + ); + let config = HooksConfig { + enabled: true, + hooks: vec![ + Hook::new(HookEvent::MessageSubmit, &failing), + Hook::new(HookEvent::MessageSubmit, &replacing), + ], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Replaced("recovered".to_string()) + ); + } + + #[cfg(not(windows))] + #[test] + fn message_submit_invalid_stdout_keeps_text_and_runs_later_hooks() { + let dir = tempfile::tempdir().expect("tempdir"); + let invalid = write_hook_script( + &dir, + "invalid_stdout.sh", + r#"#!/bin/sh +printf '%s\n' 'not json' +"#, + ); + let replacing = write_hook_script( + &dir, + "replace_after_invalid.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"valid later"}' +"#, + ); + let config = HooksConfig { + enabled: true, + hooks: vec![ + Hook::new(HookEvent::MessageSubmit, &invalid), + Hook::new(HookEvent::MessageSubmit, &replacing), + ], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Replaced("valid later".to_string()) + ); + } + + #[cfg(not(windows))] + #[test] + fn message_submit_continue_on_error_false_blocks_on_failure() { + let dir = tempfile::tempdir().expect("tempdir"); + let command = write_hook_script( + &dir, + "fail.sh", + r#"#!/bin/sh +printf '%s\n' 'hard failure' >&2 +exit 7 +"#, + ); + let mut hook = Hook::new(HookEvent::MessageSubmit, &command); + hook.continue_on_error = false; + let config = HooksConfig { + enabled: true, + hooks: vec![hook], + working_dir: Some(dir.path().to_path_buf()), + ..HooksConfig::default() + }; + let executor = HookExecutor::new(config, dir.path().to_path_buf()); + + assert_eq!( + executor.execute_message_submit_transform(&submit_context(&dir), "original"), + MessageSubmitOutcome::Blocked { + reason: "hard failure".to_string() + } + ); + } + #[test] fn has_hooks_for_event_fast_path_returns_false_for_empty_config() { let executor = HookExecutor::disabled(); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 0f34531e..7b6c5e7d 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -4500,19 +4500,33 @@ async fn dispatch_user_message( app: &mut App, config: &Config, engine_handle: &EngineHandle, - message: QueuedMessage, + mut message: QueuedMessage, ) -> Result<()> { - // #455 (observer-only): fire `message_submit` hooks before - // dispatch. Hooks see the user's display text via the - // `with_message` builder. Read-only — they can log, audit, or - // notify but cannot mutate the message that goes to the engine. + // #1364: run mutable `message_submit` hooks before dispatch. Hooks see the + // user's display text and may replace or block it before file mentions, + // skill wrapping, history, and model input are resolved. // Fast-path skip when no hooks configured. if app .hooks .has_hooks_for_event(crate::hooks::HookEvent::MessageSubmit) { let context = app.base_hook_context().with_message(&message.display); - let _ = app.execute_hooks(crate::hooks::HookEvent::MessageSubmit, &context); + match app + .hooks + .execute_message_submit_transform(&context, &message.display) + { + crate::hooks::MessageSubmitOutcome::Unchanged => {} + crate::hooks::MessageSubmitOutcome::Replaced(text) => { + message.display = text; + } + crate::hooks::MessageSubmitOutcome::Blocked { reason } => { + app.status_message = Some(reason); + app.is_loading = false; + app.dispatch_started_at = None; + app.runtime_turn_status = None; + return Ok(()); + } + } } // Set immediately to prevent double-dispatch before TurnStarted event arrives. diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index a666712a..9fc84482 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -2072,6 +2072,114 @@ async fn dispatch_user_message_failed_send_clears_loading_state() { assert!(app.dispatch_started_at.is_none()); } +#[cfg(not(windows))] +fn write_message_submit_hook(dir: &TempDir, name: &str, body: &str) -> String { + let path = dir.path().join(name); + std::fs::write(&path, body).expect("write message_submit hook"); + format!("sh {}", path.display()) +} + +#[cfg(not(windows))] +fn configure_single_message_submit_hook(app: &mut App, dir: &TempDir, command: String) { + app.hooks = crate::hooks::HookExecutor::new( + crate::hooks::HooksConfig { + enabled: true, + hooks: vec![crate::hooks::Hook::new( + crate::hooks::HookEvent::MessageSubmit, + &command, + )], + working_dir: Some(dir.path().to_path_buf()), + ..crate::hooks::HooksConfig::default() + }, + dir.path().to_path_buf(), + ); +} + +#[cfg(not(windows))] +#[tokio::test] +async fn dispatch_user_message_uses_transformed_message_submit_text() { + let dir = TempDir::new().expect("tempdir"); + let command = write_message_submit_hook( + &dir, + "replace.sh", + r#"#!/bin/sh +printf '%s\n' '{"text":"[hooked] hello"}' +"#, + ); + let mut app = create_test_app(); + configure_single_message_submit_hook(&mut app, &dir, command); + let mut engine = crate::core::engine::mock_engine_handle(); + let config = Config::default(); + + dispatch_user_message( + &mut app, + &config, + &engine.handle, + QueuedMessage::new("hello".to_string(), None), + ) + .await + .expect("dispatch user message"); + + assert_eq!(app.last_submitted_prompt.as_deref(), Some("[hooked] hello")); + assert!(app.history.iter().any(|cell| matches!( + cell, + HistoryCell::User { content } if content == "[hooked] hello" + ))); + assert_eq!(app.api_messages.len(), 1); + assert!(matches!( + &app.api_messages[0].content[0], + ContentBlock::Text { text, .. } if text == "[hooked] hello" + )); + match engine.rx_op.recv().await.expect("send message op") { + crate::core::ops::Op::SendMessage { content, .. } => { + assert_eq!(content, "[hooked] hello"); + } + other => panic!("expected SendMessage, got {other:?}"), + } +} + +#[cfg(not(windows))] +#[tokio::test] +async fn dispatch_user_message_blocked_by_message_submit_hook_does_not_start_turn() { + let dir = TempDir::new().expect("tempdir"); + let command = write_message_submit_hook( + &dir, + "block.sh", + r#"#!/bin/sh +printf '%s\n' '{"reason":"blocked by test hook"}' +exit 2 +"#, + ); + let mut app = create_test_app(); + configure_single_message_submit_hook(&mut app, &dir, command); + let mut engine = crate::core::engine::mock_engine_handle(); + let config = Config::default(); + + dispatch_user_message( + &mut app, + &config, + &engine.handle, + QueuedMessage::new("hello".to_string(), None), + ) + .await + .expect("blocked submit is handled locally"); + + assert_eq!(app.status_message.as_deref(), Some("blocked by test hook")); + assert!(app.api_messages.is_empty()); + assert!( + app.history + .iter() + .all(|cell| !matches!(cell, HistoryCell::User { .. })) + ); + assert!(!app.is_loading); + assert!(app.dispatch_started_at.is_none()); + assert!(app.runtime_turn_status.is_none()); + assert!( + engine.rx_op.try_recv().is_err(), + "blocked submit must not send any engine operation" + ); +} + #[test] fn turn_liveness_watchdog_clears_stale_dispatch() { let mut app = create_test_app(); diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0fe1dd3f..0132c07b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -376,6 +376,55 @@ obvious when hooks are globally suppressed. Hooks are configured under `[[hooks.hooks]]` entries — see the existing hook-system documentation for the full schema. +### Mutable `message_submit` hooks + +`message_submit` hooks run before a submitted message is added to +history or sent to the model. Unlike observer-only lifecycle hooks, +non-background `message_submit` hooks can replace or block the +submitted text. + +```toml +[[hooks.hooks]] +event = "message_submit" +command = "~/.codewhale/hooks/inject-context.sh" +timeout_secs = 2 +continue_on_error = true +``` + +The hook receives JSON on stdin: + +```json +{ + "event": "message_submit", + "text": "original user text", + "session_id": "sess_12345678", + "workspace": "/path/to/workspace", + "mode": "agent", + "model": "deepseek-chat", + "total_tokens": 1234 +} +``` + +If the hook exits `0` and prints JSON with a string `text` field, +that value replaces the submitted text: + +```json +{ "text": "replacement user text" } +``` + +Exit `0` with empty stdout, or stdout JSON without `text`, leaves +the current text unchanged. Exit `2` blocks the submission before +the turn starts; a `reason` field, stderr, or stdout can provide the +status message shown in the TUI. Other non-zero exits follow the +hook's `continue_on_error` setting. + +Multiple `message_submit` hooks run in config order, and each hook +receives the text produced by the previous hook. Hooks marked +`background = true` are observer-only and cannot transform or block +the message. Existing environment variables remain available. +`shell_env` hooks keep their existing `KEY=VALUE` stdout contract; +the JSON stdout contract applies only to `message_submit`. + ### Composer stash (`/stash`, Ctrl+S) Press **Ctrl+S** in the composer to park the current draft to diff --git a/docs/rfcs/1364-hooks-lifecycle.md b/docs/rfcs/1364-hooks-lifecycle.md new file mode 100644 index 00000000..f7f759c1 --- /dev/null +++ b/docs/rfcs/1364-hooks-lifecycle.md @@ -0,0 +1,276 @@ +# RFC: Hook Lifecycle Data Flow + +**Issue:** #1364 +**Status:** Draft +**Date:** 2026-05-28 + +## 1. Problem + +CodeWhale already has lifecycle hooks and MCP support, but the current hook +surface is mostly observer-only. This blocks portable extensions that need to +participate in the agent data flow: + +- memory/context injection before a user message reaches the model +- post-turn background analysis that prepares context for the next turn +- sub-agent lifecycle visibility for orchestration and audit extensions + +The current `message_submit` event fires before dispatch, but its output is +ignored. `TurnComplete`, `AgentSpawned`, and `AgentComplete` exist internally, +but they are not exposed as configurable hook events. + +## 2. PR split + +This issue should be implemented as three PRs. Each PR should be independently +reviewable and should leave the hook system in a useful state. + +### PR 1: Mutable `message_submit` + +Add a structured hook execution path for `message_submit` that can transform or +block the user's submitted text before it is sent to the engine. + +Scope: + +- keep the existing `[[hooks.hooks]]` config shape +- pass a JSON payload to the hook on stdin +- interpret stdout JSON containing `text` as the replacement user text +- treat exit code `2` as an intentional block +- run multiple submit hooks serially in config order +- keep existing env vars for compatibility +- keep `shell_env` stdout parsing unchanged + +Non-goals: + +- no tool argument mutation +- no global stdout JSON semantics for all hook events +- no transcript or model response mutation + +### PR 2: `turn_end` + +Expose the existing turn completion lifecycle as a hook event. + +Scope: + +- add `HookEvent::TurnEnd` with event name `turn_end` +- fire from the UI's `EngineEvent::TurnComplete` branch after core app state, + usage, cost, notifications, and receipt state have been updated +- pass turn metadata on stdin as JSON +- make failures non-blocking and warn-only +- include a `stop_hook_active` field in the payload, initially `false`, so the + contract can support re-entry protection later + +Non-goals: + +- no change to turn status +- no blocking of user input +- no transcript mutation from `turn_end` + +### PR 3: Subagent lifecycle observer hooks + +Expose subagent start and completion as observer-only hook events. + +Scope: + +- add `HookEvent::SubagentSpawn` with event name `subagent_spawn` +- add `HookEvent::SubagentComplete` with event name `subagent_complete` +- fire from the existing `AgentSpawned` and `AgentComplete` UI branches +- pass subagent metadata on stdin as JSON +- make failures non-blocking and warn-only + +Non-goals: + +- no subagent spawn gating in the first version +- no subagent prompt/result mutation +- no changes to subagent scheduling + +## 3. PR 1 detailed plan + +### 3.1 Contract + +Configuration: + +```toml +[[hooks.hooks]] +event = "message_submit" +command = "~/.deepseek/hooks/inject-memory.sh" +timeout_secs = 2 +continue_on_error = true +``` + +Input payload on stdin: + +```json +{ + "event": "message_submit", + "text": "original user text", + "session_id": "sess_xxxx", + "workspace": "/path/to/workspace", + "mode": "agent", + "model": "deepseek-chat", + "total_tokens": 1234 +} +``` + +Output payload on stdout: + +```json +{ "text": "replacement user text" } +``` + +Rules: + +- exit `0` with stdout JSON containing `text: string` replaces the current text +- exit `0` with empty stdout leaves the current text unchanged +- exit `0` with JSON that does not contain `text` leaves the current text + unchanged +- exit `2` blocks submission before the message is appended to history or sent + to the engine +- other non-zero exits follow `continue_on_error` + - `true`: warn, keep the current text, continue later hooks + - `false`: stop later hooks and block submission with an error message +- `background = true` on `message_submit` remains observer-only and cannot + transform or block submission + +Multiple hooks: + +- hooks run in config order +- each hook receives the latest transformed text +- the final transformed text is the only text used by file mention expansion, + skill wrapping, auto routing, history, and `api_messages` + +### 3.2 Implementation steps + +1. Add structured submit outcome types in `crates/tui/src/hooks.rs`: + +```rust +pub enum MessageSubmitOutcome { + Unchanged, + Replaced(String), + Blocked { reason: String }, +} +``` + +2. Add a stdin-capable sync executor: + +```rust +fn execute_sync_with_stdin( + &self, + hook: &Hook, + env_vars: &HashMap, + stdin_json: &serde_json::Value, +) -> HookResult +``` + +This should reuse the existing timeout, working directory, stdout, stderr, and +error handling behavior from `execute_sync`. + +3. Add a `message_submit` transform entrypoint: + +```rust +pub fn execute_message_submit_transform( + &self, + context: &HookContext, + original_text: &str, +) -> MessageSubmitOutcome +``` + +This method should: + +- filter configured `MessageSubmit` hooks through existing condition matching +- build a JSON payload for each hook using the current text +- run non-background hooks through `execute_sync_with_stdin` +- run background hooks with the existing observer-only path +- parse stdout JSON only for non-background hooks +- return the final text or a block result + +4. Apply the transformed message in `dispatch_user_message`: + +- run the transform before `last_submitted_prompt`, file mentions, history, and + `api_messages` +- create a local mutable `QueuedMessage` or replacement display text +- if blocked, show a status message or toast and return without dispatch + +5. Update `/hooks events`: + +- keep `message_submit` listed +- update description to say it can transform or block user text + +6. Update user-facing docs: + +- document the stdin/stdout contract +- document exit code `2` +- document that `shell_env` still uses `KEY=VALUE` stdout + +### 3.3 Test plan + +Unit tests in `crates/tui/src/hooks.rs`: + +- parses stdout `{"text":"changed"}` as replacement +- empty stdout means unchanged +- JSON without `text` means unchanged +- malformed stdout means unchanged with warning semantics +- exit `2` maps to blocked +- multiple hooks apply transforms in order +- background `message_submit` hook cannot transform +- `continue_on_error = false` blocks on non-zero failure + +TUI integration or focused dispatch tests: + +- transformed text is written to `api_messages` +- transformed text is written to visible history +- transformed text is used by file mention expansion +- blocked submit does not append user history +- blocked submit does not push an API message +- blocked submit leaves loading state false + +Manual smoke test: + +1. Add a config hook that prepends `[hooked] ` to every submitted message. +2. Submit `hello`. +3. Verify the transcript and model input use `[hooked] hello`. +4. Replace the hook with one that exits `2`. +5. Submit `hello`. +6. Verify no turn starts and the TUI shows the block reason. + +## 4. Shared payload conventions + +All new structured hook payloads should include: + +- `event` +- `session_id` +- `workspace` +- `mode` +- `model` + +Event-specific payloads should add only fields that are stable and useful for +extension authors. Avoid leaking secrets, full tool outputs, or unbounded +transcript content in the first version. + +## 5. Compatibility + +- Existing hook config remains valid. +- Existing observer-only hooks keep working. +- Existing env vars remain available. +- `shell_env` keeps its existing stdout `KEY=VALUE` contract. +- Structured stdout is interpreted only by `message_submit` in PR 1. + +## 6. Review checkpoints + +PR 1 should be accepted only if: + +- submit mutation is covered by tests +- submit blocking is covered by tests +- the unchanged path preserves current behavior +- `shell_env` tests still prove the old stdout contract +- the docs clearly mark `message_submit` as the only mutable hook + +PR 2 should be accepted only if: + +- `turn_end` fires after `TurnComplete` app state updates +- failure is warn-only +- payload contains status and usage + +PR 3 should be accepted only if: + +- subagent hooks are observer-only +- failures do not affect subagent lifecycle +- payloads do not include unbounded or secret data diff --git a/web/app/[locale]/docs/page.tsx b/web/app/[locale]/docs/page.tsx index cfe384b5..aacdbc06 100644 --- a/web/app/[locale]/docs/page.tsx +++ b/web/app/[locale]/docs/page.tsx @@ -184,6 +184,11 @@ command = "~/.codewhale/hooks/pre.sh" # / message_submit / mode_change /

完整参考：config.example.toml。

+

+ message_submit hooks run before a user message is sent to the model. A non-background hook can print + {'{"text":"replacement"}'} on stdout to replace the message, or exit with code 2 to block the submission. + shell_env keeps its existing KEY=VALUE stdout contract. +

{/* MCP */} @@ -434,6 +439,11 @@ command = "~/.codewhale/hooks/pre.sh" # / message_submit / mode_change /

Full reference: config.example.toml.

+

+ message_submit hooks run before a user message is sent to the model. A non-background hook can print + {'{"text":"replacement"}'} on stdout to replace the message, or exit with code 2 to block the submission. + shell_env keeps its existing KEY=VALUE stdout contract. +

@@ -547,4 +557,4 @@ command = "~/.codewhale/hooks/pre.sh" # / message_submit / mode_change / )} ); -} \ No newline at end of file +}