feat(subagent): explore vertical slice — Agent tool end-to-end (#331) - #340
Merged
Merged
Conversation
Wire AgentRegistry, spawn_builtin, and AgentTool so the main LLM can synchronously delegate read-only explore tasks without polluting parent context. Closes AI-Shell-Team#331.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds built-in sub-agent registry and tool filtering, session-aware tool execution, isolated built-in sub-agent spawning, a new ChangesSub-agent explore vertical slice
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ParentSession
participant AgentTool
participant AgentRegistry
participant SubSession
ParentSession->>AgentTool: execute_async_in_session(args, session)
AgentTool->>AgentRegistry: resolve("explore")
AgentRegistry-->>AgentTool: AgentDefinition
AgentTool->>SubSession: spawn_builtin(parent, registry, "explore", prompt)
SubSession->>SubSession: run tool loop with filtered read-only tools
SubSession-->>AgentTool: SpawnResult
AgentTool-->>ParentSession: ToolResult
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
Contributor
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
F16shen
marked this pull request as ready for review
July 7, 2026 10:30
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/aish-llm/src/session.rs (1)
1161-1177: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the by-name tool path session-aware too.
This updates the main retry path, but
execute_tool_by_namestill dispatches throughexecute_async(args), so invoking a session-aware tool such asAgentthrough that public API won’t receive the hostingLlmSession.Proposed fix
- Ok(tool.as_ref().execute_async(args).await) + Ok(tool.as_ref().execute_async_in_session(args, self).await)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aish-llm/src/session.rs` around lines 1161 - 1177, The by-name tool execution path is still not session-aware because execute_tool_by_name dispatches through execute_async(args) instead of the session-bound execution path. Update execute_tool_by_name to pass the current LlmSession into the tool invocation, matching the retry flow in session.rs where execute_async_in_session(self) is already used, so tools like Agent receive the hosting session consistently.
🧹 Nitpick comments (1)
crates/aish-tools/src/agent_tool/prompt.rs (1)
8-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStatic
subagent_typeenum will drift from the registry as more agents are added.
parameters()hardcodes"enum": ["explore"], whileAgentTool::new()buildsdescriptiondynamically fromregistry.list_for_tool_description(). When a second built-in agent is registered (per the stack's stated future direction), this schema needs a manual, easy-to-forget update, whereas the description text will already list it — causing the LLM to see a type in the description that the schema then rejects.Consider deriving the enum from
AgentRegistry(e.g., pass the list of registeredsubagent_types intoparameters()) so schema and description stay in sync automatically.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aish-tools/src/agent_tool/prompt.rs` around lines 8 - 28, The `parameters()` schema in `agent_tool::prompt` hardcodes the `subagent_type` enum to a single value, which will drift from the dynamically generated agent list in `AgentTool::new()`. Update `parameters()` to derive the enum from `AgentRegistry` (or accept the registered subagent types as input) so the JSON schema stays in sync with `registry.list_for_tool_description()` and newly registered built-in agents are accepted automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aish-llm/src/agents/spawn.rs`:
- Around line 217-238: The spawn_builtin test is bypassing the allowlist by
manually adding read_file to the sub-session, so it does not verify that the
tool was forwarded from the parent’s allowed specs. Update
test_spawn_builtin_mock_sequence in spawn.rs to rely only on
register_mock_from_specs and the parent/session tool registration, and remove
the direct sub.register_tool(Box::new(MockTool::new("read_file"))) call so the
test proves spawn_builtin forwards an allowed tool spec correctly.
In `@crates/aish-tools/src/agent_tool/agent_tool.rs`:
- Around line 73-79: The mapping in spawn_result_to_tool_result currently treats
LoopStatus::Incomplete the same as LoopStatus::Complete, which hides partial or
truncated sub-agent output. Update spawn_result_to_tool_result in agent_tool.rs
so LoopStatus::Incomplete is surfaced distinctly from success, using
ToolResult::error or another non-success result with a message that makes the
incomplete state clear, while keeping LoopStatus::Complete as the only success
path.
- Around line 121-157: AgentTool::execute_async_in_session still routes
cancelled sub-agent failures through the generic retry-once path, causing a
second full spawn attempt. Update the sub-agent execution flow around the
spawn_fn and spawn_builtin result handling so cancelled or otherwise
unrecoverable results are marked as non-retryable/short-circuited, and ensure
ToolResult::error or spawn_result_to_tool_result preserves that status so the
wrapper skips the retry.
---
Outside diff comments:
In `@crates/aish-llm/src/session.rs`:
- Around line 1161-1177: The by-name tool execution path is still not
session-aware because execute_tool_by_name dispatches through
execute_async(args) instead of the session-bound execution path. Update
execute_tool_by_name to pass the current LlmSession into the tool invocation,
matching the retry flow in session.rs where execute_async_in_session(self) is
already used, so tools like Agent receive the hosting session consistently.
---
Nitpick comments:
In `@crates/aish-tools/src/agent_tool/prompt.rs`:
- Around line 8-28: The `parameters()` schema in `agent_tool::prompt` hardcodes
the `subagent_type` enum to a single value, which will drift from the
dynamically generated agent list in `AgentTool::new()`. Update `parameters()` to
derive the enum from `AgentRegistry` (or accept the registered subagent types as
input) so the JSON schema stays in sync with
`registry.list_for_tool_description()` and newly registered built-in agents are
accepted automatically.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c2c9de6d-7bf9-4b71-885c-9962be76e422
📒 Files selected for processing (12)
crates/aish-llm/src/agents/mod.rscrates/aish-llm/src/agents/registry.rscrates/aish-llm/src/agents/spawn.rscrates/aish-llm/src/agents/tools.rscrates/aish-llm/src/lib.rscrates/aish-llm/src/session.rscrates/aish-llm/src/types.rscrates/aish-shell/src/app.rscrates/aish-tools/src/agent_tool/agent_tool.rscrates/aish-tools/src/agent_tool/prompt.rscrates/aish-tools/src/lib.rscrates/aish-tools/tests/agent_tool_test.rs
Assert allowlist forwarding in spawn_builtin tests, mark cancelled/fatal sub-agent results as short-circuit to skip retry, and route execute_tool_by_name through execute_async_in_session.
F16shen
force-pushed
the
feat/subagent-explore-slice
branch
from
July 7, 2026 11:27
18a86a7 to
5a01a8f
Compare
This was referenced Jul 8, 2026
6 tasks
17 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AgentRegistry(explore only), tool allowlist filtering, andspawn_builtinon top of [Feature]: Sub-agent Phase 1 — 抽取 tool loop 与 spawn 测试基建 #330 spawn infrastructureAgentToolinaish-toolsand register it in the main shell sessionAgent(subagent_type=explore, …); only the final conclusion returns to the parent loopTest plan
make ci-check(format, clippy, workspace tests)AgentRegistry+resolve_tools_for_agentunit testsspawn_builtinmock LLM sequence + parent session isolationAgentToolparameter validation + mock spawn paths/etcnginx configs via explore sub-agentCloses #331
Summary by CodeRabbit