fix: reject a subagent type whose tools list resolves to nothing

A .sigit/agents/*.md type with a tools: field naming only tools outside the subagent read-only set (for instance grep or bash carried over from a Claude Code config) resolved to an empty allow-list. The task ran anyway: the subagent was offered zero tools, so it answered from the model's priors with no file access, and the only trace was a log-file warning the user never sees. effective_subagent_tools now returns an error in that case, and the task tool surfaces it to the caller instead of running a toolless subagent. A type that lists a mix of valid and invalid tools still narrows to the valid subset, and one with no tools: field still inherits the full default set.

paydii committed Jul 11, 2026 at 15:10 UTC 16655e188dc0c48d107b15ebb13cd8b61b9626af
1 file changed +50 -10
src/tools.rs
+50 -10
@@ -688,13 +688,15 @@ fn subagent_tool_specs(allowed: &[String]) -> Vec<ToolSpec> {
688 /// base read-only set; anything else it lists is dropped with a warning
689 /// rather than silently granted, since that's the whole security property
690 /// this function exists to enforce (see the module doc above).
691 -fn effective_subagent_tools(agent_type: Option<&subagents::AgentType>) -> Vec<String> {
691 +fn effective_subagent_tools(
692 + agent_type: Option<&subagents::AgentType>,
693 +) -> Result<Vec<String>, String> {
694 let Some(agent_type) = agent_type else {
693 - return SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();
695 + return Ok(SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect());
696 };
697 let Some(requested) = &agent_type.tools else {
698 // No `tools:` field: inherit the full default read-only set.
697 - return SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();
699 + return Ok(SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect());
700 };
701
702 let mut effective = Vec::new();
@@ -711,7 +713,23 @@ fn effective_subagent_tools(agent_type: Option<&subagents::AgentType>) -> Vec<St
713 );
714 }
715 }
714 - effective
716 +
717 + // A `tools:` field that intersects the ceiling to nothing is a
718 + // misconfiguration, not a request for a toolless subagent: running it
719 + // anyway would answer from the model's priors with no file access and no
720 + // signal beyond a log line. Surface it as an error instead.
721 + if effective.is_empty() {
722 + return Err(format!(
723 + "Error: subagent type \"{}\" at {} sets `tools: {}`, none of which \
724 + is in the subagent read-only set ({}). Fix the type's `tools:` \
725 + field, or omit it to inherit the default read-only tools.",
726 + agent_type.name,
727 + agent_type.path.display(),
728 + requested.join(", "),
729 + SUBAGENT_TOOL_NAMES.join(", "),
730 + ));
731 + }
732 + Ok(effective)
733 }
734
735 /// Spec for the `task` tool. Lives in the `*_as_specs`/`build_tool_specs`
@@ -829,7 +847,10 @@ async fn exec_task_with(arguments: &str, factory: Option<&SubagentFactory>) -> S
847 .as_ref()
848 .map(|t| t.system_prompt.as_str())
849 .unwrap_or(SUBAGENT_SYSTEM_PROMPT);
832 - let allowed_tools = effective_subagent_tools(agent_type.as_ref());
850 + let allowed_tools = match effective_subagent_tools(agent_type.as_ref()) {
851 + Ok(tools) => tools,
852 + Err(err) => return err,
853 + };
854
855 let Some(backend) = factory.and_then(|build| build(system_prompt)) else {
856 return SUBAGENT_UNAVAILABLE.to_string();
@@ -3330,7 +3351,7 @@ mod tests {
3351
3352 #[test]
3353 fn test_subagent_toolset_is_read_only() {
3333 - let specs = subagent_tool_specs(&effective_subagent_tools(None));
3354 + let specs = subagent_tool_specs(&effective_subagent_tools(None).unwrap());
3355 let mut names: Vec<&str> = specs.iter().map(|spec| spec.name.as_str()).collect();
3356 names.sort_unstable();
3357 assert_eq!(
@@ -3762,7 +3783,7 @@ mod tests {
3783
3784 #[test]
3785 fn effective_subagent_tools_defaults_to_full_read_only_set_without_a_type() {
3765 - let mut tools = effective_subagent_tools(None);
3786 + let mut tools = effective_subagent_tools(None).unwrap();
3787 tools.sort();
3788 let mut expected: Vec<String> = SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();
3789 expected.sort();
@@ -3778,7 +3799,7 @@ mod tests {
3799 system_prompt: "Be helpful.".to_string(),
3800 path: PathBuf::from("/x/generalist.md"),
3801 };
3781 - let mut tools = effective_subagent_tools(Some(&agent_type));
3802 + let mut tools = effective_subagent_tools(Some(&agent_type)).unwrap();
3803 tools.sort();
3804 let mut expected: Vec<String> = SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();
3805 expected.sort();
@@ -3794,7 +3815,7 @@ mod tests {
3815 system_prompt: "Read notes.".to_string(),
3816 path: PathBuf::from("/x/notes-reader.md"),
3817 };
3797 - let tools = effective_subagent_tools(Some(&agent_type));
3818 + let tools = effective_subagent_tools(Some(&agent_type)).unwrap();
3819 assert_eq!(tools, vec!["read_file".to_string(), "glob".to_string()]);
3820 }
3821
@@ -3815,10 +3836,29 @@ mod tests {
3836 system_prompt: "...".to_string(),
3837 path: PathBuf::from("/x/sneaky.md"),
3838 };
3818 - let tools = effective_subagent_tools(Some(&agent_type));
3839 + let tools = effective_subagent_tools(Some(&agent_type)).unwrap();
3840 assert_eq!(tools, vec!["read_file".to_string()]);
3841 assert!(!tools.contains(&"edit_file".to_string()));
3842 assert!(!tools.contains(&"run_command".to_string()));
3843 assert!(!tools.contains(&"task".to_string()));
3844 }
3845 +
3846 + #[test]
3847 + fn effective_subagent_tools_errors_when_allow_list_intersects_to_nothing() {
3848 + // A type whose `tools:` names only out-of-ceiling tools (e.g. Claude
3849 + // Code names carried over) must error, not silently run toolless.
3850 + let agent_type = subagents::AgentType {
3851 + name: "misconfigured".to_string(),
3852 + description: "names only invalid tools".to_string(),
3853 + tools: Some(vec!["grep".to_string(), "bash".to_string()]),
3854 + system_prompt: "...".to_string(),
3855 + path: PathBuf::from("/x/misconfigured.md"),
3856 + };
3857 + let err = effective_subagent_tools(Some(&agent_type)).unwrap_err();
3858 + assert!(
3859 + err.contains("misconfigured"),
3860 + "message names the type: {err}"
3861 + );
3862 + assert!(err.contains("read-only set"), "message explains why: {err}");
3863 + }
3864 }