fix: skip arg whitelist for handlers accepting **kwargs (#572)#684
fix: skip arg whitelist for handlers accepting **kwargs (#572)#684bensig merged 2 commits intoMemPalace:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates MCP tool-call dispatch to avoid schema-based argument filtering when the target handler declares **kwargs, enabling pass-through handlers to receive unexpected/extra arguments (per #572/#626 context) while keeping whitelisting for handlers with explicit parameters.
Changes:
- Detects whether a tool handler accepts
**kwargsviainspect.signature(). - Skips schema-property argument filtering when
**kwargsis present on the handler.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mempalace/mcp_server.py
Outdated
| # Skip filtering for handlers that accept **kwargs — they intentionally | ||
| # handle arbitrary arguments (e.g., pass-through to ChromaDB). | ||
| import inspect | ||
| handler = TOOLS[tool_name]["handler"] | ||
| sig = inspect.signature(handler) | ||
| accepts_var_keyword = any( | ||
| p.kind == inspect.Parameter.VAR_KEYWORD for p in sig.parameters.values() | ||
| ) | ||
| schema_props = TOOLS[tool_name]["input_schema"].get("properties", {}) | ||
| tool_args = {k: v for k, v in tool_args.items() if k in schema_props} | ||
| if not accepts_var_keyword: | ||
| tool_args = {k: v for k, v in tool_args.items() if k in schema_props} |
There was a problem hiding this comment.
Fixed — wrapped inspect.signature() in try/except, defaults to filtering on failure (safe fallback). On the explicit opt-in flag: keeping signature-based detection for now since no current handlers use **kwargs and adding a per-tool flag would be over-engineering. If a handler is added with **kwargs in the future, the inspect check catches it automatically. 7096540
The schema-based argument filter (from MemPalace#647) strips all kwargs not declared in input_schema. This breaks handlers that accept **kwargs for pass-through to ChromaDB or other backends. Add inspect.Parameter.VAR_KEYWORD check before filtering — handlers with **kwargs receive all arguments unfiltered. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jphein
force-pushed
the
fix/kwargs-var-keyword
branch
from
2b67734 to
5374e8c
Compare
Wrap inspect.signature() in try/except — on failure, default to filtering (safe fallback). Addresses Copilot feedback on fragility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bensig
left a comment
bensig
left a comment
There was a problem hiding this comment.
Code review + security audit clean.
…ged, add MemPalace#681, bump test count to 715 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Per @bensig's feedback on #626: "needs a VAR_KEYWORD check before filtering."
Files changed
Test plan
🤖 Generated with Claude Code