fix: MCP notifications must not receive JSON-RPC responses (#7)

[detour1999]

Summary

Closes #7.

ccvault's MCP server was sending JSON-RPC error responses to notifications (messages without an id), violating JSON-RPC 2.0 §4.1 ("A Notification is a Request object without an id member … The Server MUST NOT reply to a Notification"). Claude Code's Zod validator rejects the id:null error line and drops the stdio transport, so MCP tools were completely unavailable.

Verification

Reporter's exact repro before this PR:

{"jsonrpc":"2.0","id":1,"result":{...}}
{"jsonrpc":"2.0","id":null,"error":{"code":-32601,"message":"Method not found","data":"notifications/initialized"}}

Two lines. Second one breaks Claude Code.

After this PR:

{"jsonrpc":"2.0","id":1,"result":{...}}

One line. Notification is silent.

Also verified notifications/cancelled and notifications/roots/list_changed (same class of bug, hit during audit) now produce zero output.

What changed

• internal/mcp/server.go — three-part fix:
  1. Renamed the dead case "initialized": to case "notifications/initialized": (the actual MCP spec name — the bare form was never reached because Claude Code sends the prefixed version)
  2. Added explicit no-op handlers for notifications/cancelled and notifications/roots/list_changed so debug logs are informative
  3. Guarded the default branch: if req.ID == nil { return } before sendError, so any unknown notification produces no response
• internal/mcp/server_test.go (new) — six tests via an injected io.Writer:
  • Three named notifications assert zero output
  • Unknown notification (notifications/some/future/thing) asserts zero output — guards the default-branch fix
  • Unknown REQUEST (with id) asserts a proper -32601 response — regression guard so we don't over-swallow
  • initialize asserts the expected result shape — regression guard for the working path

Refactor for testability

Server now has an out io.Writer field defaulting to os.Stdout in NewServer. Tests supply a bytes.Buffer to inspect emitted bytes without spawning a subprocess. NewServer's signature is unchanged; the field is unexported.

Not doing (out of scope)

• JSON-RPC batch support — spec-defined (§6) but no MCP client sends batches today. Filed conceptually during the audit; happy to open a follow-up issue if that changes.
• jsonrpc version field validation — missing/wrong jsonrpc field is currently accepted. Permissive here is arguably a feature (robust to slightly non-compliant clients); no compelling reason to tighten.

Test plan

• 6 new unit tests, all passing
• Full suite green under -race -count=1
• Reporter's exact repro against built binary — one output line, no error
• Manual probe of notifications/cancelled and notifications/roots/list_changed — both silent, exit 0

Design doc

Full audit + plan: `docs/plans/2026-08-03-mcp-notifications-fix.md`.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• Bug Fixes

  • MCP notifications are now handled silently without generating unnecessary responses.
  • Unknown notifications without request IDs are ignored, while unknown requests with IDs return clear errors.
  • Initialization responses now follow the expected format.
  • Response output handling is more reliable and testable.

• Tests

  • Added coverage for supported notifications, unknown notifications, unknown requests, and initialization responses.

• Documentation

  • Added an implementation plan outlining notification handling improvements and verification steps.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8ec5b4e2-a092-4d87-990f-1083800dfe96

📥 Commits

Reviewing files that changed from the base of the PR and between 1b70cd1 and 29feff0.

📒 Files selected for processing (3)

• docs/plans/2026-08-03-mcp-notifications-fix.md
• internal/mcp/server.go
• internal/mcp/server_test.go

---

Walkthrough

The MCP server now writes responses through an injectable io.Writer. It silently handles standard and unknown notifications, while identified unknown requests still return method-not-found errors. Tests cover notification silence, error responses, and initialization response shape.

Changes

MCP notification handling

Layer / File(s)	Summary
Configurable response output internal/mcp/server.go, internal/mcp/server_test.go, docs/plans/...	Server uses an injectable output writer, defaults it to stdout, and enables buffer-backed response tests.
Notification dispatch behavior internal/mcp/server.go, internal/mcp/server_test.go, docs/plans/...	The dispatcher handles notifications/initialized, notifications/cancelled, and notifications/roots/list_changed without responses. Unknown notifications remain silent. Identified unknown requests return -32601.
Protocol validation and delivery plan internal/mcp/server_test.go, docs/plans/...	Tests validate the initialize response shape. The plan records race testing, full-suite checks, reproduction steps, delivery actions, and excluded work.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a rabbit with a quiet reply,
No stray errors will hop by.
Buffers catch each answer bright,
Known notes fade without a byte.
MCP now thumps along just right!

✨ 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 fix/mcp-notifications

---

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