Skip to content

Commit 07f9ac6

Browse files
Ubuntuclaude
andcommitted
fix(mcp): suppress response for JSON-RPC notifications (v1.0.656-alpha)
ADR-035 W11 fix-up wedge #12 — agy's MCP tools/call STILL failed after v1.0.654's dot-name filter. Different error, same root family. ## Root cause: JSON-RPC 2.0 §4.1 violation JSON-RPC 2.0 §4.1: 'The Server MUST NOT reply to a Notification.' A notification is a request without an `id` field. Our hub's MCP handler had a default-case error path that wrote an error frame for every unknown method — including notifications. agy 1.0.1 sends `notifications/roots/list_changed` to every connected MCP server periodically (host-verified in agytest's stderr log). Our hub fell through to default → wrote a JSON-RPC error response. The bridge piped that to agy's stdin as an unsolicited frame. agy's strict MCP client read the unsolicited frame, classified it as a protocol violation, and closed the stdio transport. Every subsequent tools/call from the LLM hit the dead transport → 'connection closed: client is closing: invalid request'. The same hub also failed for any other notification method we didn't explicitly enumerate (notifications/cancelled, etc.) — notifications/initialized only worked because it had its own case. ## Smoking gun agytest's stderr log captured the exact frame: IN {"jsonrpc":"2.0","method":"notifications/roots/list_changed","params":{}} agytest's permissive Python parser silently ignored it; our strict hub responded with an error. The cross-server log diff (agytest worked, termipod didn't, same input frame) was the load-bearing clue. Manual reproduction: $ echo '{"jsonrpc":"2.0","method":"notifications/roots/list_changed"}' | hub-mcp-bridge {"jsonrpc":"2.0","error":{"code":-32601,"message":"method not found: ..."}} (should have been empty body per JSON-RPC spec) ## Fix Insert a notification gate BEFORE the per-method switch in server/mcp.go. The standalone daemon (hubmcpserver/run.go) already handles this correctly via a 'no id → no reply' branch; only the in-process /mcp/<token> route was buggy. Lock test: TestMCP_NotificationsGetNoResponse exercises 5 notifications including the host-verified notifications/roots/list_changed and an unknown-method case (to confirm the default-path no longer fires for notifications). Each must return 204 with empty body. ## Why agy's prior diagnosis missed it agy proposed three theories — stdout corruption, PATH discrepancy, sandbox restrictions — all wrong. The actual cause was a wire-level frame agy sent AFTER initialize that hadn't been considered. Finding it required reading the agytest server's stderr log (where agy ALSO sent notifications/roots/list_changed) and comparing what agytest's permissive parser ignored vs what our strict-handler responded to. Verify-don't-guess + cross-server log comparison was the load-bearing technique. ## MCP debug arc summary Four wedges, each a distinct layer of the strict-client wall: v1.0.649 — hard-coded protocolVersion → agy 2025-11-25 rejected v1.0.653 — workdir .mcp.json pinned stale token → 401 v1.0.654 — dot-named tool aliases → tools/list batch rejected v1.0.656 — replied to notifications → client closed transport Each surfaced only after the prior was fixed — strict clients fail at the first wall they hit, hiding everything behind it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9bc1abb commit 07f9ac6

5 files changed

Lines changed: 207 additions & 3 deletions

File tree

docs/changelog.md

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
> **Type:** reference
44
> **Status:** Current (2026-05-23)
55
> **Audience:** contributors, operators
6-
> **Last verified vs code:** v1.0.655
6+
> **Last verified vs code:** v1.0.656
77
88
**TL;DR.** Append-only record of what shipped in each tagged release.
99
One section per version, newest first. Format follows
@@ -23,6 +23,134 @@ binding). Seed entries prior to that are in
2323

2424
---
2525

