Skip to content

[RFC] feat(agent): allow per-AIAgent tool injection via extra_tools= - #13315

Open
liujinkun2025 wants to merge 1 commit into
NousResearch:mainfrom
liujinkun2025:feat/agent-extra-tools
Open

[RFC] feat(agent): allow per-AIAgent tool injection via extra_tools=#13315
liujinkun2025 wants to merge 1 commit into
NousResearch:mainfrom
liujinkun2025:feat/agent-extra-tools

Conversation

@liujinkun2025

Copy link
Copy Markdown
Contributor

Motivation

Today every tool an agent can reach must be registered in the global
tools.registry at import time. That fits cross-cutting capabilities
(memory, session_search, todo, send_message, …) but is a poor
fit for two other kinds of tool:

  1. Domain-specific — only meaningful inside one platform or scenario
    (e.g. a Feishu doc reader, RL training, a Home Assistant entity
    getter). Today such tools are visible to any agent that opts into
    their toolset, even on unrelated channels.
  2. Instance-bound — the handler needs to close over a runtime value
    owned by the caller (API client, per-event whitelist, auth context).
    Today those handlers resort to thread-locals or module singletons,
    with the familiar correctness concerns around asyncio.to_thread
    and multi-account fan-out.

The pattern already exists — just not generalized

Hermes already implements per-agent tool injection for exactly one
consumer: the memory provider.

run_agent.py lines ~1408-1422:

if self._memory_manager and self.tools is not None:
    _existing = {t.get("function", {}).get("name") for t in self.tools}
    for _schema in self._memory_manager.get_all_tool_schemas():
        _tname = _schema.get("name", "")
        if _tname in _existing:
            continue
        self.tools.append({"type": "function", "function": _schema})
        self.valid_tool_names.add(_tname)

And in _invoke_tool (~line 7998):

elif self._memory_manager and self._memory_manager.has_tool(function_name):
    return self._memory_manager.handle_tool_call(function_name, function_args)

That is conceptually extra_tools — schemas merged into self.tools
plus a dispatch branch that checks per-agent handler storage before
falling through to the global registry. It's just hard-wired for
memory.

Proposal

Promote the pattern to a generic AIAgent.__init__ parameter:

AIAgent(
    ...,
    extra_tools=[{"schema": <openai-fn-schema>, "handler": <callable>}, ...],
)

Behavior (see the commit for the ~25-line implementation):

  • Schemas merge into self.tools under the same {"type": "function", ...}
    envelope as registry tools, so the model sees them uniformly.
  • Handlers are indexed in a per-instance self._extra_tool_handlers
    dict and dispatched by _invoke_tool after the hard-coded
    agent-level built-ins (todo / memory / clarify /
    delegate_task / session_search) and before the registry
    fallback. Order means extra_tools can augment an agent but cannot
    shadow critical built-ins.
  • Name collisions with entries already in self.tools (registry or
    earlier extra_tools) are logged and skipped.

Concrete use case

PR #13045
reworks the Feishu document-comment integration. Two findings there
would have been unnecessary with extra_tools:

  • feishu_doc_read and the drive-comment tools had to live in the
    global registry, even though they are only meaningful inside the
    comment handler. They were deleted and replaced with an ad-hoc
    two-pass <NEED_DOC_READ> sentinel protocol on top of plaintext
    responses. That protocol has been harder to secure (three injection
    variants found so far) than a proper function-call tool would be.
  • The lark client had to be injected via threading.local because
    there was no other way to pass an instance-bound dependency to a
    registry-level handler. This raised correctness concerns around
    asyncio.to_thread and multi-account paths.

