refactor(mcp-parser): Generic method handling and improved logging#364
Conversation
Remove hardcoded method switch (tools/call, resources/read, prompts/get) in favor of a generic approach that handles any MCP JSON-RPC method. All methods are logged uniformly at Info level; payload is logged at Debug level (truncated to 128 chars). MCPExtension now stores raw Params instead of method-specific sub-structs, making the parser forward-compatible with future MCP protocol changes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The container image has no standalone kill/grep/cat/tr binaries. Replace broken instructions with a pure-bash-builtin one-liner that finds the authbridge PID dynamically via /proc/*/cmdline. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
esnible
left a comment
There was a problem hiding this comment.
Clean refactor that simplifies the MCP parser by removing method-specific handling in favor of storing raw params. Forward-compatible design — new MCP methods won't require code changes.
One stale doc reference needs fixing (references a removed struct field), and two helper methods are now dead code.
Areas reviewed: Go (code + tests), Documentation (3 READMEs)
Commits: 3 commits, all signed-off ✓
CI status: all passing ✓
must-fix: authbridge/demos/mcp-parser/README.md line 288 still references pctx.Extensions.MCP.Tool.Name — this struct field no longer exists after this refactor. Readers trying to write policy plugins will get a compile error. Update to pctx.Extensions.MCP.Params["name"].
suggestion: authbridge/authlib/plugins/mcpparser.go — stringParam() and mapParam() methods (lines 68-77) are now dead code with no callers. Consider removing them.
nit: authbridge/authlib/plugins/mcpparser.go line 12 — doc comment still says "populates … with the parsed method, tool name, resource URI, or prompt name" but now it stores raw params. A one-line update would keep this accurate.
- Update README reference from .Tool.Name to .Params["name"] - Update MCPParser doc comment to reflect raw params design Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
All three items addressed:
|
Dead code in this PR -- no callers remain after removing the method switch. The A2A parser (PR rossoctl#363) will re-add them when it merges. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Summary
Logging behavior
level=INFO msg=mcp-parser method=tools/calllevel=INFO msg=mcp-parser method=initializelevel=DEBUG msg="mcp-parser: payload" method=tools/call body={...truncated to 128 chars...}Test plan
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com