26+
## v1.0.656-alpha — 2026-05-23
27+
28+
ADR-035 W11 fix-up wedge #12 — agy's MCP `tools/call` STILL failed
29+
after v1.0.654's dot-name filter. Different error, same root family
30+
(strict-client rejection on protocol non-compliance). Caught from
31+
agytest's stderr log: agy sends `notifications/roots/list_changed`
32+
to every connected MCP server, but our `/mcp/<token>` endpoint
33+
returned a JSON-RPC error response for it.
34+
35+
### Root cause: JSON-RPC 2.0 §4.1 violation
36+
37+
JSON-RPC 2.0 §4.1: *"The Server MUST NOT reply to a Notification."*
38+
A notification is a request without an `id` field. Our hub's MCP
39+
handler had a default-case error path that wrote an error frame for
40+
every unknown method — including notifications. The flow:
41+
42+
1. agy spawns hub-mcp-bridge subprocess, sends `initialize` (works)
43+
2. agy sends `notifications/initialized` (works — explicitly handled)
44+
3. agy sends `tools/list` (works after v1.0.654 dot-name filter)
45+
4. **agy sends `notifications/roots/list_changed`** — this is the
46+
trigger. The hub falls through to the default `method not found`
47+
error and writes a JSON-RPC error frame back through the bridge
48+
to agy's stdin.
49+
5. agy receives an unsolicited error frame (no request was
50+
outstanding) → MCP client treats this as a protocol violation
51+
→ closes the stdio transport.
52+
6. Subsequent `tools/call` from the LLM hits a closed transport →
53+
`connection closed: calling "tools/call": client is closing:
54+
invalid request`.
55+
56+
The same hub also failed any other notification method we didn't
57+
explicitly enumerate (`notifications/cancelled`,
58+
`notifications/progress`, etc.) — `notifications/initialized` only
59+
worked because it had its own case branch.
60+
61+
### The smoking gun
62+
63+
Two pieces of evidence aligned:
64+
65+
1. **agytest's stderr log** captured the exact frame agy sends:
66+
```
67+
IN {"jsonrpc":"2.0","method":"notifications/roots/list_changed","params":{}}
68+
```
69+
No `id`, periodic, sent to every MCP server. agytest (permissive
70+
Python parser) silently ignores it. Our hub didn't.
71+
72+
2. **Manual probe against the deployed bridge** confirmed:
73+
```
74+
$ echo '{"jsonrpc":"2.0","method":"notifications/roots/list_changed"}' | hub-mcp-bridge
75+
{"jsonrpc":"2.0","error":{"code":-32601,"message":"method not found: ..."}}
76+
```
77+
An error frame on stdout where the spec requires silence.
78+
79+
### Fix
80+
81+
Insert a notification gate BEFORE the per-method switch in
82+
`server/mcp.go`:
83+
84+
```go
85+
isNotification := len(req.ID) == 0 || string(req.ID) == "null"
86+
if isNotification {
87+
w.WriteHeader(http.StatusNoContent)
88+
return
89+
}
90+
```
91+
92+
The standalone daemon path (`hubmcpserver/run.go`) already handles
93+
this correctly — only the in-process `/mcp/<token>` route was buggy.
94+
95+
Lock test: `TestMCP_NotificationsGetNoResponse` exercises 5
96+
notifications (including the host-verified
97+
`notifications/roots/list_changed` and an unknown method that must
98+
also produce no body) and asserts each returns 204 with empty body.
99+
100+
### Why agy's diagnosis missed this
101+
102+
agy's three theories from the prior session were all dead ends:
103+
104+
1. *"hub-mcp-bridge writes non-JSON to stdout"* — wrong, manual
105+
probe showed clean stdout.
106+
2. *"PATH discrepancy / agy can't find the binary"* — wrong, agy
107+
logs showed the bridge started fine.
108+
3. *"sandbox restricts the bridge's network access"* — wrong,
109+
manual `tools/call` from a wrapped bridge worked end-to-end.
110+
111+
The actual cause was a wire-level frame agy sent AFTER initialize
112+
that hadn't been considered. Finding it required reading the
113+
agytest server's stderr log (where agy ALSO sent
114+
`notifications/roots/list_changed`) and comparing what agytest's
115+
permissive parser ignored vs what our strict-handler responded
116+
to. Verify-don't-guess discipline + cross-server log comparison
117+
was the load-bearing technique.
118+
119+
### MCP debug arc, summarized
120+
121+
Four wedges, each a distinct layer of the strict-client wall:
122+
123+
| Tag | Bug | What broke |
124+
|---|---|---|
125+
| v1.0.649 | hub hard-coded protocolVersion 2024-11-05; agy sends 2025-11-25 | Negotiation |
126+
| v1.0.653 | workdir .mcp.json pinned stale token | Auth |
127+
| v1.0.654 | catalog held dot-named aliases (spec violation) | Catalog wire shape |
128+
| v1.0.656 (here) | hub replied to JSON-RPC notifications | Protocol-frame discipline |
129+
130+
Each surfaced only after the previous was fixed — strict clients
131+
fail at the first wall they hit, hiding everything behind it.
132+
133+
### Deploy
134+
135+
Builds clean, all tests pass:
136+
137+
```
138+
ok github.com/termipod/hub/internal/server 107.7s
139+
```
140+
141+
```bash
142+
sudo cp /tmp/hub-server /usr/local/bin/hub-server
143+
cp /tmp/host-runner ~/.local/bin/host-runner
144+
sudo systemctl restart termipod-hub.service
145+
# restart your tmux host-runner
146+
```
147+
148+
Then spawn an antigravity steward, ask "list termipod projects"
149+
— agy should invoke `projects_list` via the bridge and return the
150+
team's projects without "client is closing" errors.
151+
152+
---
153+
26154
## v1.0.655-alpha — 2026-05-23
27155

28156
Three independent fixes from the post-v1.0.654 review — one UX bug

hub/internal/buildinfo/buildinfo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
// mobile and hub use the same x.y.z-alpha numbering. Use
1414
// `make bump VERSION=...` from the repo root to update both files
1515
// atomically.
16-
const Version = "1.0.655-alpha"
16+
const Version = "1.0.656-alpha"
1717

1818
var (
1919
Commit string

hub/internal/server/mcp.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,27 @@ func (s *Server) handleMCP(w http.ResponseWriter, r *http.Request) {
117117
return
118118
}
119119

120+
// JSON-RPC 2.0 §4.1: a request without an `id` is a Notification —
121+
// "The Server MUST NOT reply to a Notification." Pre-v1.0.656 the
122+
// default-case error path below blindly wrote an error response for
123+
// every unknown method, including notifications, which produced an
124+
// unsolicited frame on the client's stdin. agy 1.0.1 sends
125+
// `notifications/roots/list_changed` to every MCP server it
126+
// connects to; the unsolicited error frame our hub returned looked
127+
// to agy like a protocol violation, agy closed its MCP client, and
128+
// every subsequent tools/call surfaced as `connection closed:
129+
// client is closing: invalid request`. Drop responses to all
130+
// notifications here, before the per-method switch — most methods
131+
// arrive as requests, the few that arrive as notifications
132+
// (`notifications/initialized`, `notifications/roots/list_changed`,
133+
// `notifications/cancelled`, …) all want the same 204-no-content
134+
// treatment.
135+
isNotification := len(req.ID) == 0 || string(req.ID) == "null"
136+
if isNotification {
137+
w.WriteHeader(http.StatusNoContent)
138+
return
139+
}
140+
120141
switch req.Method {
121142
case "initialize":
122143
// Parse the client-requested protocolVersion out of params so we

hub/internal/server/mcp_authority_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,61 @@ import (
1313
"github.com/termipod/hub/internal/auth"
1414
)
1515

16+
// TestMCP_NotificationsGetNoResponse pins the JSON-RPC 2.0 §4.1
17+
// contract: the server MUST NOT reply to a notification (a request
18+
// without `id`). Pre-v1.0.656 the in-process /mcp/<token> handler
19+
// wrote an error frame for every unknown method including
20+
// notifications. agy 1.0.1 sends `notifications/roots/list_changed`
21+
// to every connected MCP server (host-verified in the agytest
22+
// stderr log); the unsolicited error frame our hub returned looked
23+
// to agy like a protocol violation, agy closed its MCP client, and
24+
// subsequent tools/call surfaced as `connection closed: client is
25+
// closing: invalid request`.
26+
func TestMCP_NotificationsGetNoResponse(t *testing.T) {
27+
dir := t.TempDir()
28+
dbPath := dir + "/hub.db"
29+
token, err := Init(dir, dbPath)
30+
if err != nil {
31+
t.Fatalf("Init: %v", err)
32+
}
33+
s, err := New(Config{Listen: "127.0.0.1:0", DBPath: dbPath, DataRoot: dir})
34+
if err != nil {
35+
t.Fatalf("New: %v", err)
36+
}
37+
t.Cleanup(func() { _ = s.Close() })
38+
srv := httptest.NewServer(s.router)
39+
t.Cleanup(srv.Close)
40+
41+
// Each of these is a notification: no `id` field. Per JSON-RPC
42+
// the response body MUST be empty.
43+
cases := []string{
44+
`{"jsonrpc":"2.0","method":"notifications/roots/list_changed","params":{}}`,
45+
`{"jsonrpc":"2.0","method":"notifications/initialized"}`,
46+
`{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":"abc"}}`,
47+
`{"jsonrpc":"2.0","method":"notifications/progress","params":{"progressToken":"x","progress":0.5}}`,
48+
// An unknown notification method must also produce no body —
49+
// the default-case must not fire on notifications.
50+
`{"jsonrpc":"2.0","method":"this/does/not/exist","params":{}}`,
51+
}
52+
for _, line := range cases {
53+
req, _ := http.NewRequestWithContext(context.Background(), "POST",
54+
srv.URL+"/mcp/"+token, bytes.NewReader([]byte(line)))
55+
req.Header.Set("Content-Type", "application/json")
56+
resp, err := http.DefaultClient.Do(req)
57+
if err != nil {
58+
t.Fatalf("notification POST: %v (line=%s)", err, line)
59+
}
60+
body, _ := io.ReadAll(resp.Body)
61+
resp.Body.Close()
62+
if resp.StatusCode != http.StatusNoContent {
63+
t.Errorf("notification %q: status=%d want 204 (body=%s)", line, resp.StatusCode, body)
64+
}
65+
if len(bytes.TrimSpace(body)) != 0 {
66+
t.Errorf("notification %q: returned body %q — JSON-RPC notifications must produce no response", line, body)
67+
}
68+
}
69+
}
70+
1671
// TestMCPAuthority_RoundTrip verifies the consolidation: a spawned-agent
1772
// MCP client posting tools/list to /mcp/<token> sees the rich-authority
1873
// catalog (e.g. projects.list, agents.spawn, schedules.create) advertised

pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev
1616
# https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html
1717
# In Windows, build-name is used as the major, minor, and patch parts
1818
# of the product and file versions while build-number is used as the build suffix.
19-
version: 1.0.655-alpha+10655
19+
version: 1.0.656-alpha+10656
2020

2121
environment:
2222
sdk: ^3.10.7

0 commit comments

Comments
 (0)