feat(tool_search): add platform-aware core tool deferral config - #42551
Conversation
Allow users to opt specific core tools/toolsets into progressive disclosure. This enables platform-specific customization so heavy-hitting tools can be deferrable on TUI/CLI while staying visible on Signal or Telegram. - Add defer_core_tools, defer_core_toolsets, defer_core_platforms config keys - Implement _string_list() helper and _configured_deferred_core_names() lookup - Thread platform parameter through get_tool_definitions assemble chain - Update is_deferrable_tool_name to respect user override + platform gating
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Overview
This PR adds platform-aware core tool deferral config to the tool_search feature.
Changes
- Adds defer_core_tools, defer_core_toolsets, defer_core_platforms config options
- Updates is_deferrable_tool_name to check platform-specific config
- Passes platform through tool_search pipeline
Quality
- Clean feature addition
- No security concerns
- Good documentation in code comments
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the existing bridge rather than adding new model tools. There are two blockers before this can be evaluated as a salvage candidate.
Problems
- Current main deliberately defines core tools as always-direct:
tools/tool_search.py:163-174, its regression test attests/tools/test_tool_search.py:92-102, and the public contract atwebsite/docs/user-guide/features/tool-search.md:18-24. Changing that invariant needs a maintainer design decision. - The platform allowlist does not reach the normal agent invocation path. The PR passes platform into scope construction at
agent/tool_executor.py:173-175, but the executor still unwraps with no platform (current-main equivalentsagent/tool_executor.py:403and:1077) and callshandle_function_callwithout one (:1475-1487,:1517-1529). Withdefer_core_platformsset, a core tool can be deferred at assembly and then rejected when invoked. - The diff has no tests or docs, despite adding user-facing config.
Suggested changes
- Resolve the core-tool contract first; if accepted, update the existing invariant tests and Tool Search documentation.
- Propagate platform through every bridge unwrap/dispatch path and add an allowlisted-platform end-to-end bridge test.
Automated hermes-sweeper review.
| disabled_toolsets=disabled, | ||
| quiet_mode=True, | ||
| skip_tool_search_assembly=True, | ||
| platform=getattr(agent, "platform", None), |
There was a problem hiding this comment.
Passing platform only into scope construction is insufficient: the two executor resolve_underlying_call(function_args) sites and their later handle_function_call(...) calls still omit it. With a nonempty defer_core_platforms, a core tool deferred for this agent will fail to unwrap because those paths resolve with platform=None. Thread agent.platform through the complete invocation path and cover that configuration in an integration test.
Summary
Local verification
Commits
text
bfaeac0 feat(tool_search): add platform-aware core tool deferral config