Skip to content

Commit 464dea4

Browse files
authored
fix(connect): resolve duplicate mcpproxy entries deterministically (#1769)
## Bug `findEntryJSONBytes` and `findEntryTOMLBytes` (`internal/connect/connect.go`) return the first entry in `range serversMap` whose URL points at this instance. Go randomizes map iteration, so when a client config holds two such entries the answer changes from call to call. That's a common shape: a legacy `?apikey=` entry under a custom key left next to the current `mcpproxy` entry. `findEquivalentJSONServerName` was already made deterministic for exactly this reason (Spec 091 FR-005). The status lookup was not. ## Trigger A Cursor `mcp.json` with these entries: ```json {"mcpServers": { "mcpproxy": {"url": "http://127.0.0.1:8080/mcp"}, "old-proxy": {"url": "http://127.0.0.1:8080/mcp?apikey=OLD"} }} ``` With this config, `GetStatus` reports `server_name` as `mcpproxy` or `old-proxy` depending on the call. `ClientSecret`, which the credential reconciler uses, reads the credential from whichever entry won. So it sometimes sees `OLD` and sometimes the current key. ## Fix Both finders now scan in a fixed order through a small `entryScanOrder` helper: the default `mcpproxy` key first, then the remaining names sorted. The name-only fallback and the PointsHere preference are unchanged. ## Verification - `go test -count=1 -run TestFindEntry_TwoEntriesPointingHere ./internal/connect/` passes. The test covers both the JSON (cursor) and TOML (codex) paths and runs 50 lookups each. - With `connect.go` reverted (`git stash push internal/connect/connect.go`), the same test fails: `cursor lookup 0: got entry "old-proxy" (pointsHere=true), want "mcpproxy"`. - `go test -count=1 ./internal/connect/` passes. - `go vet ./internal/connect/` is clean. - `gofmt -l internal/connect/` is clean. Commands were run with `GOFLAGS=-mod=readonly CGO_ENABLED=0`.
1 parent 3af8e25 commit 464dea4

2 files changed

Lines changed: 72 additions & 4 deletions

File tree

‎internal/connect/connect.go‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,8 +1610,8 @@ func (s *Service) findEntryJSONBytes(client ClientDef, raw []byte) (loc entryLoc
16101610
// scan: a sibling entry that genuinely points at us wins over it.
16111611
var nameOnly *entryLocation
16121612

1613-
for name, v := range serversMap {
1614-
entry, ok := v.(map[string]interface{})
1613+
for _, name := range entryScanOrder(serversMap) {
1614+
entry, ok := serversMap[name].(map[string]interface{})
16151615
if !ok {
16161616
continue
16171617
}
@@ -1646,6 +1646,25 @@ func (s *Service) findEntryJSONBytes(client ClientDef, raw []byte) (loc entryLoc
16461646
return entryLocation{}, false, true
16471647
}
16481648

1649+
// entryScanOrder is the order the entry finders inspect serversMap in: the
1650+
// default "mcpproxy" key first, then the rest sorted. Go randomizes map
1651+
// iteration, and with two entries pointing here (an old ?apikey= one left
1652+
// beside the current one) GetStatus's server name and ClientSecret's
1653+
// credential would otherwise flip between calls.
1654+
func entryScanOrder(serversMap map[string]interface{}) []string {
1655+
names := make([]string, 0, len(serversMap))
1656+
for name := range serversMap {
1657+
if name != defaultServerName {
1658+
names = append(names, name)
1659+
}
1660+
}
1661+
sort.Strings(names)
1662+
if _, ok := serversMap[defaultServerName]; ok {
1663+
names = append([]string{defaultServerName}, names...)
1664+
}
1665+
return names
1666+
}
1667+
16491668
// classifyEndpointMatch turns a matched entry into the EndpointMatch value the
16501669
// UI renders. PointsHere is authoritative — it means the matcher recognized our
16511670
// exact base URL. Otherwise the entry matched on its name alone: if it names an
@@ -1859,8 +1878,8 @@ func (s *Service) findEntryTOMLBytes(raw []byte) (loc entryLocation, found, pars
18591878

18601879
var nameOnly *entryLocation
18611880

1862-
for name, v := range serversMap {
1863-
entry, ok := v.(map[string]interface{})
1881+
for _, name := range entryScanOrder(serversMap) {
1882+
entry, ok := serversMap[name].(map[string]interface{})
18641883
if !ok {
18651884
continue
18661885
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package connect
2+
3+
import "testing"
4+
5+
// Two entries pointing at this instance (a legacy ?apikey= one under a custom
6+
// key beside the current "mcpproxy" one) must resolve to the same entry on
7+
// every lookup, preferring the default key.
8+
func TestFindEntry_TwoEntriesPointingHere_Deterministic(t *testing.T) {
9+
svc := NewServiceWithHome("127.0.0.1:8080", "", t.TempDir())
10+
11+
jsonRaw := []byte(`{"mcpServers": {
12+
"mcpproxy": {"url": "http://127.0.0.1:8080/mcp"},
13+
"old-proxy": {"url": "http://127.0.0.1:8080/mcp?apikey=OLD"},
14+
"another": {"url": "http://127.0.0.1:8080/mcp?apikey=OTHER"}
15+
}}`)
16+
tomlRaw := []byte(`
17+
[mcp_servers.mcpproxy]
18+
url = "http://127.0.0.1:8080/mcp"
19+
20+
[mcp_servers.old-proxy]
21+
url = "http://127.0.0.1:8080/mcp?apikey=OLD"
22+
23+
[mcp_servers.another]
24+
url = "http://127.0.0.1:8080/mcp?apikey=OTHER"
25+
`)
26+
27+
cases := []struct {
28+
client string
29+
raw []byte
30+
}{
31+
{"cursor", jsonRaw},
32+
{"codex", tomlRaw},
33+
}
34+
for _, tc := range cases {
35+
client := FindClient(tc.client)
36+
if client == nil {
37+
t.Fatalf("client %q not found", tc.client)
38+
}
39+
for i := 0; i < 50; i++ {
40+
loc, found, parsedOK := svc.findEntryFromBytes(*client, tc.raw)
41+
if !parsedOK || !found {
42+
t.Fatalf("%s: found=%v parsedOK=%v", tc.client, found, parsedOK)
43+
}
44+
if loc.Name != defaultServerName || !loc.PointsHere {
45+
t.Fatalf("%s lookup %d: got entry %q (pointsHere=%v), want %q", tc.client, i, loc.Name, loc.PointsHere, defaultServerName)
46+
}
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)