fix(agent): avoid fuzzy repair for MCP tool names - #62701
Conversation
|
Thanks for the focused regression fix. Current The regression coverage targets the affected helper in Automated hermes-sweeper review. |
e846a8b to
b9e2eb0
Compare
|
Rebased onto current |
b9e2eb0 to
07c6bbd
Compare
Deep upstream-already-fixed analysis — verdict: STILL_OPEN ✅ (does NOT qualify for close)I inspected the current upstream/main ( What I checked (real commands, real output)1. The repair function still uses
2. The function has NOT been removed/replaced/renamed to something safe. if tc.function.name not in agent.valid_tool_names:
repaired = agent._repair_tool_call(tc.function.name)
if repaired:
tc.function.name = repaired # ← silent swap happens here3. MCP tool names DO reach this function — they are in 4. No newer tool-resolution path bypasses fuzzy repair for namespaced names. 5. Clean-apply check. ConclusionThe gap is real and unaddressed on main: (Note: main's HEAD advanced from the |
07c6bbd to
d31b9b7
Compare
Thanks — this is a focused, well-scoped fix. Confirmed against current One design point worth surfacing for maintainers: this disables fuzzy repair for all unmatched |
92d4cc7 to
8aa6fed
Compare
|
Thanks for the thorough review and the design-point framing. Agreed on the safety-over-convenience trade-off: genuine intra-operation typos (e.g. Noted the overlap with #37100 (surgical suffix-scoring approach). The two are mutually exclusive on the same region of No |
b9ebe14 to
f4ca46a
Compare
9824368 to
357fa20
Compare
|
Rebased onto current upstream/main (7965462). The CI failures (slice 1/8 + 8/8) were pre-existing vercel sandbox test issues now fixed on main — not related to this PR's MCP tool name guard. |
9dfe645 to
4d76060
Compare
4d76060 to
baf4d7f
Compare
baf4d7f to
79fb33a
Compare
79fb33a to
1ea531e
Compare
1ea531e to
b75f427
Compare
1cb5ffc to
4c233d6
Compare
…vior Add pre_db_checkpoint, pre_fuzzy_repair, pre_delegation_credentials, and pre_compression hooks so plugins can override or veto specific core behaviors without patching core files. Motivation: several open PRs (NousResearch#72549, NousResearch#62701, NousResearch#61499, NousResearch#58512) fix real bugs by patching core internals that the plugin system cannot reach. Rather than carrying fork-specific patches across every upstream rebase, expose narrow, fail-open hook points that let a user-installed plugin (~/.hermes/plugins/) implement the same fixes without touching core files. Hook contracts (all fail-open — no plugin loaded = original behavior): - pre_db_checkpoint: fired before WAL checkpoint in SessionDB.close() and pre-VACUUM paths. Return {"mode": "PASSIVE"} to override the default TRUNCATE mode. Enables plugins to prevent page-tear under SIGTERM races (NousResearch#45383) without patching hermes_state.py. - pre_fuzzy_repair: fired before the fuzzy-match fallback in repair_tool_call(). Return {"skip": True} to suppress fuzzy matching for specific tool names (e.g. MCP names where fuzzy substitution changes semantics — NousResearch#62701). - pre_delegation_credentials: fired at the top of _resolve_delegation_credentials(). Return a full credential dict (with "provider" key) to short-circuit built-in resolution. Enables plugins to implement Nous JWT rotation (NousResearch#61499) before the direct-endpoint path runs. - pre_compression: fired just before context compression begins. Return {"skip": True, "reason": str} to veto compression for this tick. Side-effect hooks (e.g. rebinding a shared context engine) may run without returning (NousResearch#58512). Changes: - hermes_cli/plugins.py: add four hooks to VALID_HOOKS with docs - hermes_state.py: fire pre_db_checkpoint in close() and pre-VACUUM - hermes_state_search.py: fire pre_db_checkpoint in optimize VACUUM - agent/agent_runtime_helpers.py: fire pre_fuzzy_repair before fuzzy - tools/delegate_tool.py: fire pre_delegation_credentials at entry - agent/conversation_compression.py: fire pre_compression before start - tests/hermes_cli/test_extension_hooks.py: 26 tests covering all four hooks (default behavior, override, error fallback, kwargs) Validation: - python -m pytest tests/hermes_cli/test_extension_hooks.py → 26 passed - python -m pytest tests/test_wal_checkpoint_strategy.py tests/run_agent/test_repair_tool_call_name.py tests/hermes_cli/test_plugins.py tests/tools/test_delegate.py::TestDelegationCredentialResolution tests/run_agent/test_message_sequence_repair.py → 95 passed - python -m py_compile on all six modified files → OK - Pre-existing failures (No module named 'openai') confirmed identical on unmodified upstream/main — not caused by this change.
91dadbd to
1f43cab
Compare
b03fed4 to
223e64a
Compare
|
Rebased onto current upstream/main (395c70d). All required checks pass (47/47 pass+skip, no pending, no failures). The MCP fuzzy-repair guard (return None for |
223e64a to
02f4ee8
Compare
02f4ee8 to
b58aae2
Compare
Summary
mcp__ghidra_mcp__disassemble_functionWhy
MCP tool names encode both the server and the operation. The generic fuzzy fallback can silently turn a nonexistent MCP tool into a different valid operation inside the same namespace. For example, a model may hallucinate
mcp__ghidra_mcp__disassemble_function; because Ghidra MCP exposesdecompile_functionbut notdisassemble_function, fuzzy repair can rewrite the call tomcp__ghidra_mcp__decompile_functioninstead of letting the normal unknown-tool correction path run.That is a semantic substitution, not a safe spelling repair. It is especially risky for analysis tools where disassembly and decompilation are different evidence surfaces.
Validation
python3 -m py_compile agent/agent_runtime_helpers.py tests/run_agent/test_repair_tool_call_name.pypython3 -m pytest tests/run_agent/test_repair_tool_call_name.py -q→ 32 passedgit diff --checkNotes
Exact MCP matches and deterministic repairs still work:
_toolsuffix stripping remains allowed when it resolves to an exact registered MCP toolmcp__...names