Skip to content

Commit 0a492a2

Browse files
authored
Merge pull request #28 from 0xeb/fix/jsonrpc-notification-handling
Fix JSON-RPC 2.0 notification handling in StreamableHttpServerWrapper
2 parents b4bfd88 + 811840a commit 0a492a2

4 files changed

Lines changed: 140 additions & 11 deletions

File tree

.githooks/README.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Git Hooks
2+
3+
This directory contains git hooks for the fastmcpp project.
4+
5+
## Installation
6+
7+
Run one of the following commands to enable the hooks:
8+
9+
```bash
10+
# Option 1: Configure git to use this directory for hooks
11+
git config core.hooksPath .githooks
12+
13+
# Option 2: Symlink (Linux/macOS)
14+
ln -sf ../../.githooks/pre-commit .git/hooks/pre-commit
15+
```
16+
17+
## Available Hooks
18+
19+
### pre-commit
20+
21+
Automatically formats staged C++ files with clang-format before commit.
22+
23+
- Finds clang-format (prefers versioned like clang-format-19)
24+
- Formats only staged `.cpp`, `.hpp`, `.h`, `.c` files
25+
- Re-stages formatted files automatically
26+
27+
This ensures all committed code follows the project's formatting style.

.githooks/pre-commit

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,34 @@
11
#!/bin/bash
2-
# Pre-commit hook: auto-format staged C++ files
3-
#
4-
# Enable with: git config core.hooksPath .githooks
2+
# Pre-commit hook to automatically format C++ files with clang-format
3+
# Install: git config core.hooksPath .githooks
54

6-
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\.(cpp|hpp)$')
5+
# Find clang-format (prefer versioned, fall back to unversioned)
6+
CLANG_FORMAT=""
7+
for cf in clang-format-19 clang-format-18 clang-format-17 clang-format; do
8+
if command -v "$cf" &> /dev/null; then
9+
CLANG_FORMAT="$cf"
10+
break
11+
fi
12+
done
713

8-
if [ -z "$STAGED_FILES" ]; then
14+
if [ -z "$CLANG_FORMAT" ]; then
15+
echo "Warning: clang-format not found, skipping formatting"
916
exit 0
1017
fi
1118

12-
# Check if clang-format is available
13-
if ! command -v clang-format &> /dev/null; then
14-
echo "Warning: clang-format not found, skipping auto-format"
19+
# Get staged C++ files
20+
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\.(cpp|hpp|h|c)$')
21+
22+
if [ -z "$STAGED_FILES" ]; then
1523
exit 0
1624
fi
1725

18-
# Auto-format and re-stage
26+
# Format each staged file
1927
for file in $STAGED_FILES; do
2028
if [ -f "$file" ]; then
21-
clang-format -i "$file"
29+
$CLANG_FORMAT -i "$file"
2230
git add "$file"
2331
fi
2432
done
2533

26-
exit 0
34+
echo "Formatted $(echo "$STAGED_FILES" | wc -w) file(s) with $CLANG_FORMAT"

src/server/streamable_http_server.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,27 @@ bool StreamableHttpServerWrapper::start()
211211
return;
212212
}
213213

214+
// Check if this is a notification (no "id" field means notification)
215+
// JSON-RPC 2.0 spec: server MUST NOT reply to notifications
216+
bool is_notification = !message.contains("id") || message["id"].is_null();
217+
218+
if (is_notification)
219+
{
220+
// For notifications, call handler but don't send response body
221+
// This is required by JSON-RPC 2.0 spec and MCP protocol
222+
try
223+
{
224+
handler_(message); // Process but ignore result
225+
}
226+
catch (...)
227+
{
228+
// Silently ignore errors for notifications
229+
}
230+
res.set_header("Mcp-Session-Id", session_id);
231+
res.status = 202; // Accepted, no content
232+
return;
233+
}
234+
214235
// Normal request - process with handler
215236
auto response = handler_(message);
216237

tests/server/streamable_http_integration.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,78 @@ void test_default_timeout_allows_slow_tool()
452452
server.stop();
453453
}
454454

455+
void test_notification_handling()
456+
{
457+
std::cout << " test_notification_handling... " << std::flush;
458+
459+
const int port = 18356;
460+
const std::string host = "127.0.0.1";
461+
462+
// Create minimal handler
463+
tools::ToolManager tool_mgr;
464+
std::unordered_map<std::string, std::string> descriptions;
465+
auto handler = mcp::make_mcp_handler("notification_test", "1.0.0", tool_mgr, descriptions);
466+
467+
server::StreamableHttpServerWrapper server(handler, host, port, "/mcp");
468+
bool started = server.start();
469+
assert(started && "Server failed to start");
470+
471+
std::this_thread::sleep_for(std::chrono::milliseconds(500));
472+
473+
try
474+
{
475+
httplib::Client cli(host, port);
476+
cli.set_connection_timeout(5, 0);
477+
cli.set_read_timeout(5, 0);
478+
479+
// First initialize to get a session
480+
Json init_request = {{"jsonrpc", "2.0"},
481+
{"id", 1},
482+
{"method", "initialize"},
483+
{"params",
484+
{{"protocolVersion", "2024-11-05"},
485+
{"capabilities", Json::object()},
486+
{"clientInfo", {{"name", "test"}, {"version", "1.0"}}}}}};
487+
488+
auto init_res = cli.Post("/mcp", init_request.dump(), "application/json");
489+
assert(init_res && init_res->status == 200);
490+
491+
std::string session_id = init_res->get_header_value("Mcp-Session-Id");
492+
assert(!session_id.empty() && "Should have session ID");
493+
494+
// Now send a notification (no "id" field = notification per JSON-RPC 2.0)
495+
// JSON-RPC 2.0 spec: server MUST NOT reply to notifications
496+
Json notification = {{"jsonrpc", "2.0"}, {"method", "notifications/initialized"}};
497+
498+
httplib::Headers headers = {{"Mcp-Session-Id", session_id}};
499+
auto notif_res = cli.Post("/mcp", headers, notification.dump(), "application/json");
500+
501+
// Server should return 202 Accepted with no content body
502+
assert(notif_res && "Notification request should succeed");
503+
assert(notif_res->status == 202 && "Notification should return 202 Accepted");
504+
assert(notif_res->body.empty() && "Notification response should have no body");
505+
506+
// Test another common notification: notifications/cancelled
507+
Json cancel_notification = {{"jsonrpc", "2.0"},
508+
{"method", "notifications/cancelled"},
509+
{"params", {{"requestId", "123"}, {"reason", "timeout"}}}};
510+
511+
auto cancel_res = cli.Post("/mcp", headers, cancel_notification.dump(), "application/json");
512+
assert(cancel_res && cancel_res->status == 202 && "Cancel notification should return 202");
513+
assert(cancel_res->body.empty() && "Cancel notification response should have no body");
514+
515+
std::cout << "PASSED\n";
516+
}
517+
catch (const std::exception& e)
518+
{
519+
std::cout << "FAILED: " << e.what() << "\n";
520+
server.stop();
521+
throw;
522+
}
523+
524+
server.stop();
525+
}
526+
455527
int main()
456528
{
457529
std::cout << "Streamable HTTP Integration Tests\n";
@@ -465,6 +537,7 @@ int main()
465537
test_server_info();
466538
test_error_handling();
467539
test_default_timeout_allows_slow_tool();
540+
test_notification_handling();
468541

469542
std::cout << "\nAll tests passed!\n";
470543
return 0;

0 commit comments

Comments
 (0)