Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion brain-bar/Sources/BrainBar/BrainBarServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ final class BrainBarServer: @unchecked Sendable {
framed = data
} else {
// Newline-delimited JSON-RPC (Claude Code v2.1+ / MCP 2025-11-25)
guard let jsonData = try? JSONSerialization.data(withJSONObject: response) else { return false }
guard let jsonData = try? MCPFraming.encodeJSONResponse(response) else { return false }
var data = jsonData
data.append(0x0A) // trailing \n
framed = data
Expand Down
32 changes: 31 additions & 1 deletion brain-bar/Sources/BrainBar/MCPFraming.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,45 @@ struct MCPFraming: Sendable {

/// Encode a JSON-RPC response with Content-Length framing.
static func encode(_ response: [String: Any]) throws -> Data {
let jsonData = try JSONSerialization.data(withJSONObject: response)
let jsonData = try encodeJSONResponse(response)
let header = "Content-Length: \(jsonData.count)\r\n\r\n"
var frame = Data(header.utf8)
frame.append(jsonData)
return frame
}

/// Encode JSON-RPC envelopes with a deterministic top-level key order.
///
/// Claude Desktop currently rejects otherwise-valid JSON-RPC objects when
/// Foundation serializes `result` or `id` before `jsonrpc`.
static func encodeJSONResponse(_ response: [String: Any]) throws -> Data {
guard let jsonrpc = response["jsonrpc"],
let id = response["id"],
response["result"] != nil || response["error"] != nil else {
return try JSONSerialization.data(withJSONObject: response)
Comment on lines +127 to +131
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Keep notification envelopes on the deterministic path too.

This helper still falls back to JSONSerialization when result/error is absent, so newline-delimited notifications can still come out with Foundation-dependent top-level ordering. If those messages hit the same client/parser, the original ordering bug is not fully closed here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Sources/BrainBar/MCPFraming.swift` around lines 127 - 131, The
encodeJSONResponse helper currently falls back to JSONSerialization when
"result"/"error" are missing, which lets notification envelopes use
non-deterministic Foundation ordering; update encodeJSONResponse to always use
the deterministic JSON path (remove the JSONSerialization fallback) so
notifications as well as responses are encoded via the deterministic encoder
used elsewhere (referencing encodeJSONResponse and the
deterministicEncoder/encoding helper in MCPFraming) and return that Data for all
messages.

}

var data = Data("{\"jsonrpc\":".utf8)
data.append(try encodeJSONValue(jsonrpc))
data.append(Data(",\"id\":".utf8))
data.append(try encodeJSONValue(id))
if let result = response["result"] {
data.append(Data(",\"result\":".utf8))
data.append(try encodeJSONValue(result))
} else if let error = response["error"] {
data.append(Data(",\"error\":".utf8))
data.append(try encodeJSONValue(error))
}
data.append(0x7D) // }
return data
}

// MARK: - Private

private static func encodeJSONValue(_ value: Any) throws -> Data {
try JSONSerialization.data(withJSONObject: value, options: [.fragmentsAllowed])
}

private func parseContentLength(_ header: String) -> Int? {
for line in header.split(separator: "\r\n") {
let trimmed = line.trimmingCharacters(in: .whitespaces)
Expand Down
21 changes: 21 additions & 0 deletions brain-bar/Tests/BrainBarTests/MCPFramingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,27 @@ final class MCPFramingTests: XCTestCase {
XCTAssertTrue(str.contains("protocolVersion"))
}

func testEncodesJSONRPCEnvelopeWithJSONRPCFirst() throws {
let response: [String: Any] = [
"id": 2,
"jsonrpc": "2.0",
"result": [
"tools": [
["name": "brain_search"]
]
] as [String: Any]
]

let framed = try MCPFraming.encode(response)
let frame = try XCTUnwrap(String(data: framed, encoding: .utf8))
let body = try XCTUnwrap(frame.components(separatedBy: "\r\n\r\n").last)

XCTAssertTrue(
body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":"#),
"Claude Desktop expects the JSON-RPC envelope to serialize with jsonrpc first; got: \(body.prefix(80))"
)
}

// MARK: - Empty body

func testRejectsZeroContentLength() {
Expand Down
18 changes: 18 additions & 0 deletions brain-bar/Tests/BrainBarTests/MCPRouterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ final class MCPRouterTests: XCTestCase {
XCTAssertEqual(toolNames, expected)
}

func testEncodedToolsListEnvelopeStartsWithJSONRPC() throws {
let router = MCPRouter()
let response = router.handle([
"jsonrpc": "2.0",
"id": 2,
"method": "tools/list",
])

let framed = try MCPFraming.encode(response)
let frame = try XCTUnwrap(String(data: framed, encoding: .utf8))
let body = try XCTUnwrap(frame.components(separatedBy: "\r\n\r\n").last)

XCTAssertTrue(
body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":"#),
"Claude Desktop expects jsonrpc to be the first envelope key for tools/list; got: \(body.prefix(80))"
Comment on lines +155 to +157
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix the expected prefix.

tools/list returns an object in result, so the framed body starts with ...,"result":{..., not ...,"result":".... As written, this assertion will fail even when the encoder is correct.

🔧 Proposed fix
-        XCTAssertTrue(
-            body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":"#),
-            "Claude Desktop expects jsonrpc to be the first envelope key for tools/list; got: \(body.prefix(80))"
-        )
+        XCTAssertTrue(
+            body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":{"#),
+            "Claude Desktop expects jsonrpc to be the first envelope key for tools/list; got: \(body.prefix(80))"
+        )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
XCTAssertTrue(
body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":"#),
"Claude Desktop expects jsonrpc to be the first envelope key for tools/list; got: \(body.prefix(80))"
XCTAssertTrue(
body.hasPrefix(#"{"jsonrpc":"2.0","id":2,"result":{"#),
"Claude Desktop expects jsonrpc to be the first envelope key for tools/list; got: \(body.prefix(80))"
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Tests/BrainBarTests/MCPRouterTests.swift` around lines 155 - 157,
The assertion currently expects the framed response to contain "result":" (a
string) but tools/list returns an object, so update the XCTAssertTrue that uses
body.hasPrefix(...) in MCPRouterTests.swift to expect ...,"result":{ instead of
...,"result":"; locate the XCTAssertTrue call (the one checking body.hasPrefix
and the accompanying failure message) and change the prefix string accordingly
so the test matches an object-valued result.

)
}

func testEachToolHasInputSchema() throws {
let router = MCPRouter()
let request: [String: Any] = [
Expand Down
Loading