Other likely consumers:

  • rl_* — only meaningful when an RL training run is active.
  • homeassistant/* — only meaningful when HASS_TOKEN is configured
    and a Home Assistant server is reachable.
  • Future external plugins via hermes_cli/plugins: today
    PluginContext.register_tool calls registry.register, making the
    plugin's tools visible to every agent with no hook for per-agent
    scoping.

Discussion points

  1. Is the general direction acceptable? If not, what's the preferred
    alternative for "tool real enough to show the model but should not
    live in the global registry"?
  2. Bikeshed: parameter name — extra_tools, agent_tools,
    instance_tools, scoped_tools? Happy to follow whatever
    convention fits hermes's style.
  3. Dispatch order: current placement is after hard-coded built-ins,
    before registry fallback. I chose "cannot shadow built-ins" as
    the default safety posture; open to reversing it if the convention
    is "caller knows best".

Test plan

  • 11 new unit tests in tests/run_agent/test_extra_tools.py
    covering construction (None / empty / single / multiple),
    shadow prevention (registry collision, built-in collision),
    dispatch (handler invoked for its name, unknown names fall
    through to registry, built-in dispatch order wins over
    extra_tools), and schema-envelope symmetry with registry tools.
  • Existing tests/run_agent/test_run_agent.py (275 tests) passes
    unchanged.

Generalizes the existing per-agent tool injection pattern (currently
hard-wired for memory_manager at run_agent.py:1408-1422 / :7998) into a
first-class ``AIAgent.__init__`` parameter.

Motivation
----------
Today every tool an agent can reach must be registered in the global
``tools.registry`` at import time.  This works for cross-cutting
capabilities (memory, session_search, todo, send_message) but fits
poorly for:

  - Domain-specific tools that only make sense inside one platform or
    scenario (e.g. a Feishu doc reader, RL training tools, a Home
    Assistant entity reader).  Today such tools are visible to any
    agent that opts in to their toolset, even on unrelated channels.
  - Instance-bound tools where the handler needs to close over a
    runtime value owned by the caller (API client, per-event
    whitelist, auth context).  Today those handlers resort to
    thread-locals or module singletons with the familiar correctness
    concerns around asyncio.to_thread and multi-account fan-out.

Design
------
New optional parameter:

    AIAgent(extra_tools=[{"schema": ..., "handler": ...}, ...])

Behavior:
  - Schemas merge into ``self.tools`` so the model sees them under the
    same ``{"type": "function", ...}`` envelope as registry tools.
  - Handlers are indexed in a per-instance ``self._extra_tool_handlers``
    dict and dispatched by ``_invoke_tool`` AFTER the hard-coded
    agent-level built-ins (todo / memory / clarify / delegate_task /
    session_search) and BEFORE the registry fallback.  Order means
    extra_tools can augment an agent with domain-specific tools that
    need never appear in the global registry, but cannot shadow the
    critical built-ins by accident.
  - Name collisions with names already in ``self.tools`` (registry or
    earlier extra_tools entries) are logged and skipped.

The shape intentionally mirrors the existing memory-manager injection
so the two can be unified in the future.

First known consumer: the Feishu document-comment handler reworked in
the global registry and close its workaround of a plaintext sentinel
protocol over the agent's text response.

This PR is posted in RFC mode — happy to adjust naming (``extra_tools``
vs ``agent_tools`` / ``scoped_tools`` / ``instance_tools``), shadow-
handling semantics, or the dispatch position if maintainers prefer a
different convention.

Change-Id: I1891b7f89f8455479adb4d46a7b1c765e34598c5
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Implementation PR for RFC #13344 (per-AIAgent tool injection via extra_tools=).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the concrete RFC and reference implementation. The per-agent need remains real on current main: tool definitions are assembled in agent/agent_init.py:1159-1168, while the only analogous instance-scoped injection is memory at agent/agent_init.py:1419-1420.

Problems

  • The patch targets pre-refactor seams. Current AIAgent.__init__ forwards to agent/agent_init.py (run_agent.py:416-491), and current sequential execution bypasses _invoke_tool for registry fallback (agent/tool_executor.py:1406-1418, 1448-1460). The feature must be wired through those current paths, not only the old run_agent.py method bodies.
  • run_agent.py:1337 only checks currently exposed schemas. The PR test intentionally permits an absent todo schema (tests/run_agent/test_extra_tools.py:149-170), although dispatch then executes the builtin. Main's memory manager rejects reserved _HERMES_CORE_TOOLS names before advertisement/routing (agent/memory_manager.py:400-424); this API should preserve that invariant.

Suggested changes

  • Port initialization and dispatch to agent/agent_init.py and both executor modes, with shared behavior.
  • Reject reserved names before schema insertion and add a disabled-core-tool collision test.

Automated hermes-sweeper review.

Comment thread run_agent.py
for spec in extra_tools:
schema = spec["schema"]
name = schema["name"]
if name in _existing:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_existing only represents the currently exposed schema list. A disabled todo/clarify/other agent-owned name can therefore be advertised and stored here even though the hard-coded dispatcher wins later. Reject a central reserved-name set before insertion, as current agent/memory_manager.py:400-424 does.

Comment thread run_agent.py
max_iterations=function_args.get("max_iterations"),
parent_agent=self,
)
elif function_name in self._extra_tool_handlers:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main's sequential executor bypasses _invoke_tool and falls through directly to the registry (agent/tool_executor.py:1406-1418, 1448-1460); only its concurrent worker uses _invoke_tool (:569-578). Port this dispatch to the current shared/sequential execution paths or extra tools will not run consistently.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants