PR #20356: fix(acp): drop shadowing import json in build_tool_start (#20250)

• Repository: NousResearch/hermes-agent
• Author: Beandon13
• State: open | merged: False
• Link: https://github.com/NousResearch/hermes-agent/pull/20356

Description (problem / solution / changelog)

Summary

• The generic-fallback branch of acp_adapter.tools.build_tool_start had a function-scope import json even though json is imported at module scope. Python treats it as a local binding for the entire function body, which made the earlier json.dumps call in the _POLISHED_TOOLS branch raise UnboundLocalError for any polished tool without a dedicated branch.
• Affected tools include discord, discord_admin, kanban_*, ha_*, browser_* (the ones not given dedicated rendering branches), feishu_*, yb_*, vision_analyze, image_generate, text_to_speech, cronjob, send_message, mixture_of_agents, clarify, etc.
• This matches the secondary error reported in #20250:

  UnboundLocalError: cannot access local variable 'json' where it is not associated with a value
    File ".../acp_adapter/server.py", line 622, in _replay_session_history
      if not await _send(build_tool_start(tool_call_id, tool_name, args)):
    File ".../acp_adapter/tools.py", line 1128, in build_tool_start

• Removed the redundant import, kept a comment explaining the foot-gun, and added a parametrized regression test covering 20 affected polished tool names.

Refs #20250 (this is the secondary build_tool_start error; the primary lifecycle issue is not addressed here).

Testing

• scripts/run_tests.sh tests/acp/test_tools.py::TestBuildToolStart -q
• scripts/run_tests.sh tests/acp/test_tools.py -q (full file)
• Manual repro before fix: build_tool_start("tc", "discord", {}) → UnboundLocalError. After fix: returns a ToolCallStart.

Test output

▶ running pytest with 4 workers, hermetic env, in /tmp/hermes-20250a-fix
  (TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0; all credential env vars unset)
bringing up nodes...
bringing up nodes...

...............................                                          [100%]
31 passed in 2.48s

Full file:

........................................................................ [ 96%]
...                                                                      [100%]
75 passed in 2.70s

Changed files

• acp_adapter/tools.py (modified, +8/-1)
• tests/acp/test_tools.py (modified, +40/-0)

PR #20357: fix(acp): accept allow_permanent kwarg in approval callback (#20250)

• Repository: NousResearch/hermes-agent
• Author: Beandon13
• State: closed | merged: False
• Link: https://github.com/NousResearch/hermes-agent/pull/20357

Description (problem / solution / changelog)

Summary

• acp_adapter.permissions.make_approval_callback returned a callback whose signature was (command, description) only.
• tools.approval.prompt_dangerous_approval always invokes its callback with allow_permanent= (see tools/approval.py line 596), so the ACP callback raised:

  TypeError: make_approval_callback.<locals>._callback() got an unexpected keyword argument 'allow_permanent'

• The error was swallowed inside prompt_dangerous_approval and converted to a hard "deny", leaving VS Code/Zed ACP users unable to approve any dangerous command and only this log line to point at the cause:

  Approval callback failed: ... unexpected keyword argument 'allow_permanent'

• Updates the inner callback to accept *, allow_permanent=True and to drop the allow_always option from the ACP options list when allow_permanent is False (mirrors the CLI's [a]lways suppression behaviour for tirith-flagged commands).
• Adds 6 regression tests in tests/acp/test_permissions.py.

Refs #20250 (this is the second of two secondary errors reported alongside the prompt-lifecycle issue; the first was the UnboundLocalError in build_tool_start, addressed separately in #20356).

Testing

• scripts/run_tests.sh tests/acp/test_permissions.py -q
• scripts/run_tests.sh tests/acp/test_permissions.py::TestApprovalCallbackSignature -q
• Verified that the new tests fail on origin/main (TypeError on the kwarg), and pass after the patch.

Test output

▶ running pytest with 4 workers, hermetic env, in /tmp/hermes-20250b-fix
  (TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0; all credential env vars unset)
bringing up nodes...
bringing up nodes...

...........                                                              [100%]
11 passed in 2.46s

Changed files

• acp_adapter/permissions.py (modified, +18/-6)
• tests/acp/test_permissions.py (modified, +113/-0)