feat(hermes): Add Hermes Mnemosyne tool whitelist#364
Conversation
4d99f69 to
3961f0e
Compare
AxDSan
left a comment
There was a problem hiding this comment.
Sorry for the delay, was IRL busy the last few days.
@timbeaulac clean implementation of #358. The shape matches what we discussed: filter before registration, loud unknown-tool validation, both provider surfaces updated in sync. The count-agnostic filter against live ALL_TOOL_SCHEMAS is the right call here since the tool surface churns between releases.
What I verified:
py_compileclean on both provider inits and the parity test.- The filter semantics: omitted/null preserves all,
[]exposes none while keeping provider context, non-empty list restricts to listed tools in listed order, unknown names raiseValueErrorat schema registration. Reads exactly like the spec. has_tool()andhandle_tool_call()share the filtered surface, so direct routing does not bypass the allowlist. Good.- CI green: docs-check and test on 3.10/3.11/3.12/3.13 all SUCCESS after the parity-test import isolation fix.
LGTM with one ask:
CHANGELOG entry. New user-facing config knob (memory.mnemosyne.tools) deserves a one-line [Unreleased] entry under the next version block. No version bump asked since this is opt-in and additive: existing deployments with no tools key see no behavior change, and the worst-case failure (unknown tool name) is loud at registration time.
Refreshing the parity-test note: that import-isolation tweak is the kind of fix that lands in a PR and then quietly breaks the next contributor's local test run because they don't know to set the PYTHONPATH. Worth promoting it to a conftest.py or a fixture so it is automatic. Optional, not a blocker for this PR.
Closing #358 cleanly.
3961f0e to
57b9789
Compare
|
Thanks — and no worries at all on timing. Appreciate the careful review. I added the I’ll leave the |
Summary
Adds a provider-local
memory.mnemosyne.toolsallowlist for Hermes Mnemosyne tool exposure, matching the direction discussed in #358.Explicitly implements AxDSan’s requested shape from #358: filter before registration, loud unknown-tool validation, and both Hermes provider paths updated in sync.
This implementation is intentionally count-agnostic: it filters the live
ALL_TOOL_SCHEMASlist rather than assuming 15, 23, 26, or 33 tools. I verified the branch against current upstreamorigin/main/ latest releasev3.10.0, where the full Hermes tool surface is in the low 30s.Behavior:
nullconfig preserves existing behavior and exposes all Mnemosyne Hermes tools.tools: []exposes no Mnemosyne tools while preserving the provider memory context / prefetch surface.toolsexposes only the listed tools, in listed order.ValueErrorfrom schema registration.has_tool()andhandle_tool_call()share the same filtered surface, so filtered-out tools are not accepted by direct routing.hermes_memory_provider/__init__.pyintegrations/hermes/src/mnemosyne_hermes/__init__.pyTests
Local targeted checks passed:
GitHub Actions passed on PR #364 after isolating the parity test imports so they do not pollute the rest of the suite:
Related