Skip to content

[mcp] fix: reject null RPC params - #276

Merged
Wangmerlyn merged 1 commit into
mainfrom
codex/mcp-reject-null-params
Jul 2, 2026
Merged

[mcp] fix: reject null RPC params#276
Wangmerlyn merged 1 commit into
mainfrom
codex/mcp-reject-null-params

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Reject explicit JSON-RPC params:null and other non-object params before method dispatch.
  • Preserve omitted params as legacy empty-object direct calls.
  • Add regression coverage proving stop_keep params:null does not stop existing sessions.
  • Update AGENTS and MCP/CLI docs for the parameter contract.

Test Plan

  • RED: PYTHONPATH=src pytest 'tests/mcp/test_server.py::test_jsonrpc_stop_keep_rejects_null_params_without_stopping_sessions' -q failed before the fix because active-job was stopped.
  • PYTHONPATH=src pytest 'tests/mcp/test_server.py::test_jsonrpc_stop_keep_rejects_null_params_without_stopping_sessions' -q
  • PYTHONPATH=src pytest tests/mcp/test_server.py -q
  • mkdocs build --strict
  • pre-commit run --all-files --show-diff-on-failure
  • PYTHONPATH=src pytest tests -q

Local Review

  • Local subagent code review: no Critical, Important, or Minor issues; ready to merge.

Summary by CodeRabbit

  • Bug Fixes
    • Clarified JSON-RPC handling so omitted params are treated as an empty object for compatibility, while params: null or other non-object values are rejected with Invalid params.
    • Prevented invalid direct calls from triggering any method side effects before validation completes.
  • Documentation
    • Updated user-facing JSON-RPC and CLI guidance to reflect the expected behavior for omitted and invalid params values.
  • Tests
    • Added coverage to verify invalid params are rejected and the active session remains unchanged.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de40df81-daa9-4d32-a4ad-b2a719b7e2f6

📥 Commits

Reviewing files that changed from the base of the PR and between e467e5f and 7425f5a.

📒 Files selected for processing (5)
  • AGENTS.md
  • docs/guides/mcp.md
  • docs/reference/cli.md
  • src/keep_gpu/mcp/server.py
  • tests/mcp/test_server.py

📝 Walkthrough

Walkthrough

The server's JSON-RPC request handler now explicitly checks for the presence of the params key rather than using a default-get pattern, preserving the existing rejection of non-dictionary params. A new test validates that null params are rejected without side effects. Documentation is updated to reflect this validation behavior.

Changes

JSON-RPC params validation

Layer / File(s) Summary
Server params extraction and validation
src/keep_gpu/mcp/server.py
_handle_request uses an explicit "params" in payload check instead of payload.get, defaulting to {} only when absent, and continues to raise JSONRPC_INVALID_PARAMS for non-dict params.
Regression test for null params
tests/mcp/test_server.py
New test confirms stop_keep with params=None returns INVALID_PARAMS and leaves the active session and its controller unaffected.
Documentation updates
AGENTS.md, docs/guides/mcp.md, docs/reference/cli.md
Docs clarify that omitted params default to an empty object while params: null or non-object params triggers -32602 Invalid params before any method side effects.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • Wangmerlyn/KeepGPU#86: Both PRs modify JSON-RPC params handling and validation logic in src/keep_gpu/mcp/server.py, resulting in aligned -32602 Invalid params behavior.

Poem

A rabbit checks each params with care,
Is it null, or nothing there?
Empty dict for the missing kind,
But null gets firmly left behind.
No side effects, no stopping spree—
Just -32602, plain to see. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: rejecting null RPC params in MCP JSON-RPC handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mcp-reject-null-params

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Wangmerlyn
Wangmerlyn merged commit 5648108 into main Jul 2, 2026
6 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/mcp-reject-null-params branch July 2, 2026 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant