@sigitcodereview / sigit / commits / 347a11b

feat: add configurable subagent types (.sigit/agents/*.md)

A subagent type is a Markdown file with YAML frontmatter (name, description, and an optional comma-separated tools: allow-list) whose body becomes the task tool's system prompt for that run. Discovered from .sigit/agents/ and .claude/agents/ in the project, then $SIGIT_CONFIG_DIR/agents/ and ~/.claude/agents/ (personal) — same dual-root pattern as skills.rs. Passing subagent_type: "code-reviewer" to task swaps in that type's prompt instead of the generic research-subagent one; task's tool description lists discovered types so the model knows when to use one. Security-critical part: a type's tools: field can only ever narrow the subagent's toolset, never widen it. tools.rs still owns SUBAGENT_TOOL_NAMES (the hard read-only ceiling) and intersects it against whatever a type requests in effective_subagent_tools — so a .sigit/agents/*.md file cannot grant itself edit_file or run_command and bypass the permission system just by listing it in tools:. Anything outside the ceiling is dropped with a logged warning, not silently ignored. Covered by both pure unit tests (effective_subagent_tools_*) and an end-to-end test that runs a custom type through a real scripted backend and checks the actual offered/gated toolset. Required threading a per-call system prompt through SubagentFactory (previously baked in once at registration) so the same registered factory can serve the default prompt or any resolved type's prompt. Also adds /agents to list what's discovered, mirroring /skills. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

paydii committed Jul 10, 2026 at 23:13 UTC 347a11bd49ec9278746c9616a22328bcb9be12cd

5 files changed +370 -84

.agents/AGENTS.md

+19 -3

@@ -110,7 +110,13 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

110 spec list (`all_tools`) and the execute `match` (`execute_tool`). `run_command` also enforces

111 commit attribution: when a command creates a new commit that lacks the

112 `Co-Authored-By: siGit Code` trailer (`COMMIT_CO_AUTHOR_TRAILER`), it amends the trailer in —

113 - unless the commit already exists on a remote, which is never rewritten.

113 + unless the commit already exists on a remote, which is never rewritten. Also owns the `task`

114 + tool: a nested agent loop in a fresh conversation, offered only when `subagent_available()`

115 + (a subagent factory is registered — see `register_subagent_factory_for` in `main.rs`; on-device

116 + registers a `None`-returning factory since onde has a single shared history). A subagent's

117 + toolset is a hard-gated read-only allow-list (`SUBAGENT_TOOL_NAMES`) that is never expanded by a

118 + configurable subagent type (see `src/subagents.rs`) — only ever narrowed — so a `.sigit/agents/*.md`

119 + file can't grant itself `edit_file`/`run_command` and bypass the permission system.

120 - **`src/skills.rs`** — [Agent Skills](https://agentskills.io) support. Discovers skill

121 folders (each with a `SKILL.md`: YAML frontmatter `name` + `description`, then Markdown

122 instructions) from `.sigit/skills/` and `.claude/skills/` in the cwd, `$SIGIT_CONFIG_DIR/skills/`,

@@ -119,6 +125,16 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

125 calls `skill` with a name) loads the full `SKILL.md` body. The `skill` tool is appended in the

126 `*_as_specs`/`build_tool_specs` layer (not in `all_tools()`) so its description can be dynamic,

127 and only when at least one skill exists.

128 +- **`src/subagents.rs`** — configurable subagent types for the `task` tool. Discovers Markdown

129 + files (YAML frontmatter `name` + `description`, optional comma-separated `tools:` allow-list,

130 + then a Markdown body that becomes the subagent's system prompt) from `.sigit/agents/` and

131 + `.claude/agents/` in the cwd, `$SIGIT_CONFIG_DIR/agents/`, and `~/.claude/agents/`. Passing a

132 + type's `name` as `task`'s `subagent_type` argument swaps in that system prompt and, if `tools:`

133 + is set, narrows the offered toolset to its *intersection* with `SUBAGENT_TOOL_NAMES` — the

134 + security-relevant narrowing logic lives in `tools.rs` next to that constant, not here; this

135 + module only discovers and parses files. `SubagentFactory` (in `tools.rs`) takes the resolved

136 + system prompt per call rather than baking one in at registration, so a single registered

137 + factory serves both the default research subagent and every configured type.

138 - **`src/mcp.rs`** — [Model Context Protocol](https://modelcontextprotocol.io) *client*. Two

139 transports: **Streamable HTTP** (one JSON-RPC POST endpoint, `url` in `mcp.toml`; replies are

140 `application/json` or SSE) and **stdio** (`command` + optional `args`/`[server.env]` in

@@ -165,8 +181,8 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

181 - **`src/credentials.rs`** — local session-token store (TOML, `0600` on Unix).

182 - **`src/models.rs`** — model-picker types shared across platforms.

183

168 -Slash commands (`/help`, `/models`, `/skills`, `/mcp`, `/login`, `/logout`, `/whoami`, `/reload`,

169 -`/plan`, `/permissions`, `/init`, `/clear`, `/status`) are advertised via `advertise_commands` in

184 +Slash commands (`/help`, `/models`, `/skills`, `/agents`, `/mcp`, `/login`, `/logout`, `/whoami`,

185 +`/reload`, `/plan`, `/permissions`, `/init`, `/clear`, `/status`) are advertised via `advertise_commands` in

186 `main.rs` and handled in both the TUI and ACP sessions. `/init` is special: instead of replying

187 directly it substitutes `instructions::INIT_PROMPT` for the user text and runs a normal agent

188 turn that explores the repo and writes (or improves) `AGENTS.md` through the ordinary tools and

src/chat.rs

@@ -1356,6 +1356,8 @@ mod tui {

1356 Local(Option<bool>),

1357 /// List discovered Agent Skills.

1358 Skills,

1359 + /// List configured subagent types (`.sigit/agents/*.md`) for `task`.

1360 + Agents,

1361 /// List configured MCP servers and their tools.

1362 Mcp,

1363 /// explicitly load the selected (or default) on-device model

@@ -1401,6 +1403,7 @@ mod tui {

1403 "/models" => SlashCommand::Models(arg.and_then(|s| s.parse::<usize>().ok())),

1404 "/local" => SlashCommand::Local(parse_on_off(arg)),

1405 "/skills" => SlashCommand::Skills,

1406 + "/agents" => SlashCommand::Agents,

1407 "/mcp" => SlashCommand::Mcp,

1408 "/load" => SlashCommand::Load,

1409 "/login" => SlashCommand::Login(arg.map(str::to_string)),

@@ -2120,6 +2123,7 @@ mod tui {

2123 /models N — switch to model N\n\

2124 /local [on|off]— toggle on-device inference mode\n\

2125 /skills — list available Agent Skills\n\

2126 + /agents — list configured subagent types (.sigit/agents/*.md)\n\

2127 /mcp — list MCP servers and their tools\n\

2128 /load — load the selected on-device model\n\

2129 /login E P — sign in to siGit Code Cloud\n\

@@ -2237,6 +2241,11 @@ mod tui {

2241 app.messages

2242 .push(ChatMessage::system(crate::skills::format_skills_list()));

2243 }

2244 + SlashCommand::Agents => {

2245 + app.messages.push(ChatMessage::system(

2246 + crate::subagents::format_agent_types_list(),

2247 + ));

2248 + }

2249 SlashCommand::Mcp => {

2250 app.messages

2251 .push(ChatMessage::system(crate::mcp::status_summary()));

src/main.rs

+24 -9

@@ -323,19 +323,21 @@ fn agent_tools_as_specs() -> Vec<ToolSpec> {

323

324 /// Register the `task` tool's subagent factory for an OpenAI-compatible

325 /// provider: each subagent run gets a FRESH `OpenAiBackend` against the same

326 -/// endpoint (its own conversation history), seeded with the subagent system

327 -/// prompt. The provider config is captured by clone. Called once at startup by

328 -/// whichever surface resolved the provider; on-device paths register a factory

329 -/// that returns `None` instead (onde has a single shared history, so a second

330 -/// concurrent context is not possible yet).

326 +/// endpoint (its own conversation history), seeded with whichever system

327 +/// prompt the caller resolved (the default subagent prompt, or a configured

328 +/// subagent type's prompt — see `tools::exec_task_with`). The provider config

329 +/// is captured by clone. Called once at startup by whichever surface resolved

330 +/// the provider; on-device paths register a factory that returns `None`

331 +/// instead (onde has a single shared history, so a second concurrent context

332 +/// is not possible yet).

333 fn register_subagent_factory_for(cfg: &provider::ProviderConfig) {

334 let cfg = cfg.clone();

333 - tools::set_subagent_factory(Box::new(move || {

335 + tools::set_subagent_factory(Box::new(move |system_prompt: &str| {

336 Some(Arc::new(OpenAiBackend::new(

337 cfg.base_url.clone(),

338 cfg.api_key.clone(),

339 cfg.model.clone(),

338 - Some(tools::SUBAGENT_SYSTEM_PROMPT.to_string()),

340 + Some(system_prompt.to_string()),

341 )) as Arc<dyn InferenceBackend>)

342 }));

343 }

@@ -786,6 +788,10 @@ impl SiGitAgent {

788 "on|off (optional)",

789 ),

790 AvailableCommand::new("skills", "List available Agent Skills"),

791 + AvailableCommand::new(

792 + "agents",

793 + "List configured subagent types (.sigit/agents/*.md) for task",

794 + ),

795 AvailableCommand::new("mcp", "List MCP servers and their tools"),

796 AvailableCommand::new("load", "Load the selected on-device model"),

797 with_hint("login", "Sign in to siGit Code Cloud", "<email> <password>"),

@@ -2225,6 +2231,8 @@ enum SlashCommand {

2231 Local(Option<bool>),

2232 /// List discovered Agent Skills.

2233 Skills,

2234 + /// List configured subagent types (`.sigit/agents/*.md`) for `task`.

2235 + Agents,

2236 /// List configured MCP servers and their tools.

2237 Mcp,

2238 /// Explicitly load the selected (or default) on-device model.

@@ -2264,6 +2272,7 @@ fn parse_slash(input: &str) -> Option<SlashCommand> {

2272 "/models" => SlashCommand::Models(argument.and_then(|v| v.parse::<usize>().ok())),

2273 "/local" => SlashCommand::Local(parse_on_off(argument)),

2274 "/skills" => SlashCommand::Skills,

2275 + "/agents" => SlashCommand::Agents,

2276 "/mcp" => SlashCommand::Mcp,

2277 "/load" => SlashCommand::Load,

2278 "/login" => SlashCommand::Login(argument.map(str::to_string)),

@@ -2376,6 +2385,7 @@ async fn exec_slash_acp(

2385 /models N - switch to model N\n\

2386 /local [on|off]- toggle on-device inference mode\n\

2387 /skills - list available Agent Skills\n\

2388 + /agents - list configured subagent types (.sigit/agents/*.md)\n\

2389 /mcp - list MCP servers and their tools\n\

2390 /load - load the selected on-device model\n\

2391 /login E P - sign in to siGit Code Cloud\n\

@@ -2465,6 +2475,11 @@ async fn exec_slash_acp(

2475 .send_assistant_message(cx, session_id, skills::format_skills_list())

2476 .ok();

2477 }

2478 + SlashCommand::Agents => {

2479 + agent

2480 + .send_assistant_message(cx, session_id, subagents::format_agent_types_list())

2481 + .ok();

2482 + }

2483 SlashCommand::Mcp => {

2484 agent

2485 .send_assistant_message(cx, session_id, mcp::status_summary())

@@ -2869,7 +2884,7 @@ async fn run_interactive(tty: std::fs::File, mut cleanup_tty: std::fs::File) ->

2884 let _ = load_tx.send(Ok(()));

2885 // On-device inference has a single shared history; no

2886 // subagent context is possible yet.

2872 - tools::set_subagent_factory(Box::new(|| None));

2887 + tools::set_subagent_factory(Box::new(|_system_prompt: &str| None));

2888 let backend = Arc::new(LocalBackend::new(Arc::clone(&engine)))

2889 as Arc<dyn InferenceBackend>;

2890 (backend, startup_model_name)

@@ -3003,7 +3018,7 @@ async fn run_acp_server() -> anyhow::Result<()> {

3018 } else {

3019 // On-device inference has a single shared conversation history, so a

3020 // second concurrent subagent context is not possible yet.

3006 - tools::set_subagent_factory(Box::new(|| None));

3021 + tools::set_subagent_factory(Box::new(|_system_prompt: &str| None));

3022 }

3023

3024 let stdin = tokio::io::stdin().compat();

src/subagents.rs

+3 -1

@@ -33,7 +33,9 @@

34 use std::path::{Path, PathBuf};

36 -use crate::skills::{extract_frontmatter, parse_frontmatter_fields, strip_frontmatter, validate_name};

36 +use crate::skills::{

37 + extract_frontmatter, parse_frontmatter_fields, strip_frontmatter, validate_name,

38 +};

40 /// A discovered subagent type.

41 #[derive(Debug, Clone, PartialEq, Eq)]

src/tools.rs

+315 -71

@@ -10,6 +10,7 @@ use std::sync::atomic::{AtomicU64, Ordering};

10 use std::sync::{Arc, Mutex, OnceLock};

12 use crate::backend::{InferenceBackend, ToolResult, ToolSpec};

13 +use crate::subagents;

15 const WEBSITE_READ_CHAR_LIMIT: usize = 20_000;

16 const WEBSITE_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20);

@@ -468,6 +469,13 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {

469 // toolset is strictly read-only and never includes `task` itself (no

470 // recursion), so a delegated agent can research but not mutate state.

471 //

472 +// A caller may optionally pick a *configurable subagent type* (see

473 +// `crate::subagents`, `.sigit/agents/*.md`) via the `subagent_type` argument.

474 +// A type can only ever replace the system prompt and *narrow* the toolset —

475 +// never grant tools outside `SUBAGENT_TOOL_NAMES` — so this stays exactly as

476 +// safe as the default research subagent: a `.sigit/agents/*.md` file cannot

477 +// grant itself `edit_file` or `run_command` and bypass the permission system.

478 +//

479 // This module is backend-agnostic and cannot construct a backend, so the

480 // surface that knows the active provider (`run_acp_server` / `run_interactive`

481 // in `main.rs`) registers a factory at startup. The factory returns `None`

@@ -476,7 +484,14 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {

484

485 pub const TASK_TOOL_NAME: &str = "task";

486

479 -/// The tool names a subagent may call, filtered from [`all_tools`].

487 +/// Hard cap on how many configurable subagent types are embedded in the

488 +/// `task` tool's description, to bound the tool description size — same

489 +/// rationale as `skills::MAX_ADVERTISED_SKILLS`.

490 +const MAX_ADVERTISED_AGENT_TYPES: usize = 100;

491 +

492 +/// The tool names a subagent may call, filtered from [`all_tools`]. This is

493 +/// the hard ceiling for every subagent, default or custom-typed — see the

494 +/// module doc above.

495 const SUBAGENT_TOOL_NAMES: &[&str] = &[

496 "read_file",

497 "list_directory",

@@ -485,7 +500,8 @@ const SUBAGENT_TOOL_NAMES: &[&str] = &[

500 "read_website",

501 ];

502

488 -/// System prompt seeding every subagent conversation.

503 +/// System prompt seeding a subagent conversation when no `subagent_type` is

504 +/// given (or it doesn't resolve to a configured type).

505 pub const SUBAGENT_SYSTEM_PROMPT: &str = "You are a focused research subagent. \

506 You are given one self-contained task by a calling agent. Investigate it using \

507 the read-only tools available to you (read_file, list_directory, search_files, \

@@ -504,9 +520,12 @@ const SUBAGENT_UNAVAILABLE: &str = "The task tool is not available on-device \

520 yet: on-device inference has a single conversation context. Do the research \

521 yourself with the read-only tools.";

522

507 -/// Builds a fresh backend for one subagent run, or `None` when the active

508 -/// inference cannot host a second conversation (on-device).

509 -pub type SubagentFactory = Box<dyn Fn() -> Option<Arc<dyn InferenceBackend>> + Send + Sync>;

523 +/// Builds a fresh backend for one subagent run seeded with the given system

524 +/// prompt, or `None` when the active inference cannot host a second

525 +/// conversation (on-device). Takes the prompt per call (rather than baking

526 +/// one in at registration) so a configurable subagent type's prompt can

527 +/// replace [`SUBAGENT_SYSTEM_PROMPT`] on a call-by-call basis.

528 +pub type SubagentFactory = Box<dyn Fn(&str) -> Option<Arc<dyn InferenceBackend>> + Send + Sync>;

529

530 static SUBAGENT_FACTORY: OnceLock<SubagentFactory> = OnceLock::new();

531

@@ -519,19 +538,22 @@ pub fn set_subagent_factory(factory: SubagentFactory) {

538 /// Whether a `task` call could run right now: a factory is registered and it

539 /// can build a backend. The spec builders (`agent_tools_as_specs` /

540 /// `build_tool_specs`) use this to offer the tool only when it works, the same

522 -/// conditional pattern as the `skill` tool.

541 +/// conditional pattern as the `skill` tool. Building a backend just to probe

542 +/// is cheap (no I/O happens until the first message), so this passes a

543 +/// placeholder prompt and discards the result.

544 pub fn subagent_available() -> bool {

545 SUBAGENT_FACTORY

546 .get()

526 - .is_some_and(|factory| factory().is_some())

547 + .is_some_and(|factory| factory(SUBAGENT_SYSTEM_PROMPT).is_some())

548 }

549

550 /// The read-only toolset offered to a subagent, filtered from [`all_tools`] by

530 -/// name. Never contains `task` (recursion) or any mutating tool.

531 -pub fn subagent_tool_specs() -> Vec<ToolSpec> {

551 +/// name. `allowed` must already be a subset of [`SUBAGENT_TOOL_NAMES`] —

552 +/// see [`effective_subagent_tools`], the only intended way to build one.

553 +fn subagent_tool_specs(allowed: &[String]) -> Vec<ToolSpec> {

554 all_tools()

555 .into_iter()

534 - .filter(|tool| SUBAGENT_TOOL_NAMES.contains(&tool.name))

556 + .filter(|tool| allowed.iter().any(|name| name == tool.name))

557 .map(|tool| ToolSpec {

558 name: tool.name.to_string(),

559 description: tool.description.to_string(),

@@ -540,22 +562,75 @@ pub fn subagent_tool_specs() -> Vec<ToolSpec> {

562 .collect()

563 }

564

565 +/// The toolset a subagent call should actually get: [`SUBAGENT_TOOL_NAMES`]

566 +/// by default, or — when `agent_type` sets a `tools:` allow-list — its

567 +/// *intersection* with that ceiling. A type can never add a tool outside the

568 +/// base read-only set; anything else it lists is dropped with a warning

569 +/// rather than silently granted, since that's the whole security property

570 +/// this function exists to enforce (see the module doc above).

571 +fn effective_subagent_tools(agent_type: Option<&subagents::AgentType>) -> Vec<String> {

572 + let Some(agent_type) = agent_type else {

573 + return SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();

574 + };

575 + let Some(requested) = &agent_type.tools else {

576 + // No `tools:` field: inherit the full default read-only set.

577 + return SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();

578 + };

579 +

580 + let mut effective = Vec::new();

581 + for name in requested {

582 + if SUBAGENT_TOOL_NAMES.contains(&name.as_str()) {

583 + effective.push(name.clone());

584 + } else {

585 + log::warn!(

586 + "agent type \"{}\" at {} lists tool \"{name}\", which is not in the \

587 + subagent read-only allow-list ({}) — ignoring it",

588 + agent_type.name,

589 + agent_type.path.display(),

590 + SUBAGENT_TOOL_NAMES.join(", ")

591 + );

592 + }

593 + }

594 + effective

595 +}

596 +

597 /// Spec for the `task` tool. Lives in the `*_as_specs`/`build_tool_specs`

598 /// layer (like `skill` and MCP tools), not in [`all_tools`], because it is

599 /// only offered when [`subagent_available`] is true.

600 pub fn task_tool_spec() -> ToolSpec {

601 + let types = subagents::discover_agent_types();

602 + let mut description = "Delegate a self-contained task to a subagent that \

603 + runs in a fresh conversation and returns only its final answer. The \

604 + subagent's tools are read-only by default (read files, list \

605 + directories, search, glob, read websites) — it cannot modify \

606 + anything. Use this for exploratory questions whose intermediate \

607 + file reads would otherwise clutter this conversation (e.g. \"where \

608 + is X implemented and how does it work?\"). The subagent cannot see \

609 + this conversation, so the prompt must be fully self-contained: \

610 + include absolute paths, symbol names, and exactly what the answer \

611 + should contain."

612 + .to_string();

613 + if !types.is_empty() {

614 + description.push_str(

615 + "\n\nOptionally pass `subagent_type` to use one of these specialized \

616 + subagent types instead of the general-purpose default:\n",

617 + );

618 + // Same cap as `skills::MAX_ADVERTISED_SKILLS`: this list is baked into

619 + // every turn's tool spec, so bound its context cost even though a

620 + // pathological number of type files is unlikely. `/agents` (see

621 + // `subagents::format_agent_types_list`) shows the full list on request.

622 + for agent_type in types.iter().take(MAX_ADVERTISED_AGENT_TYPES) {

623 + description.push_str("- ");

624 + description.push_str(&agent_type.name);

625 + description.push_str(": ");

626 + description.push_str(&agent_type.description);

627 + description.push('\n');

628 + }

629 + }

630 +

631 ToolSpec {

632 name: TASK_TOOL_NAME.to_string(),

549 - description: "Delegate a self-contained research task to a subagent that \

550 - runs in a fresh conversation and returns only its final answer. The \

551 - subagent can read files, list directories, search, glob, and read \

552 - websites, but cannot modify anything. Use this for exploratory \

553 - questions whose intermediate file reads would otherwise clutter this \

554 - conversation (e.g. \"where is X implemented and how does it work?\"). \

555 - The subagent cannot see this conversation, so the prompt must be \

556 - fully self-contained: include absolute paths, symbol names, and \

557 - exactly what the answer should contain."

558 - .to_string(),

633 + description,

634 parameters_schema: json!({

635 "type": "object",

636 "properties": {

@@ -566,6 +641,10 @@ pub fn task_tool_spec() -> ToolSpec {

641 "prompt": {

642 "type": "string",

643 "description": "The full task for the subagent. Must be self-contained: the subagent sees nothing of this conversation."

644 + },

645 + "subagent_type": {

646 + "type": "string",

647 + "description": "Optional: the name of a configured subagent type (listed in this tool's description) to use instead of the general-purpose default."

648 }

649 },

650 "required": ["description", "prompt"],

@@ -596,21 +675,64 @@ async fn exec_task_with(arguments: &str, factory: Option<&SubagentFactory>) -> S

675 .get("description")

676 .and_then(Value::as_str)

677 .unwrap_or("(no description)");

678 + let subagent_type_name = args

679 + .get("subagent_type")

680 + .and_then(Value::as_str)

681 + .filter(|s| !s.trim().is_empty());

682 +

683 + let agent_type = match subagent_type_name {

684 + Some(name) => match subagents::resolve_agent_type(name) {

685 + Some(agent_type) => Some(agent_type),

686 + None => {

687 + let available = subagents::discover_agent_types()

688 + .into_iter()

689 + .map(|t| t.name)

690 + .collect::<Vec<_>>();

691 + return if available.is_empty() {

692 + format!(

693 + "Error: no subagent type named \"{name}\" (none are configured). \

694 + Omit subagent_type to use the default research subagent."

695 + )

696 + } else {

697 + format!(

698 + "Error: no subagent type named \"{name}\". Available: {}. Omit \

699 + subagent_type to use the default research subagent.",

700 + available.join(", ")

701 + )

702 + };

703 + }

704 + },

705 + None => None,

706 + };

707 +

708 + let system_prompt = agent_type

709 + .as_ref()

710 + .map(|t| t.system_prompt.as_str())

711 + .unwrap_or(SUBAGENT_SYSTEM_PROMPT);

712 + let allowed_tools = effective_subagent_tools(agent_type.as_ref());

713

600 - let Some(backend) = factory.and_then(|build| build()) else {

714 + let Some(backend) = factory.and_then(|build| build(system_prompt)) else {

715 return SUBAGENT_UNAVAILABLE.to_string();

716 };

717

604 - log::info!("task: running subagent — {description}");

605 - run_subagent(backend.as_ref(), prompt).await

718 + log::info!(

719 + "task: running subagent — {description} (type: {})",

720 + agent_type

721 + .as_ref()

722 + .map(|t| t.name.as_str())

723 + .unwrap_or("default")

724 + );

725 + run_subagent(backend.as_ref(), prompt, &allowed_tools).await

726 }

727

608 -/// The nested agent loop: a fresh conversation, read-only tools, a round cap,

609 -/// and only the final text returned. Mirrors the main loops in `main.rs` /

610 -/// `chat.rs`: offer tools each round, and pass `tools = None` on the last

611 -/// round to force a text answer.

612 -async fn run_subagent(backend: &dyn InferenceBackend, prompt: &str) -> String {

613 - let specs = subagent_tool_specs();

728 +/// The nested agent loop: a fresh conversation, a round cap, and only the

729 +/// final text returned. Mirrors the main loops in `main.rs` / `chat.rs`:

730 +/// offer tools each round, and pass `tools = None` on the last round to

731 +/// force a text answer. `allowed` is the toolset for this run — the default

732 +/// read-only set, or a configured type's narrowed subset of it (see

733 +/// [`effective_subagent_tools`]).

734 +async fn run_subagent(backend: &dyn InferenceBackend, prompt: &str, allowed: &[String]) -> String {

735 + let specs = subagent_tool_specs(allowed);

736

737 let mut result = match backend.send_message_with_tools(prompt, &specs, None).await {

738 Ok(r) => r,

@@ -628,17 +750,17 @@ async fn run_subagent(backend: &dyn InferenceBackend, prompt: &str) -> String {

750 let mut tool_results = Vec::with_capacity(result.tool_calls.len());

751 for call in &result.tool_calls {

752 // Hard gate, not just advertisement: even if the model asks for a

631 - // tool outside the offered set, only read-only tools execute here.

632 - let content = if SUBAGENT_TOOL_NAMES.contains(&call.name.as_str()) {

753 + // tool outside the offered set, only the allowed tools execute here.

754 + let content = if allowed.iter().any(|name| name == &call.name) {

755 // Boxed to break the async cycle: execute_tool → task →

756 // run_subagent → execute_tool.

757 Box::pin(execute_tool(&call.name, &call.arguments)).await

758 } else {

759 format!(

638 - "Error: `{}` is not available to a subagent. Only these \

639 - read-only tools are: {}.",

760 + "Error: `{}` is not available to this subagent. Only these \

761 + tools are: {}.",

762 call.name,

641 - SUBAGENT_TOOL_NAMES.join(", ")

763 + allowed.join(", ")

764 )

765 };

766 tool_results.push(ToolResult {

@@ -3088,7 +3210,7 @@ mod tests {

3210

3211 #[test]

3212 fn test_subagent_toolset_is_read_only() {

3091 - let specs = subagent_tool_specs();

3213 + let specs = subagent_tool_specs(&effective_subagent_tools(None));

3214 let mut names: Vec<&str> = specs.iter().map(|spec| spec.name.as_str()).collect();

3215 names.sort_unstable();

3216 assert_eq!(

@@ -3325,7 +3447,8 @@ mod tests {

3447 let file = dir.join("notes.txt");

3448 fs::write(&file, "subagent secret: 4217").unwrap();

3449

3328 - // Script: one read_file tool call, then a final text answer.

3450 + // Script: one read_file tool call, a final text answer, then a third

3451 + // response for the custom-agent-type phase further down.

3452 let (port, requests) = start_scripted_endpoint(vec![

3453 completion_tool_call(

3454 "call_1",

@@ -3333,17 +3456,22 @@ mod tests {

3456 &serde_json::json!({ "path": file }).to_string(),

3457 ),

3458 completion_text("The file contains the number 4217."),

3459 + completion_text("Custom type answer."),

3460 ]);

3461

3462 // Register the real process-global factory, pointing a fresh

3463 // OpenAiBackend at the scripted endpoint — exactly what the surfaces

3340 - // do at startup. This is the only test that touches the OnceLock.

3341 - set_subagent_factory(Box::new(move || {

3464 + // do at startup. This is the only test that touches the OnceLock

3465 + // (it's a `OnceLock`: later `set_subagent_factory` calls from other

3466 + // tests are silently ignored, so every test needing a real factory

3467 + // has to share this one registration — see the custom-agent-type

3468 + // phase later in this same test).

3469 + set_subagent_factory(Box::new(move |system_prompt: &str| {

3470 Some(Arc::new(crate::backend::OpenAiBackend::new(

3471 format!("http://127.0.0.1:{port}"),

3472 "test-key",

3473 "scripted-model",

3346 - Some(SUBAGENT_SYSTEM_PROMPT.to_string()),

3474 + Some(system_prompt.to_string()),

3475 )) as Arc<dyn InferenceBackend>)

3476 }));

3477 assert!(subagent_available());

@@ -3358,48 +3486,164 @@ mod tests {

3486 // Only the subagent's final text comes back.

3487 assert_eq!(result, "The file contains the number 4217.");

3488

3361 - let recorded = requests.lock().unwrap();

3362 - assert_eq!(recorded.len(), 2, "expected exactly two completions");

3489 + // Scoped so the `MutexGuard` is dropped before phase 2's `.await` —

3490 + // holding a `std::sync::MutexGuard` across an await point is a bug

3491 + // (it can deadlock or block the executor), and clippy's

3492 + // `await_holding_lock` lint doesn't reliably see through a manual

3493 + // `drop()` call, so use an explicit block instead.

3494 + {

3495 + let recorded = requests.lock().unwrap();

3496 + assert_eq!(recorded.len(), 2, "expected exactly two completions");

3497

3364 - // The subagent conversation is fresh (subagent system prompt) and was

3365 - // offered only the read-only toolset.

3366 - let first = &recorded[0];

3367 - assert_eq!(first["messages"][0]["role"], "system");

3368 - assert!(

3369 - first["messages"][0]["content"]

3370 - .as_str()

3498 + // The subagent conversation is fresh (subagent system prompt) and

3499 + // was offered only the read-only toolset.

3500 + let first = &recorded[0];

3501 + assert_eq!(first["messages"][0]["role"], "system");

3502 + assert!(

3503 + first["messages"][0]["content"]

3504 + .as_str()

3505 + .unwrap()

3506 + .contains("research subagent")

3507 + );

3508 + let offered: Vec<&str> = first["tools"]

3509 + .as_array()

3510 .unwrap()

3372 - .contains("research subagent")

3511 + .iter()

3512 + .map(|tool| tool["function"]["name"].as_str().unwrap())

3513 + .collect();

3514 + for name in &offered {

3515 + assert!(

3516 + SUBAGENT_TOOL_NAMES.contains(name),

3517 + "non-read-only tool offered: {name}"

3518 + );

3519 + }

3520 + assert!(!offered.contains(&TASK_TOOL_NAME));

3521 +

3522 + // The read-only tool actually executed: its output travelled back

3523 + // to the endpoint as a tool result on the second request.

3524 + let messages = recorded[1]["messages"].as_array().unwrap();

3525 + let tool_message = messages

3526 + .iter()

3527 + .find(|message| message["role"] == "tool")

3528 + .expect("no tool result in second request");

3529 + assert_eq!(tool_message["tool_call_id"], "call_1");

3530 + assert!(

3531 + tool_message["content"]

3532 + .as_str()

3533 + .unwrap()

3534 + .contains("subagent secret: 4217")

3535 + );

3536 + }

3537 +

3538 + // ── Phase 2: a configured subagent type, on the same registered

3539 + // factory and endpoint (the third scripted response above). Proves

3540 + // `subagent_type` actually swaps the system prompt and narrows the

3541 + // offered toolset end to end, not just at the pure-function level

3542 + // (see `effective_subagent_tools_*` below for that).

3543 + let agents_root = dir.join(".sigit").join("agents");

3544 + fs::create_dir_all(&agents_root).unwrap();

3545 + fs::write(

3546 + agents_root.join("notes-reader.md"),

3547 + "---\nname: notes-reader\ndescription: Reads notes files only\ntools: read_file\n---\n\nYou only ever read notes files and report exactly what they say.\n",

3548 + )

3549 + .unwrap();

3550 +

3551 + let prev_cwd = std::env::current_dir().unwrap();

3552 + std::env::set_current_dir(&dir).unwrap();

3553 +

3554 + let args = serde_json::json!({

3555 + "description": "read a note",

3556 + "prompt": "What does the notes file say?",

3557 + "subagent_type": "notes-reader",

3558 + })

3559 + .to_string();

3560 + let result = execute_tool(TASK_TOOL_NAME, &args).await;

3561 + assert_eq!(result, "Custom type answer.");

3562 +

3563 + let recorded = requests.lock().unwrap();

3564 + assert_eq!(recorded.len(), 3, "expected a third recorded request");

3565 + let last = &recorded[2];

3566 + assert_eq!(last["messages"][0]["role"], "system");

3567 + assert_eq!(

3568 + last["messages"][0]["content"].as_str().unwrap(),

3569 + "You only ever read notes files and report exactly what they say."

3570 );

3374 - let offered: Vec<&str> = first["tools"]

3571 + let offered: Vec<&str> = last["tools"]

3572 .as_array()

3573 .unwrap()

3574 .iter()

3575 .map(|tool| tool["function"]["name"].as_str().unwrap())

3576 .collect();

3380 - for name in &offered {

3381 - assert!(

3382 - SUBAGENT_TOOL_NAMES.contains(name),

3383 - "non-read-only tool offered: {name}"

3384 - );

3385 - }

3386 - assert!(!offered.contains(&TASK_TOOL_NAME));

3387 -

3388 - // The read-only tool actually executed: its output travelled back to

3389 - // the endpoint as a tool result on the second request.

3390 - let messages = recorded[1]["messages"].as_array().unwrap();

3391 - let tool_message = messages

3392 - .iter()

3393 - .find(|message| message["role"] == "tool")

3394 - .expect("no tool result in second request");

3395 - assert_eq!(tool_message["tool_call_id"], "call_1");

3396 - assert!(

3397 - tool_message["content"]

3398 - .as_str()

3399 - .unwrap()

3400 - .contains("subagent secret: 4217")

3577 + assert_eq!(

3578 + offered,

3579 + vec!["read_file"],

3580 + "notes-reader's tools: read_file should narrow the offered toolset to just that"

3581 );

3582 + drop(recorded);

3583

3584 + std::env::set_current_dir(prev_cwd).unwrap();

3585 let _ = fs::remove_dir_all(&dir);

3586 }

3587 +

3588 + #[test]

3589 + fn effective_subagent_tools_defaults_to_full_read_only_set_without_a_type() {

3590 + let mut tools = effective_subagent_tools(None);

3591 + tools.sort();

3592 + let mut expected: Vec<String> = SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();

3593 + expected.sort();

3594 + assert_eq!(tools, expected);

3595 + }

3596 +

3597 + #[test]

3598 + fn effective_subagent_tools_inherits_full_set_when_type_omits_tools_field() {

3599 + let agent_type = subagents::AgentType {

3600 + name: "generalist".to_string(),

3601 + description: "does anything".to_string(),

3602 + tools: None,

3603 + system_prompt: "Be helpful.".to_string(),

3604 + path: PathBuf::from("/x/generalist.md"),

3605 + };

3606 + let mut tools = effective_subagent_tools(Some(&agent_type));

3607 + tools.sort();

3608 + let mut expected: Vec<String> = SUBAGENT_TOOL_NAMES.iter().map(|s| s.to_string()).collect();

3609 + expected.sort();

3610 + assert_eq!(tools, expected);

3611 + }

3612 +

3613 + #[test]

3614 + fn effective_subagent_tools_narrows_to_the_types_allow_list() {

3615 + let agent_type = subagents::AgentType {

3616 + name: "notes-reader".to_string(),

3617 + description: "reads notes".to_string(),

3618 + tools: Some(vec!["read_file".to_string(), "glob".to_string()]),

3619 + system_prompt: "Read notes.".to_string(),

3620 + path: PathBuf::from("/x/notes-reader.md"),

3621 + };

3622 + let tools = effective_subagent_tools(Some(&agent_type));

3623 + assert_eq!(tools, vec!["read_file".to_string(), "glob".to_string()]);

3624 + }

3625 +

3626 + #[test]

3627 + fn effective_subagent_tools_drops_anything_outside_the_read_only_ceiling() {

3628 + // The security-critical case: a type file cannot grant itself a

3629 + // mutating tool by listing it in `tools:` — it's silently dropped,

3630 + // never passed through to the offered/gated toolset.

3631 + let agent_type = subagents::AgentType {

3632 + name: "sneaky".to_string(),

3633 + description: "tries to grant itself write access".to_string(),

3634 + tools: Some(vec![

3635 + "read_file".to_string(),

3636 + "edit_file".to_string(),

3637 + "run_command".to_string(),

3638 + "task".to_string(),

3639 + ]),

3640 + system_prompt: "...".to_string(),

3641 + path: PathBuf::from("/x/sneaky.md"),

3642 + };

3643 + let tools = effective_subagent_tools(Some(&agent_type));

3644 + assert_eq!(tools, vec!["read_file".to_string()]);

3645 + assert!(!tools.contains(&"edit_file".to_string()));

3646 + assert!(!tools.contains(&"run_command".to_string()));

3647 + assert!(!tools.contains(&"task".to_string()));

3648 + }

3649 }