Skip to content

fix(acp): parse <tool_use> XML tags from Copilot ACP long_context mode - #45231

Open
EdderTalmor wants to merge 2 commits into
NousResearch:mainfrom
EdderTalmor:fix/45139-copilot-acp-tool-use-xml
Open

EdderTalmor wants to merge 2 commits into
NousResearch:mainfrom
EdderTalmor:fix/45139-copilot-acp-tool-use-xml

Conversation

@EdderTalmor

Copy link
Copy Markdown

Summary

When Copilot ACP is invoked with --context long_context, the model emits
Anthropic/Claude native <tool_use> XML tags instead of Hermes' expected
0xN format. This commit adds regex parsing for both self-closing
<tool_use name="..." arguments="..." id="..." /> and opening/closing
<tool_use>{JSON}</tool_use> formats.

Changes

  • Added _TOOL_USE_XML_RE regex pattern to match <tool_use> XML tags
  • Updated _extract_tool_calls_from_text() to parse <tool_use> tags
  • Added test cases for all supported <tool_use> formats

Testing

  • All existing tests pass
  • Added 5 new test cases covering:
    • Self-closing tags with JSON arguments
    • Self-closing tags with simple arguments
    • Self-closing tags with input attribute
    • Opening/closing tags with JSON content
    • Mixed <tool_use> and Hermes XML formats

Fixes #45139

EdderTalmor added 2 commits June 12, 2026 09:15
- tui_gateway/slash_worker.py: Set HERMES_SLASH_WORKER=1 for non-interactive subprocess
- cli.py: Auto-approve destructive slash commands when HERMES_SLASH_WORKER=1
- hermes_cli/web_server.py: Add gateway config cache (30s TTL) and status response cache (5s TTL)
- web/src/hooks/useSidebarStatus.ts: Reduce polling from 10s to 30s
- web/src/pages/SessionsPage.tsx: Reduce polling from 5s to 15s, remove duplicate status fetch
- tests/hermes_cli/test_destructive_slash_confirm_gate.py: Add test coverage for slash worker auto-approval
- hermes_cli/web_server.py: Bypass caches during pytest runs (PYTEST_CURRENT_TEST)
When Copilot ACP is run with --context long_context, the model emits
Anthropic/Claude native <tool_use> XML tags instead of Hermes'
expected format. This commit adds regex parsing for both self-closing
<tool_use name=... arguments=... id=... /> and opening/closing
<tool_use>{JSON}</tool_use> formats.

Fixes NousResearch#45139
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified clean — bundled PR with multiple concerns.

Reviewed 4 distinct changes in this PR:

  1. <tool_use> XML parsing (copilot_acp_client.py): The regex _TOOL_USE_XML_RE correctly handles both self-closing tags (<tool_use name="..." arguments="..." id="..." />) and opening/closing tags (<tool_use>{...}</tool_use>). The arguments attribute capture group handles embedded quotes. Good test coverage with 5 test cases.

  2. HERMES_SLASH_WORKER auto-approve (cli.py): The early-return "once" in _confirm_destructive_slash when HERMES_SLASH_WORKER=1 is intentional — the TUI gateway slash worker is a non-interactive JSON protocol where the user already confirmed by sending the command. The env var is set in tui_gateway/slash_worker.py only.

  3. Status response caching (web_server.py): 5s TTL for /api/status, 30s TTL for gateway config. Thread-safe with locks. The PYTEST_CURRENT_TEST bypass prevents cross-test contamination. Acceptable for a status endpoint.

  4. Poll interval changes (useSidebarStatus.ts, SessionsPage.tsx): Sidebar poll 10s→30s, sessions overview 5s→15s. Reasonable for reducing unnecessary API calls.

No issues found.

@alt-glitch alt-glitch added type/bug Something isn't working comp/acp Agent Communication Protocol adapter provider/copilot GitHub Copilot (ACP + Chat) P2 Medium — degraded but workaround exists labels Jun 12, 2026
@alt-glitch alt-glitch added the comp/dashboard Web dashboard / control panel UI (dashboard/, landing) label Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for targeting a verified Copilot ACP regression. Current main still only extracts <tool_call> blocks and OpenAI-shaped bare JSON in agent/copilot_acp_client.py:333-343, so the <tool_use> gap remains.

Problems

  • The new helper returns SimpleNamespace calls, but current main deliberately constructs ChatCompletionMessageToolCall objects through _build_openai_tool_call() in agent/copilot_acp_client.py:236-249 (introduced by 0106082d1). Preserve that contract when adding this parser.
  • The new tests accept raw values such as "ls -la" and "path=/tmp/test.txt" as function arguments. Current execution rejects non-JSON arguments at agent/conversation_loop.py:4531-4549, so these paths still cannot execute a tool.
  • The bundled status cache needs reconsideration during salvage: current /api/status is profile-scoped at hermes_cli/web_server.py:2555-2571, while the proposed cache has one global response.

Suggested changes

  • Port only the ACP parser through the current OpenAI-shaped call factory and add a fixture from the reported long-context output.
  • Require normalized JSON-object arguments for every extracted call, with an end-to-end adapter-level assertion that the call reaches the execution path.
  • Split or omit the unrelated slash-worker and dashboard caching changes.

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists provider/copilot GitHub Copilot (ACP + Chat) 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tool execution fails silently with Copilot ACP --context long_context (Claude outputs <tool_use>)

4 participants