feat: managed agents — manager-executor pattern#34
Conversation
Manager-executor architecture for cost-optimized multi-agent workflows. Config: [managed_agents] enabled = true executor_model = 'claude-haiku-4-5' executor_max_turns = 8 max_concurrent = 3 Enhanced subagent tool: - model: per-agent model override - tools: restrict available tools per agent - max_turns: per-agent turn limit - poll_agent_id: poll background agent results System prompt injection when managed mode active: Manager plans + delegates, executors implement. No architecture changes — extends existing SubagentSpawner in mc-core, adds ManagedAgentConfig to mc-config.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a ManagedAgentConfig to config types and CLI prompt handling, and extends subagent tooling and runtime to support model routing, tool allowlists, background execution with polling, max-turn limits, and configurable concurrency with tracked background results. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Runtime as ConversationRuntime
participant Subagent as SubagentSpawner
participant BgResults as BackgroundResults
rect rgba(0,128,255,0.5)
Client->>Runtime: dispatch_tool("subagent", {task, tools?, model?, max_turns?, poll_agent_id?})
end
alt poll_agent_id provided
Runtime->>Subagent: poll_background(agent_id)
Subagent->>BgResults: lookup(agent_id)
alt result present
BgResults-->>Subagent: Some(result)
Subagent-->>Runtime: Some(Some(result))
Runtime-->>Client: result
else still running
BgResults-->>Subagent: None
Subagent-->>Runtime: None
Runtime-->>Client: {agent_id, status:"running"}
end
else spawn / foreground execution
Runtime->>Subagent: run_task(task, model_override, allowed_tools, max_turns)
Subagent->>Subagent: apply tool filter, set max_iters from max_turns
alt background spawn (fire-and-forget)
Subagent->>BgResults: store agent_id -> None (pending)
Subagent-->>Runtime: {agent_id, status:"running"}
Runtime-->>Client: {agent_id, status:"running"}
Subagent->>BgResults: update agent_id -> Some(result) when done
else foreground completes
Subagent-->>Runtime: task result
Runtime-->>Client: result
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mc/crates/mc-core/src/subagent.rs (2)
179-188:⚠️ Potential issue | 🟠 MajorSubagent execution still can't be cancelled.
run_simple_agentnow exposes per-agent turn limits, but it still has noCancellationTokenand waits on provider events unconditionally. Cancelling the parent turn will not stop a long-running executor, which is especially costly in managed mode.As per coding guidelines "Handle cancellation via
CancellationTokenin async loops".Also applies to: 208-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/subagent.rs` around lines 179 - 188, run_simple_agent currently has no cancellation support and blocks waiting on provider events; add a CancellationToken parameter (e.g., cancellation: &CancellationToken) to run_simple_agent and use it inside all async loops and wait points that poll provider events or long-running executors so the function returns early when cancelled. Specifically, check cancellation before/inside the per-turn loop, select! between provider event futures and cancellation.token.cancelled() when awaiting long operations, and propagate the token into any nested async calls that can be cancelled; apply the same pattern to the other async loops in this module that wait on provider events so parent turn cancellation stops long-running work.
60-84:⚠️ Potential issue | 🔴 CriticalBackground polling never reaches a terminal state.
background_resultsis initialized and read here, but nothing in this file ever inserts an agent ID or stores a completion result. That meanspoll_background()can only returnNone, andConversationRuntime::dispatch_toolwill report every poll as"status":"running"forever.Also applies to: 164-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/subagent.rs` around lines 60 - 84, The background_results map is never written to, so poll_background always returns None; fix by writing to background_results when a subagent is spawned and when it completes: in the code path that starts background agents (the SubagentSpawner spawn/start method or the place that calls it, and ConversationRuntime::dispatch_tool), insert an entry background_results.lock().unwrap_or_else(|e| e.into_inner()).insert(agent_id.to_string(), None) when the job is scheduled, and update that same entry to Some(result_string) (or remove/insert a terminal value) inside the completion handler/future/async task when the agent finishes (ensure you clone/hold Arc<Mutex<...>> into the task). Also update any error paths to set a terminal value (e.g., Some(error_message)) so poll_background can observe the terminal state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/crates/mc-cli/src/main.rs`:
- Around line 1672-1679: build_managed_agent_prompt currently ignores
ManagedAgentConfig.manager_model so the configured manager model has no effect;
update the function to read config.manager_model (similar to how executor_model
is read) and apply it where the manager/runtime selection and prompt text are
built (e.g., introduce a manager_model variable using
config.manager_model.as_deref().unwrap_or(<fallback>) and use that variable in
the prompt generation and any runtime/model-selection logic inside
build_managed_agent_prompt or its callers so manager_model controls the
manager's model choice).
- Around line 1678-1696: The prompt incorrectly promises "up to {max_concurrent}
executors" without reflecting the runtime hard cap in mc-core; compute an
effective max and show that instead: derive effective_max =
min(config.max_concurrent.unwrap_or(3), MAX_CONCURRENT_SUBAGENTS) (referencing
config.max_concurrent and the local max_concurrent variable) or, if you can't
import the constant, explicitly mention the runtime cap value from
MAX_CONCURRENT_SUBAGENTS in the prompt text; then update the format string used
to render the manager prompt so it uses effective_max (and/or appends "(runtime
cap: MAX_CONCURRENT_SUBAGENTS)") and mention
SubagentSpawner/MAX_CONCURRENT_SUBAGENTS in a comment so future work threads
managed_agents.max_concurrent into SubagentSpawner.
In `@mc/crates/mc-config/src/types.rs`:
- Around line 131-142: Add numeric bounds validation for ManagedAgentConfig
inside RuntimeConfig::validate(): check ManagedAgentConfig fields
executor_max_turns and max_concurrent and emit a warning if they are Some(0) (or
otherwise non-positive), and check budget_usd and emit a warning if it is
Some(value) && value <= 0.0; reference the ManagedAgentConfig struct and its
fields (executor_max_turns, max_concurrent, budget_usd) and add these checks in
RuntimeConfig::validate(), using the existing logger (e.g., log::warn or the
crate’s logger) to report which field and value is invalid so maintainers can
see the problematic config.
- Around line 131-142: The managed_agents value is currently taken only from the
final config layer which drops earlier per-field settings; change the resolution
to merge managed agent fields across all layers (same pattern used for other
layered configs): start from a default ManagedAgentConfig and for each config
layer that has managed_agents, set each field only when that field is present
(for optional fields) or override enabled when present; if you need to represent
"unset" use Option<bool> for enabled during merging, and when constructing the
final RuntimeConfig resolve enabled with
managed_agents.enabled.unwrap_or(false). Update the merging logic that currently
assigns managed_agents from a single layer (the variable named managed_agents)
to perform per-field merges instead.
In `@mc/crates/mc-core/src/subagent.rs`:
- Around line 195-205: The current allowed_tools value only filters what tool
definitions are sent to the model but does not prevent the model from requesting
arbitrary tools; update the tool invocation/dispatch logic to enforce the
allowlist at runtime: in subagent.rs wherever tool calls are executed (the
dispatcher that receives a tool name from the model), check the
Option<Vec<String>> allowed_tools and refuse/abort the call if
allowed_tools.is_some() and the requested tool name is not contained (return an
error or a controlled rejection), and only proceed to look up the tool in
tool_registry and execute it when the name passes this explicit runtime
allowlist check; keep the existing ToolDefinition filtering but add this
enforcement during actual execution.
In `@mc/crates/mc-tools/src/spec.rs`:
- Around line 94-104: The input_schema in mc-tools needs to match
dispatch_tool's runtime contract: change the schema so it supports an alternate
"poll" mode where "poll_agent_id" is required and "task" is not, and make
"max_turns" an integer (not a generic number) so runtime as_u64() semantics are
respected; update the schema generation around the input_schema object to
express a oneOf/anyOf with two variants (one requiring "task" and one requiring
"poll_agent_id") and change the "max_turns" property type to "integer"
(optionally with a minimum) to prevent non-integer values from being accepted.
Ensure you reference the input_schema definition in spec.rs and align it with
dispatch_tool's handling of poll_agent_id and max_turns in
mc-core/src/runtime.rs.
---
Outside diff comments:
In `@mc/crates/mc-core/src/subagent.rs`:
- Around line 179-188: run_simple_agent currently has no cancellation support
and blocks waiting on provider events; add a CancellationToken parameter (e.g.,
cancellation: &CancellationToken) to run_simple_agent and use it inside all
async loops and wait points that poll provider events or long-running executors
so the function returns early when cancelled. Specifically, check cancellation
before/inside the per-turn loop, select! between provider event futures and
cancellation.token.cancelled() when awaiting long operations, and propagate the
token into any nested async calls that can be cancelled; apply the same pattern
to the other async loops in this module that wait on provider events so parent
turn cancellation stops long-running work.
- Around line 60-84: The background_results map is never written to, so
poll_background always returns None; fix by writing to background_results when a
subagent is spawned and when it completes: in the code path that starts
background agents (the SubagentSpawner spawn/start method or the place that
calls it, and ConversationRuntime::dispatch_tool), insert an entry
background_results.lock().unwrap_or_else(|e|
e.into_inner()).insert(agent_id.to_string(), None) when the job is scheduled,
and update that same entry to Some(result_string) (or remove/insert a terminal
value) inside the completion handler/future/async task when the agent finishes
(ensure you clone/hold Arc<Mutex<...>> into the task). Also update any error
paths to set a terminal value (e.g., Some(error_message)) so poll_background can
observe the terminal state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ddf6ffa-83de-4520-b231-b43c43288304
📒 Files selected for processing (6)
mc/crates/mc-cli/src/main.rsmc/crates/mc-config/src/lib.rsmc/crates/mc-config/src/types.rsmc/crates/mc-core/src/runtime.rsmc/crates/mc-core/src/subagent.rsmc/crates/mc-tools/src/spec.rs
| let max_concurrent = config.max_concurrent.unwrap_or(3); | ||
| let budget_note = config | ||
| .budget_usd | ||
| .map_or("No hard budget cap. Be cost-conscious.".to_string(), |b| { | ||
| format!("Total budget: ${b:.2}. Monitor spend carefully.") | ||
| }); | ||
| format!( | ||
| r#" | ||
|
|
||
| ## Managed Agent Mode | ||
|
|
||
| You are the MANAGER in a manager-executor architecture. | ||
|
|
||
| ### Your Role | ||
| - You are the **planning and reasoning layer**. Coordinate work but do NOT execute tasks directly. | ||
| - Delegate all implementation to executor agents using the **subagent** tool. | ||
| - Each executor uses model `{executor_model}` and has up to {max_turns} turns. | ||
| - You may run up to {max_concurrent} executors by making multiple subagent calls. | ||
|
|
There was a problem hiding this comment.
Prompt overstates executor concurrency vs runtime enforcement.
The prompt says “up to {max_concurrent} executors” (Line 1695), but runtime currently enforces a hard cap of 4 in mc/crates/mc-core/src/subagent.rs (MAX_CONCURRENT_SUBAGENTS). This can mislead the manager policy and produce avoidable task queuing.
🔧 Safe short-term fix in CLI prompt
- let max_concurrent = config.max_concurrent.unwrap_or(3);
+ let max_concurrent = config.max_concurrent.unwrap_or(3).min(4);Long-term fix: thread managed_agents.max_concurrent into SubagentSpawner and enforce it there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-cli/src/main.rs` around lines 1678 - 1696, The prompt
incorrectly promises "up to {max_concurrent} executors" without reflecting the
runtime hard cap in mc-core; compute an effective max and show that instead:
derive effective_max = min(config.max_concurrent.unwrap_or(3),
MAX_CONCURRENT_SUBAGENTS) (referencing config.max_concurrent and the local
max_concurrent variable) or, if you can't import the constant, explicitly
mention the runtime cap value from MAX_CONCURRENT_SUBAGENTS in the prompt text;
then update the format string used to render the manager prompt so it uses
effective_max (and/or appends "(runtime cap: MAX_CONCURRENT_SUBAGENTS)") and
mention SubagentSpawner/MAX_CONCURRENT_SUBAGENTS in a comment so future work
threads managed_agents.max_concurrent into SubagentSpawner.
| /// Managed agent configuration. | ||
| #[derive(Debug, Clone, Default, Deserialize)] | ||
| #[serde(default)] | ||
| pub struct ManagedAgentConfig { | ||
| pub enabled: bool, | ||
| pub manager_model: Option<String>, | ||
| pub executor_model: Option<String>, | ||
| pub executor_max_turns: Option<usize>, | ||
| pub max_concurrent: Option<usize>, | ||
| pub isolation: Option<String>, | ||
| pub budget_usd: Option<f64>, | ||
| } |
There was a problem hiding this comment.
Add validation for managed-agent numeric bounds.
executor_max_turns, max_concurrent, and budget_usd are accepted without sanity checks. Invalid values (e.g., 0 or negative budget) should emit warnings in RuntimeConfig::validate().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-config/src/types.rs` around lines 131 - 142, Add numeric bounds
validation for ManagedAgentConfig inside RuntimeConfig::validate(): check
ManagedAgentConfig fields executor_max_turns and max_concurrent and emit a
warning if they are Some(0) (or otherwise non-positive), and check budget_usd
and emit a warning if it is Some(value) && value <= 0.0; reference the
ManagedAgentConfig struct and its fields (executor_max_turns, max_concurrent,
budget_usd) and add these checks in RuntimeConfig::validate(), using the
existing logger (e.g., log::warn or the crate’s logger) to report which field
and value is invalid so maintainers can see the problematic config.
Managed-agent layering is currently lossy across config layers.
managed_agents is resolved from only the last layer (Line 314-317), while enabled is a non-optional bool defaulting to false (Line 135). If an earlier layer enables managed mode and a later layer omits [managed_agents], the later default silently disables it.
💡 Suggested direction
- pub struct ManagedAgentConfig {
- pub enabled: bool,
+ pub struct ManagedAgentConfig {
+ pub enabled: Option<bool>,
pub manager_model: Option<String>,
pub executor_model: Option<String>,
pub executor_max_turns: Option<usize>,
pub max_concurrent: Option<usize>,
pub isolation: Option<String>,
pub budget_usd: Option<f64>,
}Then merge per-field across all layers (same pattern used elsewhere) and resolve enabled with unwrap_or(false) when building RuntimeConfig.
Also applies to: 314-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-config/src/types.rs` around lines 131 - 142, The managed_agents
value is currently taken only from the final config layer which drops earlier
per-field settings; change the resolution to merge managed agent fields across
all layers (same pattern used for other layered configs): start from a default
ManagedAgentConfig and for each config layer that has managed_agents, set each
field only when that field is present (for optional fields) or override enabled
when present; if you need to represent "unset" use Option<bool> for enabled
during merging, and when constructing the final RuntimeConfig resolve enabled
with managed_agents.enabled.unwrap_or(false). Update the merging logic that
currently assigns managed_agents from a single layer (the variable named
managed_agents) to perform per-field merges instead.
mc/crates/mc-tools/src/spec.rs
Outdated
| input_schema: json!({ | ||
| "type": "object", | ||
| "properties": { | ||
| "task": { "type": "string", "description": "The task description for the subagent" }, | ||
| "context": { "type": "string", "description": "Optional context to provide (file contents, specs, etc)" } | ||
| "context": { "type": "string", "description": "Optional context to provide (file contents, specs, etc)" }, | ||
| "model": { "type": "string", "description": "Optional model override (e.g. 'claude-haiku-4-5' for cheaper tasks)" }, | ||
| "tools": { "type": "array", "items": { "type": "string" }, "description": "Optional: restrict which tools the agent can use" }, | ||
| "max_turns": { "type": "number", "description": "Optional: max turns for this agent (default 8)" }, | ||
| "poll_agent_id": { "type": "string", "description": "Poll a background agent's result by ID" } | ||
| }, | ||
| "required": ["task"] |
There was a problem hiding this comment.
Schema no longer matches the runtime contract.
dispatch_tool in mc/crates/mc-core/src/runtime.rs accepts poll_agent_id as an alternate mode and parses max_turns with as_u64(), but this schema still requires task and advertises max_turns as any JSON number. That makes a valid poll request impossible to express here, and values like 1.5 look valid in the schema but are silently ignored at runtime.
🛠️ Suggested schema fix
input_schema: json!({
"type": "object",
"properties": {
"task": { "type": "string", "description": "The task description for the subagent" },
"context": { "type": "string", "description": "Optional context to provide (file contents, specs, etc)" },
"model": { "type": "string", "description": "Optional model override (e.g. 'claude-haiku-4-5' for cheaper tasks)" },
"tools": { "type": "array", "items": { "type": "string" }, "description": "Optional: restrict which tools the agent can use" },
- "max_turns": { "type": "number", "description": "Optional: max turns for this agent (default 8)" },
+ "max_turns": { "type": "integer", "minimum": 1, "description": "Optional: max turns for this agent (default 8)" },
"poll_agent_id": { "type": "string", "description": "Poll a background agent's result by ID" }
},
- "required": ["task"]
+ "anyOf": [
+ { "required": ["task"] },
+ { "required": ["poll_agent_id"] }
+ ]
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-tools/src/spec.rs` around lines 94 - 104, The input_schema in
mc-tools needs to match dispatch_tool's runtime contract: change the schema so
it supports an alternate "poll" mode where "poll_agent_id" is required and
"task" is not, and make "max_turns" an integer (not a generic number) so runtime
as_u64() semantics are respected; update the schema generation around the
input_schema object to express a oneOf/anyOf with two variants (one requiring
"task" and one requiring "poll_agent_id") and change the "max_turns" property
type to "integer" (optionally with a minimum) to prevent non-integer values from
being accepted. Ensure you reference the input_schema definition in spec.rs and
align it with dispatch_tool's handling of poll_agent_id and max_turns in
mc-core/src/runtime.rs.
- CRITICAL: Tool filter enforced at execution (not just schema) - MAJOR: manager_model applied when managed mode enabled - MAJOR: max_concurrent configurable in SubagentSpawner - MAJOR: Schema fixed — task not required for poll, integer type - MINOR: Validation for executor_max_turns and budget_usd
Manager-Executor Architecture
When enabled, system prompt tells LLM to act as MANAGER:
Enhanced subagent tool
{"task": "review auth.rs", "model": "haiku", "tools": ["read_file", "grep_search"], "max_turns": 5}No architecture changes
183 tests pass.
Summary by CodeRabbit