Skip hook loading and lifecycle events for subagents (#10596)
This commit is contained in:
committed by
GitHub
parent
73c26506c3
commit
3e15ccb882
@@ -535,6 +535,14 @@ impl CliSession {
|
||||
|
||||
/// Start an interactive session, optionally with an initial message
|
||||
pub async fn interactive(&mut self, prompt: Option<String>) -> Result<()> {
|
||||
let banners = self
|
||||
.agent
|
||||
.emit_hook_with_banners(goose::hooks::HookEvent::SessionStart, &self.session_id)
|
||||
.await;
|
||||
if !banners.is_empty() {
|
||||
output::display_banner(&banners);
|
||||
}
|
||||
|
||||
let result = self.run_interactive(prompt).await;
|
||||
|
||||
self.agent
|
||||
|
||||
@@ -1435,6 +1435,14 @@ fn set_terminal_title() {
|
||||
let _ = std::io::stdout().flush();
|
||||
}
|
||||
|
||||
pub fn display_banner(banners: &[String]) {
|
||||
for banner in banners {
|
||||
for line in banner.lines() {
|
||||
println!("{}", line);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn display_context_usage(total_tokens: usize, context_limit: usize) {
|
||||
use console::style;
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
@@ -195,6 +196,7 @@ pub struct AgentConfig {
|
||||
pub mcp_host_info: Option<GooseMcpHostInfo>,
|
||||
pub session_name_update_tx: Option<mpsc::UnboundedSender<SessionNameUpdate>>,
|
||||
pub use_login_shell_path: Option<bool>,
|
||||
pub is_subagent: bool,
|
||||
}
|
||||
|
||||
impl AgentConfig {
|
||||
@@ -216,6 +218,7 @@ impl AgentConfig {
|
||||
mcp_host_info: None,
|
||||
session_name_update_tx: None,
|
||||
use_login_shell_path: None,
|
||||
is_subagent: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -265,6 +268,7 @@ pub struct Agent {
|
||||
pub(super) retry_manager: RetryManager,
|
||||
pub(super) tool_inspection_manager: ToolInspectionManager,
|
||||
pub(super) hook_manager: crate::hooks::HookManager,
|
||||
session_start_emitted: AtomicBool,
|
||||
#[cfg(test)]
|
||||
pub(super) stop_hook_block_cap_override: Option<u32>,
|
||||
container: Mutex<Option<Container>>,
|
||||
@@ -399,6 +403,7 @@ impl Agent {
|
||||
let inspection_session_manager = Arc::clone(&config.session_manager);
|
||||
let permission_manager = Arc::clone(&config.permission_manager);
|
||||
let use_login_shell_path = config.resolve_use_login_shell_path();
|
||||
let is_subagent = config.is_subagent;
|
||||
Self {
|
||||
provider: provider.clone(),
|
||||
config,
|
||||
@@ -425,10 +430,15 @@ impl Agent {
|
||||
provider.clone(),
|
||||
inspection_session_manager,
|
||||
),
|
||||
hook_manager: crate::hooks::HookManager::load(
|
||||
std::env::current_dir().ok().as_deref(),
|
||||
use_login_shell_path,
|
||||
),
|
||||
hook_manager: if is_subagent {
|
||||
crate::hooks::HookManager::default()
|
||||
} else {
|
||||
crate::hooks::HookManager::load(
|
||||
std::env::current_dir().ok().as_deref(),
|
||||
use_login_shell_path,
|
||||
)
|
||||
},
|
||||
session_start_emitted: AtomicBool::new(false),
|
||||
#[cfg(test)]
|
||||
stop_hook_block_cap_override: None,
|
||||
container: Mutex::new(None),
|
||||
@@ -470,6 +480,22 @@ impl Agent {
|
||||
.await;
|
||||
}
|
||||
|
||||
pub async fn emit_hook_with_banners(
|
||||
&self,
|
||||
event: crate::hooks::HookEvent,
|
||||
session_id: &str,
|
||||
) -> Vec<String> {
|
||||
if event == crate::hooks::HookEvent::SessionStart {
|
||||
self.session_start_emitted.store(true, Ordering::Release);
|
||||
}
|
||||
if !self.hook_manager.has_hooks(event) {
|
||||
return Vec::new();
|
||||
}
|
||||
self.hook_manager
|
||||
.emit_collecting_banners(event, crate::hooks::HookContext::new(event, session_id))
|
||||
.await
|
||||
}
|
||||
|
||||
fn stop_hook_context(
|
||||
session_id: &str,
|
||||
last_assistant_message: &str,
|
||||
@@ -1907,7 +1933,7 @@ impl Agent {
|
||||
return Ok(Box::pin(futures::stream::empty()));
|
||||
}
|
||||
|
||||
if is_first_agent_turn {
|
||||
if is_first_agent_turn && !self.session_start_emitted.swap(true, Ordering::AcqRel) {
|
||||
self.emit_hook(crate::hooks::HookEvent::SessionStart, &session_config.id)
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -1290,7 +1290,7 @@ impl SummonClient {
|
||||
// Subagents must use Auto until get_agent_messages forwards
|
||||
// ActionRequired messages to the parent. Until then, any mode
|
||||
// that requires approval will hang on the subagent's confirmation_rx.
|
||||
let agent_config = AgentConfig::new(
|
||||
let mut agent_config = AgentConfig::new(
|
||||
self.context.session_manager.clone(),
|
||||
crate::config::permission::PermissionManager::instance(),
|
||||
None,
|
||||
@@ -1299,6 +1299,7 @@ impl SummonClient {
|
||||
crate::agents::GoosePlatform::GooseCli,
|
||||
)
|
||||
.with_use_login_shell_path(self.context.use_login_shell_path);
|
||||
agent_config.is_subagent = true;
|
||||
|
||||
let subagent_session = self
|
||||
.create_subagent_session(&task_config, "Delegated task".to_string())
|
||||
@@ -1846,7 +1847,7 @@ impl SummonClient {
|
||||
// Subagents must use Auto until get_agent_messages forwards
|
||||
// ActionRequired messages to the parent. Until then, any mode
|
||||
// that requires approval will hang on the subagent's confirmation_rx.
|
||||
let agent_config = AgentConfig::new(
|
||||
let mut agent_config = AgentConfig::new(
|
||||
self.context.session_manager.clone(),
|
||||
crate::config::permission::PermissionManager::instance(),
|
||||
None,
|
||||
@@ -1855,6 +1856,7 @@ impl SummonClient {
|
||||
crate::agents::GoosePlatform::GooseCli,
|
||||
)
|
||||
.with_use_login_shell_path(self.context.use_login_shell_path);
|
||||
agent_config.is_subagent = true;
|
||||
|
||||
let subagent_session = self
|
||||
.create_subagent_session(&task_config, description.clone())
|
||||
|
||||
@@ -391,6 +391,86 @@ impl HookManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Like [`Self::emit`], but collects banner lines from hook stdout.
|
||||
///
|
||||
/// If a hook exits successfully and its stdout contains valid JSON with a
|
||||
/// `"banner"` field, that string is collected. Multiple hooks can each
|
||||
/// contribute banner lines. Non-JSON stdout or missing `"banner"` field
|
||||
/// is silently ignored (backwards compatible).
|
||||
pub async fn emit_collecting_banners(&self, event: HookEvent, ctx: HookContext) -> Vec<String> {
|
||||
let mut banners = Vec::new();
|
||||
let Some(rules) = self.rules.get(&event) else {
|
||||
return banners;
|
||||
};
|
||||
if rules.is_empty() {
|
||||
return banners;
|
||||
}
|
||||
|
||||
let payload = match serde_json::to_string(&ctx) {
|
||||
Ok(s) => s,
|
||||
Err(err) => {
|
||||
warn!(event = %event, error = %err, "Failed to serialize hook context");
|
||||
return banners;
|
||||
}
|
||||
};
|
||||
|
||||
for rule in rules {
|
||||
if let Some(matcher) = &rule.matcher {
|
||||
let target = ctx.matcher_context.as_deref().unwrap_or("");
|
||||
if !matcher.is_match(target) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
for action in &rule.actions {
|
||||
let LoadedAction::Command { command, timeout } = action;
|
||||
debug!(
|
||||
plugin = %rule.plugin_name,
|
||||
event = %event,
|
||||
command = %command,
|
||||
"Running plugin hook (banner-collecting)",
|
||||
);
|
||||
match run_command_hook(
|
||||
command,
|
||||
&rule.plugin_root,
|
||||
&payload,
|
||||
*timeout,
|
||||
self.use_login_shell_path,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(output) if output.status.success() => {
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
if let Some(banner) = extract_banner(stdout.trim()) {
|
||||
banners.push(banner);
|
||||
}
|
||||
}
|
||||
Ok(output) => {
|
||||
warn!(
|
||||
plugin = %rule.plugin_name,
|
||||
event = %event,
|
||||
command = %command,
|
||||
"hook exited with {:?}: {}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr).trim(),
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
plugin = %rule.plugin_name,
|
||||
event = %event,
|
||||
command = %command,
|
||||
error = %err,
|
||||
"Plugin hook failed",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
banners
|
||||
}
|
||||
|
||||
/// Like [`Self::emit`], but stops at the first rule that denies the event
|
||||
/// and returns the denial. A hook denies by exiting with status code 2
|
||||
/// (reason on stderr) or by printing `{"decision":"block","reason":"..."}`
|
||||
@@ -456,6 +536,20 @@ impl HookManager {
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_banner(stdout: &str) -> Option<String> {
|
||||
if !stdout.starts_with('{') {
|
||||
return None;
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct BannerResp {
|
||||
banner: Option<String>,
|
||||
}
|
||||
|
||||
let parsed: BannerResp = serde_json::from_str(stdout).ok()?;
|
||||
parsed.banner.filter(|b| !b.is_empty())
|
||||
}
|
||||
|
||||
fn deny_reason(output: &std::process::Output) -> Option<String> {
|
||||
const DEFAULT: &str = "denied by plugin hook";
|
||||
let non_empty = |s: String| if s.is_empty() { DEFAULT.into() } else { s };
|
||||
@@ -898,4 +992,70 @@ mod tests {
|
||||
.await;
|
||||
assert!(marker.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_banner_from_json() {
|
||||
assert_eq!(
|
||||
extract_banner(r#"{"banner":" 🌱 hello"}"#),
|
||||
Some(" 🌱 hello".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_banner_ignores_non_json() {
|
||||
assert_eq!(extract_banner("just some text"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_banner_ignores_json_without_banner_field() {
|
||||
assert_eq!(extract_banner(r#"{"decision":"allow"}"#), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_banner_ignores_empty_banner() {
|
||||
assert_eq!(extract_banner(r#"{"banner":""}"#), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_collecting_banners_returns_banner_lines() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let hooks = r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"printf '{\"banner\":\" 🌱 test banner\"}'" }]}]}}"#;
|
||||
let root = write_plugin(tmp.path(), "p", hooks);
|
||||
let mgr = make_manager(vec![DiscoveredPlugin {
|
||||
name: "p".into(),
|
||||
root,
|
||||
scope: PluginScope::User,
|
||||
}]);
|
||||
|
||||
let banners = mgr
|
||||
.emit_collecting_banners(
|
||||
HookEvent::SessionStart,
|
||||
HookContext::new(HookEvent::SessionStart, "s"),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(banners, vec![" 🌱 test banner"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_collecting_banners_skips_non_json_output() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let hooks =
|
||||
r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo hello"}]}]}}"#;
|
||||
let root = write_plugin(tmp.path(), "p", hooks);
|
||||
let mgr = make_manager(vec![DiscoveredPlugin {
|
||||
name: "p".into(),
|
||||
root,
|
||||
scope: PluginScope::User,
|
||||
}]);
|
||||
|
||||
let banners = mgr
|
||||
.emit_collecting_banners(
|
||||
HookEvent::SessionStart,
|
||||
HookContext::new(HookEvent::SessionStart, "s"),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(banners.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user