diff --git a/_docs/config/index.mdx b/_docs/config/index.mdx index 9b0a34b..528be93 100644 --- a/_docs/config/index.mdx +++ b/_docs/config/index.mdx @@ -207,6 +207,20 @@ Subagents do not inherit the parent model's reasoning effort. Set `reasoningEffo Direct `@agent` invocation is available for visible `subagent` and `all` agents. For example, `@explore trace config loading` starts a child session through the Task flow. +## Disabling subagents + +When subagents waste tokens (for example, a child agent returns an empty result), turn them off. The `task` tool is hidden from the model and any `task` call — including direct `@agent` mentions, which run through the same Task flow — is rejected with a permission error. + +```sh +crabcode --disable-subagents +crabcode --disable-subagents -p "fix the failing test" +CRABCODE_DISABLE_SUBAGENTS=1 crabcode +``` + +The flag is global, so it also applies before subcommands (`crabcode --disable-subagents acp`, `crabcode --disable-subagents serve`). Remote `serve` hosts enforce whatever value was set in the host process environment. + +`CRABCODE_DISABLE_SUBAGENTS` accepts `1`, `true`, `yes`, `y`, or `on` (case-insensitive); anything else, including an unset variable, keeps subagents enabled. Either source disables — there is no re-enable override once one of them says disabled. + ## What belongs where | Need | Put it in | diff --git a/src/config/runtime.rs b/src/config/runtime.rs index 41c975d..e070417 100644 --- a/src/config/runtime.rs +++ b/src/config/runtime.rs @@ -18,6 +18,9 @@ pub struct ConfigRuntimeOptions { pub print_mode: bool, /// Skip permission prompts (print-mode `--dangerously-skip-permissions`). pub dangerously_skip_permissions: bool, + /// Hide the `task` tool and reject subagent calls. Set from + /// `--disable-subagents` / `CRABCODE_DISABLE_SUBAGENTS`. + pub disable_subagents: bool, } /// Runtime pieces derived from merged config. @@ -44,7 +47,10 @@ impl ConfigRuntime { let mut permission_rules = merged.permission_rules.clone(); if options.print_mode { - permission_rules = deny_print_mode_interactive_tools(permission_rules); + permission_rules = deny_tools(permission_rules, &["question", "update_plan"]); + } + if options.disable_subagents || crate::tools::task::subagents_disabled() { + permission_rules = deny_tools(permission_rules, &["task"]); } let tool_permissions = ToolPermissions::new(cwd) @@ -78,8 +84,8 @@ impl ConfigRuntime { } } -fn deny_print_mode_interactive_tools(mut rules: PermissionRules) -> PermissionRules { - for tool_id in ["question", "update_plan"] { +fn deny_tools(mut rules: PermissionRules, tool_ids: &[&str]) -> PermissionRules { + for tool_id in tool_ids { rules.push(PermissionRule { permission: tool_id.to_string(), pattern: "*".to_string(), @@ -167,6 +173,43 @@ mod tests { assert!(discovery.provider_is_enabled("anthropic")); } + #[test] + fn disable_subagents_denies_task_tool_only() { + // Ensure the env seam does not leak into this explicit-flag test. + let _lock = crate::tools::task::disable_subagents_env_lock(); + let prev = std::env::var(crate::tools::task::DISABLE_SUBAGENTS_ENV).ok(); + std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV); + + let merged = MergedConfig::default(); + let disabled_rt = ConfigRuntime::from_merged( + &merged, + "/tmp/workspace", + ConfigRuntimeOptions { + disable_subagents: true, + ..Default::default() + }, + ); + let default_rt = + ConfigRuntime::from_merged(&merged, "/tmp/workspace", ConfigRuntimeOptions::default()); + + assert!(!disabled_rt + .tool_permissions + .is_tool_visible_for_agent("build", "task")); + // Other tools stay available. + assert!(disabled_rt + .tool_permissions + .is_tool_visible_for_agent("build", "read")); + // Default keeps subagents enabled. + assert!(default_rt + .tool_permissions + .is_tool_visible_for_agent("build", "task")); + + match prev { + Some(value) => std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, value), + None => std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV), + } + } + #[test] fn tui_and_print_share_same_tool_and_instruction_wiring() { let mut merged = MergedConfig::default(); diff --git a/src/main.rs b/src/main.rs index ae505e4..57831f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -393,6 +393,7 @@ async fn run_print_mode( reasoning_override: Option, no_session_persistence: bool, dangerously_skip_permissions: bool, + disable_subagents: bool, cli_agent: Option<&str>, ) -> Result<()> { use crate::llm::client::stream_llm_with_cancellation; @@ -448,6 +449,7 @@ async fn run_print_mode( crate::config::ConfigRuntimeOptions { print_mode: true, dangerously_skip_permissions, + disable_subagents, }, ); let discovery = runtime.discovery; @@ -785,6 +787,12 @@ pub(crate) struct Args { #[arg(long = "dangerously-skip-permissions")] dangerously_skip_permissions: bool, + /// Disable subagents (hides the `task` tool and rejects subagent calls). + /// Same as setting `CRABCODE_DISABLE_SUBAGENTS=1`. Applies to TUI, + /// print mode, ACP, and serve. + #[arg(long = "disable-subagents", global = true)] + disable_subagents: bool, + #[arg(long = "emit-logs", hide = true)] emit_logs: bool, @@ -1042,6 +1050,14 @@ async fn main() -> Result<()> { let _ = crate::logging::log(msg); }); + // Converge flag + env into the single `CRABCODE_DISABLE_SUBAGENTS` + // seam before any runtime (TUI, print, ACP, serve) is constructed, so + // permission checks, registry omission, prompt suppression, and + // `TaskTool::execute` all agree without threading a bool everywhere. + if args.disable_subagents { + std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, "1"); + } + if args.test_notification { send_test_notification()?; return Ok(()); @@ -1222,6 +1238,7 @@ async fn main() -> Result<()> { args.reasoning_effort, args.no_session_persistence, args.dangerously_skip_permissions, + crate::tools::task::resolve_subagents_disabled(args.disable_subagents), args.agent.as_deref(), ) .await; @@ -1431,6 +1448,21 @@ mod tests { assert_eq!(args.agent.as_deref(), Some("plan")); } + #[test] + fn disable_subagents_defaults_off_and_parses_flag() { + let args = Args::try_parse_from(["crabcode", "-p", "hi"]).unwrap(); + assert!(!args.disable_subagents); + + let args = Args::try_parse_from(["crabcode", "-p", "hi", "--disable-subagents"]).unwrap(); + assert!(args.disable_subagents); + } + + #[test] + fn disable_subagents_flag_is_global_for_subcommands() { + let args = Args::try_parse_from(["crabcode", "--disable-subagents", "acp"]).unwrap(); + assert!(args.disable_subagents); + } + #[test] fn resolve_startup_agent_prefers_cli_over_default() { let registry = crate::agent::definition::AgentRegistry::default(); diff --git a/src/prompt/mod.rs b/src/prompt/mod.rs index 0a148a3..1f4e82b 100644 --- a/src/prompt/mod.rs +++ b/src/prompt/mod.rs @@ -418,7 +418,14 @@ Your output will be displayed on a command line interface. Your responses should } } } - let subagents = registry.visible_subagents(); + // Add available subagents listing (suppressed when subagents are + // disabled so the model does not spend tokens attempting `task` + // calls that will be rejected). + let subagents = if crate::tools::task::subagents_disabled() { + Vec::new() + } else { + registry.visible_subagents() + }; if !subagents.is_empty() { let subagents_xml = subagents .iter() diff --git a/src/tools/init.rs b/src/tools/init.rs index f7ed330..b37a33d 100644 --- a/src/tools/init.rs +++ b/src/tools/init.rs @@ -179,13 +179,20 @@ pub async fn register_dynamic_tools( )) .await; - registry - .register(Arc::new( - TaskTool::new(registry.clone()) - .with_sender_opt(sender.clone()) - .with_runtime_options(permissions, agent_registry, cancel_token), - )) - .await; + // Omit (don't just deny) when disabled so `registry.get("task")` is + // None and the model never sees the tool. The flag path sets + // `CRABCODE_DISABLE_SUBAGENTS=1` early in `main`, so this single env + // check covers flag + env without threading a bool through every + // `register_dynamic_tools` call site (TUI, print, ACP, serve). + if !super::task::subagents_disabled() { + registry + .register(Arc::new( + TaskTool::new(registry.clone()) + .with_sender_opt(sender.clone()) + .with_runtime_options(permissions, agent_registry, cancel_token), + )) + .await; + } // Keep terminal_session as a thin interactive alias for back-compat. registry @@ -277,6 +284,9 @@ mod tests { #[tokio::test] async fn dynamic_registry_contains_runtime_tools() { + let _lock = crate::tools::task::disable_subagents_env_lock(); + let prev = std::env::var(crate::tools::task::DISABLE_SUBAGENTS_ENV).ok(); + std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV); let registry = initialize_tool_registry_with_dynamic( None, ToolPermissions::new("."), @@ -292,10 +302,40 @@ mod tests { assert!(registry.get("bash_output").await.is_some()); assert!(registry.get("bash_kill").await.is_some()); assert!(registry.get("bash_restart").await.is_some()); + match prev { + Some(value) => std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, value), + None => std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV), + } + } + + #[tokio::test] + async fn disabled_env_omits_task_tool_from_dynamic_registry() { + let _lock = crate::tools::task::disable_subagents_env_lock(); + let prev = std::env::var(crate::tools::task::DISABLE_SUBAGENTS_ENV).ok(); + std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, "1"); + let registry = initialize_tool_registry_with_dynamic( + None, + ToolPermissions::new("."), + crate::agent::definition::AgentRegistry::default(), + CancellationToken::new(), + Arc::new(ProcessRegistry::new()), + ) + .await; + + assert!(registry.get("task").await.is_none()); + // Interactive tools stay registered. + assert!(registry.get("question").await.is_some()); + match prev { + Some(value) => std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, value), + None => std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV), + } } #[tokio::test] async fn scoped_plan_registry_hides_mutating_tools() { + let _lock = crate::tools::task::disable_subagents_env_lock(); + let prev = std::env::var(crate::tools::task::DISABLE_SUBAGENTS_ENV).ok(); + std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV); let permissions = ToolPermissions::new("."); let registry = initialize_tool_registry_with_dynamic( None, @@ -317,6 +357,10 @@ mod tests { assert!(scoped.get("apply_patch").await.is_none()); assert!(scoped.get("write").await.is_none()); assert!(scoped.get("edit").await.is_none()); + match prev { + Some(value) => std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, value), + None => std::env::remove_var(crate::tools::task::DISABLE_SUBAGENTS_ENV), + } } #[tokio::test] diff --git a/src/tools/permission.rs b/src/tools/permission.rs index 82ef798..5bd2998 100644 --- a/src/tools/permission.rs +++ b/src/tools/permission.rs @@ -299,6 +299,14 @@ impl ToolPermissions { } pub fn is_tool_allowed_for_agent(&self, agent_mode: &str, tool_id: &str) -> bool { + // Central kill-switch: `CRABCODE_DISABLE_SUBAGENTS=1` hides the + // `task` tool in every runtime (TUI, print, ACP, serve) without + // threading a flag through each entrypoint. Explicit + // `--disable-subagents` sets this env early in `main`, so both + // sources converge here plus the `ConfigRuntime` deny rule. + if tool_id == "task" && super::task::subagents_disabled() { + return false; + } self.agent_policies.is_allowed(agent_mode, tool_id) && self .global_tool_config diff --git a/src/tools/task.rs b/src/tools/task.rs index c5ba59f..0631f8a 100644 --- a/src/tools/task.rs +++ b/src/tools/task.rs @@ -15,6 +15,42 @@ pub struct TaskTool { permissions: Option, agent_registry: AgentRegistry, cancel_token: CancellationToken, + subagents_disabled: bool, +} + +/// Env var that disables subagents (`task` tool) in every runtime +/// (TUI, print mode, ACP, serve). Truthy values: `1`, `true`, `yes`, +/// `y`, `on` (case-insensitive). Same convention as +/// `CRABCODE_DISABLE_CLAUDE_CODE`. +pub const DISABLE_SUBAGENTS_ENV: &str = "CRABCODE_DISABLE_SUBAGENTS"; + +fn env_truthy_value(value: &str) -> bool { + matches!( + value.trim().to_ascii_lowercase().as_str(), + "1" | "true" | "yes" | "y" | "on" + ) +} + +/// Returns true when subagents are disabled via [`DISABLE_SUBAGENTS_ENV`]. +/// Pure env read so every runtime (including ACP/serve, which have no CLI +/// flag threading) enforces the same seam without extra plumbing. +pub fn subagents_disabled() -> bool { + std::env::var(DISABLE_SUBAGENTS_ENV) + .map(|value| env_truthy_value(&value)) + .unwrap_or(false) +} + +/// Flag/env precedence: an explicit `--disable-subagents` flag or a truthy +/// env value disables subagents. There is no re-enable override: once +/// either source says disabled, subagents stay disabled. +pub fn resolve_subagents_disabled(cli_flag: bool) -> bool { + cli_flag || subagents_disabled() +} + +#[cfg(test)] +pub(crate) fn disable_subagents_env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(|e| e.into_inner()) } fn stream_chunk_can_batch(chunk: &crate::llm::ChunkMessage) -> bool { @@ -275,6 +311,50 @@ mod tests { Some("parent-model") ); } + + #[test] + fn disable_env_truthy_values_match_existing_conventions() { + for truthy in ["1", "true", "TRUE", "yes", "y", "on", "On"] { + assert!(env_truthy_value(truthy), "{truthy} should disable"); + } + for falsy in ["", "0", "false", "no", "off", "2", "maybe"] { + assert!(!env_truthy_value(falsy), "{falsy} should not disable"); + } + } + + #[test] + fn resolve_flag_or_env_disables_with_flag_winning() { + // Serialized: mutates the process env. When the env is unset, + // the flag alone decides. + let _lock = disable_subagents_env_lock(); + let prev = std::env::var(DISABLE_SUBAGENTS_ENV).ok(); + std::env::remove_var(DISABLE_SUBAGENTS_ENV); + assert!(!resolve_subagents_disabled(false)); + assert!(resolve_subagents_disabled(true)); + match prev { + Some(value) => std::env::set_var(DISABLE_SUBAGENTS_ENV, value), + None => std::env::remove_var(DISABLE_SUBAGENTS_ENV), + } + } + + #[test] + fn disabled_task_tool_rejects_without_running_subagent() { + let task = TaskTool::new(ToolRegistry::new()).with_subagents_disabled(true); + let params = serde_json::json!({ + "subagent_type": "explore", + "description": "test", + "prompt": "look around" + }); + let ctx = + ToolContext::from_cancel_token("session", "message", "Build", CancellationToken::new()); + + let result = tokio_test::block_on(task.execute(params, &ctx)); + assert!( + matches!(result, Err(ToolError::Permission(ref msg)) if msg.contains("Subagents are disabled")), + "expected disabled rejection, got {:?}", + result + ); + } } impl TaskTool { @@ -285,9 +365,22 @@ impl TaskTool { permissions: None, agent_registry: AgentRegistry::default(), cancel_token: CancellationToken::new(), + subagents_disabled: false, } } + /// Explicit opt-out without touching the process env. Preferred in + /// tests and in call sites that already resolved the CLI flag, so + /// regression tests avoid global env races. + pub fn with_subagents_disabled(mut self, disabled: bool) -> Self { + self.subagents_disabled = disabled; + self + } + + fn subagents_disabled_effective(&self) -> bool { + self.subagents_disabled || subagents_disabled() + } + pub fn with_sender_opt(mut self, sender: Option) -> Self { self.sender = sender; self @@ -364,6 +457,12 @@ impl ToolHandler for TaskTool { } async fn execute(&self, params: Value, ctx: &ToolContext) -> Result { + if self.subagents_disabled_effective() { + return Err(ToolError::Permission( + "Subagents are disabled (task tool). Re-run without --disable-subagents or unset CRABCODE_DISABLE_SUBAGENTS to enable them.".to_string(), + )); + } + let subagent_type_str = get_string_param(¶ms, "subagent_type").unwrap_or_default(); let description = get_string_param(¶ms, "description").unwrap_or_default(); let prompt = get_string_param(¶ms, "prompt").unwrap_or_default();