feat(agents): assign #1208 coding-harness tools to relevant built-in agents#1214
Conversation
tinyhumansai#1208 added 9 coding-harness primitives (grep, glob, list, edit, apply_patch, todowrite, plan_exit, web_fetch, lsp) to the global all_tools registry but did not wire them into any agent's [tools] named allowlist — only wildcard agents (tools_agent) could see them. Distribution: - code_executor: full set (sandboxed coder writes/edits files and runs them) - planner: read-only nav (grep/glob/list) + todowrite + plan_exit + web_fetch (sandbox_mode = read_only forbids edit/apply_patch/lsp anyway) - researcher: read-only nav (grep/glob/list) + web_fetch (the simple URL-GET sibling of http_request, same allowed_domains gate) - orchestrator: todowrite + plan_exit (coordination markers; the orchestrator delegates editing to downstream agents) Other agents (archivist, critic, summarizer, help, welcome, integrations_agent, tool_maker, trigger_*) intentionally untouched — their narrow named allowlists reflect domain-specific surfaces that the new coding tools don't fit. Verified by `cargo test --lib agent::agents::loader` (20/20 pass).
📝 WalkthroughWalkthroughFour agent TOML configuration files are updated to expand their permitted tool allowlists. The code executor gains navigation and editing tools plus task flow helpers; the orchestrator adds todo and plan exit handlers; the planner gains read-only navigation and plan output tools; the researcher adds web fetch and read-only search capabilities. ChangesAgent Tool Allowlist Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/agent/agents/code_executor/agent.toml (1)
26-43: ⚡ Quick winAdd a regression assertion for this exact tool surface.
Given Line 26 through Line 43 materially expands executor capabilities, it’s worth adding/expanding a loader test that asserts expected tools for
code_executor(and complementary exclusions forplanner) so future edits don’t silently drift.🤖 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 `@src/openhuman/agent/agents/code_executor/agent.toml` around lines 26 - 43, Add a regression unit test that asserts the exact tool surface loaded for the code_executor agent (the "named" list that currently contains "shell","file_read","file_write","git_operations","node_exec","npm_exec","curl","grep","glob","list","edit","apply_patch","todowrite","plan_exit","web_fetch","lsp") and also asserts complementary exclusions for the planner agent so changes are caught. Implement this in the existing loader/agent tests (e.g., add a test named test_code_executor_tool_surface_regression or augment test_load_agents) by loading the agent configuration, reading the tool names for the agent identified as code_executor, and comparing against the exact expected list (order-sensitive or order-insensitive per your test conventions); likewise assert planner’s tool list does not include these executor-only tools. Ensure the test fails on any divergence so future edits must update the asserted list.
🤖 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.
Nitpick comments:
In `@src/openhuman/agent/agents/code_executor/agent.toml`:
- Around line 26-43: Add a regression unit test that asserts the exact tool
surface loaded for the code_executor agent (the "named" list that currently
contains
"shell","file_read","file_write","git_operations","node_exec","npm_exec","curl","grep","glob","list","edit","apply_patch","todowrite","plan_exit","web_fetch","lsp")
and also asserts complementary exclusions for the planner agent so changes are
caught. Implement this in the existing loader/agent tests (e.g., add a test
named test_code_executor_tool_surface_regression or augment test_load_agents) by
loading the agent configuration, reading the tool names for the agent identified
as code_executor, and comparing against the exact expected list (order-sensitive
or order-insensitive per your test conventions); likewise assert planner’s tool
list does not include these executor-only tools. Ensure the test fails on any
divergence so future edits must update the asserted list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4fe2044c-a275-4fe5-8081-ffaee1ae6b0d
📒 Files selected for processing (4)
src/openhuman/agent/agents/code_executor/agent.tomlsrc/openhuman/agent/agents/orchestrator/agent.tomlsrc/openhuman/agent/agents/planner/agent.tomlsrc/openhuman/agent/agents/researcher/agent.toml
Summary
grep,glob,list,edit,apply_patch,todowrite,plan_exit,web_fetch,lsp) to the globalall_toolsregistry, but did not wire them into any agent's[tools] namedallowlist.wildcard = {}agents (e.g.tools_agent) could actually see them. The agents that would benefit most —code_executor,planner,researcher,orchestrator— were silently still on the legacy surface.sandbox_mode.Distribution
code_executorsandboxedgrep,glob,list,edit,apply_patch,todowrite,plan_exit,web_fetch,lspplannerread_onlygrep,glob,list) +todowrite+plan_exit+web_fetchresearchernonegrep,glob,list) +web_fetch(the simple URL-GET sibling ofhttp_request, sameallowed_domainsgate)orchestratortodowrite+plan_exitonly — the orchestrator coordinates, it doesn't edit filesplannerdeliberately does NOT receiveedit/apply_patch/lsp—sandbox_mode = "read_only"forbids workspace mutations and downstream agents do the writing.Other agents (
archivist,critic,summarizer,help,welcome,integrations_agent,tool_maker,trigger_triage,trigger_reactor,morning_briefing) are intentionally untouched — their narrow named allowlists reflect domain-specific surfaces (memory ops, composio meta-tools, gitbooks, onboarding) that the new coding primitives don't fit.lspis capability-gated byOPENHUMAN_LSP_ENABLED; listing it incode_executor's allowlist is harmless when the gate is off (the tool is simply not registered into the runtime).Test plan
cargo test --lib agent::agents::loader— 20/20 pass (coverscode_executor_*,planner_is_read_only_with_composio_meta_tools,researcher_has_curl_for_artifact_downloads,orchestrator_has_reasoning_hint_and_named_tools,all_builtins_parse)cargo check --manifest-path Cargo.tomlcleancode_executoractually seesgrep/edit/apply_patchin its tool catalogImpact
named = [...]allowlists.--no-verifybecause the repo's pre-push hook fails on pre-existing app/ ESLint warnings (mascot / commands code) and a Tailwind class lint check — none of which this PR touches. Per CLAUDE.md (If a pre-push hook fails on something unrelated to your changes ... push with --no-verify and call it out in the PR body.)Related
Summary by CodeRabbit
New Features