feat: agent delegation design + slash command interception#7
feat: agent delegation design + slash command interception#7
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an initial design + scaffolding for agent delegation (including marker parsing and ACL concepts) and adds a Docker-based integration-test harness with mock “planner/coder/reviewer” agents.
Changes:
- Added a new (currently standalone) Rust delegation engine module and a delegation design document.
- Added Docker compose + Python mock agents + fixture config intended to exercise delegation flows.
- Added an ignored Rust integration test suite and a README describing how to run the Docker harness.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/zeroclawed/src/delegation.rs | Adds delegation engine skeleton (marker parsing, ACL validation, chaining state). |
| crates/zeroclawed/src/delegation.md | Adds delegation + slash-command interception design notes and config examples. |
| crates/zeroclawed/tests/delegation/README.md | Documents how to run the Docker-based delegation integration environment. |
| crates/zeroclawed/tests/delegation/docker-compose.yml | Defines mock agent services plus a zeroclawed service for integration testing. |
| crates/zeroclawed/tests/delegation/Dockerfile.zeroclawed | Builds/runs zeroclawed inside the Docker harness. |
| crates/zeroclawed/tests/delegation/fixtures/test-config.toml | Intended test PolyConfig fixture for the Docker environment. |
| crates/zeroclawed/tests/delegation/mock-agents/mock_agent.py | Flask mock agent behavior (planner/coder/reviewer/echo) with delegation markers. |
| crates/zeroclawed/tests/delegation/mock-agents/Dockerfile | Container image for the Python mock agents. |
| crates/zeroclawed/tests/delegation_integration.rs | Adds ignored integration tests for delegation scenarios (currently mostly TODO). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn default_delegates() -> DelegationTarget { | ||
| DelegationTarget::None | ||
| } | ||
|
|
||
| fn default_accepts_from() -> DelegationTarget { | ||
| DelegationTarget::Any | ||
| } |
There was a problem hiding this comment.
DelegationTarget only defines Keyword(String) and List(Vec<String>), but the defaults return DelegationTarget::None / DelegationTarget::Any (nonexistent variants). This won’t compile; set defaults to DelegationTarget::Keyword("none".into()) and DelegationTarget::Keyword("any".into()) (or introduce explicit enum variants).
| pub fn new(config: &'a PolyConfig, router: &'a Router) -> Self { | ||
| Self { | ||
| config, | ||
| router, | ||
| max_depth: config.delegation.max_depth, |
There was a problem hiding this comment.
DelegationEngine::new reads config.delegation.max_depth, but PolyConfig currently has no delegation field. Either extend PolyConfig with a [delegation] section (and plumb it through config parsing) or pass max_depth in explicitly so this module compiles and can be configured.
| pub fn new(config: &'a PolyConfig, router: &'a Router) -> Self { | |
| Self { | |
| config, | |
| router, | |
| max_depth: config.delegation.max_depth, | |
| pub fn new(config: &'a PolyConfig, router: &'a Router, max_depth: usize) -> Self { | |
| Self { | |
| config, | |
| router, | |
| max_depth, |
| // Check source can delegate TO target | ||
| let can_delegate = source_agent | ||
| .delegation | ||
| .as_ref() | ||
| .map(|d| d.delegates.allows(&delegation.target)) | ||
| .unwrap_or(false); // Default: no delegation | ||
|
|
There was a problem hiding this comment.
validate_delegation accesses agent.delegation, but AgentConfig in config.rs has no delegation field. This will not compile and also means the ACL config in fixtures/test-config.toml can’t be loaded as written; add the field to AgentConfig (e.g. delegation: Option<DelegationConfig>) or adjust the design to match the existing schema.
| // Try TOML-like blocks first | ||
| let block_regex = regex::Regex::new( | ||
| r"\[delegate\]\s*\n?((?:.|\n)*?)\[/delegate\]" | ||
| ).ok(); | ||
|
|
||
| if let Some(re) = block_regex { | ||
| for cap in re.captures_iter(text) { | ||
| let content = cap.get(1).map(|m| m.as_str()).unwrap_or(""); | ||
| if let Ok(req) = toml::from_str::<DelegationRequest>(content) { | ||
| requests.push(req); | ||
| } else { | ||
| warn!(content = %content, "failed to parse delegation block as TOML"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Try inline JSON format | ||
| let inline_regex = regex::Regex::new( | ||
| r"::delegate::(.*?)::" | ||
| ).ok(); | ||
|
|
||
| if let Some(re) = inline_regex { | ||
| for cap in re.captures_iter(text) { | ||
| let content = cap.get(1).map(|m| m.as_str()).unwrap_or(""); | ||
| if let Ok(req) = serde_json::from_str::<DelegationRequest>(content) { | ||
| requests.push(req); | ||
| } else { | ||
| warn!(content = %content, "failed to parse inline delegation as JSON"); | ||
| } |
There was a problem hiding this comment.
parse_delegation_markers uses regex::Regex, but crates/zeroclawed/Cargo.toml doesn’t include regex as a dependency. Add it (or implement parsing without regex) or this module won’t compile when wired in.
| // Try TOML-like blocks first | |
| let block_regex = regex::Regex::new( | |
| r"\[delegate\]\s*\n?((?:.|\n)*?)\[/delegate\]" | |
| ).ok(); | |
| if let Some(re) = block_regex { | |
| for cap in re.captures_iter(text) { | |
| let content = cap.get(1).map(|m| m.as_str()).unwrap_or(""); | |
| if let Ok(req) = toml::from_str::<DelegationRequest>(content) { | |
| requests.push(req); | |
| } else { | |
| warn!(content = %content, "failed to parse delegation block as TOML"); | |
| } | |
| } | |
| } | |
| // Try inline JSON format | |
| let inline_regex = regex::Regex::new( | |
| r"::delegate::(.*?)::" | |
| ).ok(); | |
| if let Some(re) = inline_regex { | |
| for cap in re.captures_iter(text) { | |
| let content = cap.get(1).map(|m| m.as_str()).unwrap_or(""); | |
| if let Ok(req) = serde_json::from_str::<DelegationRequest>(content) { | |
| requests.push(req); | |
| } else { | |
| warn!(content = %content, "failed to parse inline delegation as JSON"); | |
| } | |
| // Try TOML-like blocks first. | |
| let block_start = "[delegate]"; | |
| let block_end = "[/delegate]"; | |
| let mut search_from = 0; | |
| while let Some(start_rel) = text[search_from..].find(block_start) { | |
| let start = search_from + start_rel; | |
| let content_start = start + block_start.len(); | |
| if let Some(end_rel) = text[content_start..].find(block_end) { | |
| let end = content_start + end_rel; | |
| let content = text[content_start..end].trim_start_matches([' ', '\t', '\r', '\n']); | |
| if let Ok(req) = toml::from_str::<DelegationRequest>(content) { | |
| requests.push(req); | |
| } else { | |
| warn!(content = %content, "failed to parse delegation block as TOML"); | |
| } | |
| search_from = end + block_end.len(); | |
| } else { | |
| break; | |
| } | |
| } | |
| // Try inline JSON format. | |
| let inline_start = "::delegate::"; | |
| let inline_end = "::"; | |
| let mut search_from = 0; | |
| while let Some(start_rel) = text[search_from..].find(inline_start) { | |
| let start = search_from + start_rel; | |
| let content_start = start + inline_start.len(); | |
| if let Some(end_rel) = text[content_start..].find(inline_end) { | |
| let end = content_start + end_rel; | |
| let content = &text[content_start..end]; | |
| if let Ok(req) = serde_json::from_str::<DelegationRequest>(content) { | |
| requests.push(req); | |
| } else { | |
| warn!(content = %content, "failed to parse inline delegation as JSON"); | |
| } | |
| search_from = end + inline_end.len(); | |
| } else { | |
| break; |
| /// Record a delegation edge. Returns Err if cycle detected. | ||
| pub fn record_edge(&mut self, source: &str, target: &str) -> Result<()> { | ||
| let edge = (source.to_string(), target.to_string()); | ||
|
|
||
| if self.edges.contains(&edge) { | ||
| return Err(anyhow::anyhow!( | ||
| "delegation cycle detected: {} -> {}", | ||
| source, target | ||
| )); | ||
| } | ||
|
|
||
| self.edges.push(edge); | ||
| self.depth += 1; | ||
| Ok(()) |
There was a problem hiding this comment.
DelegationState::record_edge only rejects repeated (source,target) edges, which does not detect graph cycles (e.g. A→B, B→C, C→A). The included test_cycle_detection expects C→A to fail, but it will currently succeed; implement actual cycle detection (e.g., track path stack / visited nodes per chain) and align the test.
| delgates = "any" # Options: "any", "none", ["agent1", "agent2"] | ||
| accepts_from = "any" # Who can delegate TO this agent | ||
|
|
||
| [[agents]] | ||
| id = "coder" | ||
| kind = "acpx" | ||
| agent_name = "codex" | ||
| delgates = ["planner", "reviewer"] # Limited delegation |
There was a problem hiding this comment.
The config example has typos in field names (delgates instead of delegates), which makes the schema example misleading and inconsistent with the intended ACL design. Fix the field spelling to match the actual config key you plan to parse.
| delgates = "any" # Options: "any", "none", ["agent1", "agent2"] | |
| accepts_from = "any" # Who can delegate TO this agent | |
| [[agents]] | |
| id = "coder" | |
| kind = "acpx" | |
| agent_name = "codex" | |
| delgates = ["planner", "reviewer"] # Limited delegation | |
| delegates = "any" # Options: "any", "none", ["agent1", "agent2"] | |
| accepts_from = "any" # Who can delegate TO this agent | |
| [[agents]] | |
| id = "coder" | |
| kind = "acpx" | |
| agent_name = "codex" | |
| delegates = ["planner", "reviewer"] # Limited delegation |
| //! Agent Delegation Engine | ||
| //! | ||
| //! Allows agents to dynamically delegate to other configured agents. | ||
| //! Parses delegation markers in agent responses and orchestrates chained calls. | ||
|
|
There was a problem hiding this comment.
This file won’t be compiled or its unit tests run unless it’s referenced from the crate root (e.g. mod delegation; in main.rs or a lib.rs). Right now there are no references, so the delegation engine code is effectively dead/unvalidated.
| ### 2. Wait for services to be healthy | ||
|
|
||
| ```bash | ||
| docker-compose ps | ||
| # All services should show "healthy" or "Up" |
There was a problem hiding this comment.
Compose services don’t define healthchecks, so docker-compose ps will typically show only Up (not healthy). Either add healthchecks for the mock agents/zeroclawed services or adjust this README step to match what users will actually see.
| ### 2. Wait for services to be healthy | |
| ```bash | |
| docker-compose ps | |
| # All services should show "healthy" or "Up" | |
| ### 2. Verify the services are running | |
| ```bash | |
| docker-compose ps | |
| # All services should show "Up" |
| # Telegram channel (for testing - disabled in docker) | ||
| [[channels]] | ||
| kind = "telegram" | ||
| enabled = false |
There was a problem hiding this comment.
With this config, all channels are disabled and zeroclawed will exit early with "no enabled channels found in config" (see main.rs). The docker-compose zeroclawed service likely won’t stay up; enable at least one channel (or add a dedicated test mode/HTTP ingress for integration testing).
| # Telegram channel (for testing - disabled in docker) | |
| [[channels]] | |
| kind = "telegram" | |
| enabled = false | |
| # Telegram channel (enabled so the test fixture has at least one active channel) | |
| [[channels]] | |
| kind = "telegram" | |
| enabled = true |
| # Or manually test via curl | ||
| curl -X POST http://localhost:18797/v1/chat/completions \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "model": "planner", | ||
| "messages": [{"role": "user", "content": "Write a Python function to reverse a string"}] | ||
| }' | ||
| ``` |
There was a problem hiding this comment.
The curl example assumes zeroclawed exposes an OpenAI-compatible /v1/chat/completions endpoint on :18797, but the current binary doesn’t serve that API (and exits if no channels are enabled). Either adjust the docs to describe the actual ingress path used in these tests or add an HTTP ingress endpoint specifically for this integration harness.
|
Closing for now. The delegation engine and slash-command interception design here will be incorporated into a fresh PR that targets post-PR-#10 main (which restructured the context store and proxy). The branch is preserved — design doc at crates/zeroclawed/src/delegation.md and scaffolding in delegation.rs remain valuable as a starting reference. Note: the current delegation.rs uses context_store.get_recent() which no longer exists on main; the new work will adapt to the current ContextStore API. |
Description
Brief description of the changes in this PR.
Type of Change
Testing
cargo testpasses for all affected cratescargo clippypasses with no warningscargo fmtis cleanChecklist
Related Issues
Fixes # (issue number)
Breaking Changes
List any breaking changes and migration steps:
Additional Notes
Any additional context or screenshots.