Skip to content

feat(acp): read platform_toolsets.acp from config - #46832

Open
jonboh wants to merge 1 commit into
NousResearch:mainfrom
jonboh:acp-platform-tools
Open

feat(acp): read platform_toolsets.acp from config#46832
jonboh wants to merge 1 commit into
NousResearch:mainfrom
jonboh:acp-platform-tools

Conversation

@jonboh

@jonboh jonboh commented Jun 15, 2026

Copy link
Copy Markdown

What does this PR do?

Makes ACP toolset selection configurable via the profile's config.yaml:

platform_toolsets:
  acp:
    - hermes-acp
    - memory

This allows ACP sessions to include more tools like the 'memory' toolset, enabling honcho_* tools (honcho_profile, honcho_search, honcho_reasoning, honcho_context, honcho_conclude) in the editor-agent session.

Default behaviour is unchanged (['hermes-acp']).

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

Read the config tools when running the an ACP server.

How to Test

  1. Add any other tool apart from hermes_acp
  2. Ask the agent to perform that tool usage
  3. Should succeed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: NixOS 25.11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

…ng ['hermes-acp']

Makes ACP toolset selection configurable via the profile's config.yaml:

    platform_toolsets:
      acp:
        - hermes-acp
        - memory

This allows ACP sessions to include the 'memory' toolset, enabling
for example honcho_* tools (honcho_profile, honcho_search, honcho_reasoning,
honcho_context, honcho_conclude) in the editor-agent session.

Default behaviour is unchanged (['hermes-acp']).
@alt-glitch alt-glitch added type/feature New feature or request comp/acp Agent Communication Protocol adapter area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jun 15, 2026
@jonboh

jonboh commented Jun 15, 2026

Copy link
Copy Markdown
Author

This pr is more minimal than: #45958
The way in which the tools for the ACP are set differs, here we set it through the config.yaml, while in that PR, the change makes hermes honor the --toolsets passed through the cli.
Both address allowing ACP agents to access a greater set of tools than just the ACP specific 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 focused ACP configuration proposal. The current hardcoded ACP selection at acp_adapter/session.py:619-622 means the broader configuration gap is real.

Problems

  • The new lookup at acp_adapter/session.py:591 will raise if YAML contains platform_toolsets: with no value. The shared resolver handles that supported shape with config.get("platform_toolsets") or {} in hermes_cli/tools_config.py:1646.
  • The sample memory addition does not enable Honcho tools on current main: hermes-acp already includes memory (toolsets.py:390), and the provider gate resolves composites (agent/memory_manager.py:82-97; coverage at tests/agent/test_memory_provider.py:1363-1368).

Suggested changes

  • Normalize/type-check the mapping before selecting acp, retaining the existing default for null or malformed values.
  • Add regression coverage for a non-default ACP toolset, the default path, and null platform_toolsets.
  • Document the ACP config key and describe it as extending the curated surface rather than enabling memory-provider tools.

This is an automated hermes-sweeper review.

Comment thread acp_adapter/session.py
if not isinstance(cfg, dict) or cfg.get("enabled", True) is not False
]

acp_toolsets = list(config.get("platform_toolsets", {}).get("acp", ["hermes-acp"]))

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.

Please normalize/type-check platform_toolsets before this lookup. A YAML platform_toolsets: with no value parses as None, so this .get() raises; the shared resolver handles that shape with config.get("platform_toolsets") or {} in hermes_cli/tools_config.py:1646.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/acp Agent Communication Protocol adapter P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants