diff --git a/internal/connect/connect.go b/internal/connect/connect.go index 53a47dd45..b51ebf458 100644 --- a/internal/connect/connect.go +++ b/internal/connect/connect.go @@ -1611,8 +1611,8 @@ func (s *Service) findEntryJSONBytes(client ClientDef, raw []byte) (loc entryLoc // scan: a sibling entry that genuinely points at us wins over it. var nameOnly *entryLocation - for name, v := range serversMap { - entry, ok := v.(map[string]interface{}) + for _, name := range entryScanOrder(serversMap) { + entry, ok := serversMap[name].(map[string]interface{}) if !ok { continue } @@ -1647,6 +1647,25 @@ func (s *Service) findEntryJSONBytes(client ClientDef, raw []byte) (loc entryLoc return entryLocation{}, false, true } +// entryScanOrder is the order the entry finders inspect serversMap in: the +// default "mcpproxy" key first, then the rest sorted. Go randomizes map +// iteration, and with two entries pointing here (an old ?apikey= one left +// beside the current one) GetStatus's server name and ClientSecret's +// credential would otherwise flip between calls. +func entryScanOrder(serversMap map[string]interface{}) []string { + names := make([]string, 0, len(serversMap)) + for name := range serversMap { + if name != defaultServerName { + names = append(names, name) + } + } + sort.Strings(names) + if _, ok := serversMap[defaultServerName]; ok { + names = append([]string{defaultServerName}, names...) + } + return names +} + // classifyEndpointMatch turns a matched entry into the EndpointMatch value the // UI renders. PointsHere is authoritative — it means the matcher recognized our // exact base URL. Otherwise the entry matched on its name alone: if it names an @@ -1824,8 +1843,8 @@ func (s *Service) findEntryTOMLBytes(raw []byte) (loc entryLocation, found, pars var nameOnly *entryLocation - for name, v := range serversMap { - entry, ok := v.(map[string]interface{}) + for _, name := range entryScanOrder(serversMap) { + entry, ok := serversMap[name].(map[string]interface{}) if !ok { continue } diff --git a/internal/connect/entry_lookup_order_test.go b/internal/connect/entry_lookup_order_test.go new file mode 100644 index 000000000..c393712b0 --- /dev/null +++ b/internal/connect/entry_lookup_order_test.go @@ -0,0 +1,49 @@ +package connect + +import "testing" + +// Two entries pointing at this instance (a legacy ?apikey= one under a custom +// key beside the current "mcpproxy" one) must resolve to the same entry on +// every lookup, preferring the default key. +func TestFindEntry_TwoEntriesPointingHere_Deterministic(t *testing.T) { + svc := NewServiceWithHome("127.0.0.1:8080", "", t.TempDir()) + + jsonRaw := []byte(`{"mcpServers": { + "mcpproxy": {"url": "http://127.0.0.1:8080/mcp"}, + "old-proxy": {"url": "http://127.0.0.1:8080/mcp?apikey=OLD"}, + "another": {"url": "http://127.0.0.1:8080/mcp?apikey=OTHER"} + }}`) + tomlRaw := []byte(` +[mcp_servers.mcpproxy] +url = "http://127.0.0.1:8080/mcp" + +[mcp_servers.old-proxy] +url = "http://127.0.0.1:8080/mcp?apikey=OLD" + +[mcp_servers.another] +url = "http://127.0.0.1:8080/mcp?apikey=OTHER" +`) + + cases := []struct { + client string + raw []byte + }{ + {"cursor", jsonRaw}, + {"codex", tomlRaw}, + } + for _, tc := range cases { + client := FindClient(tc.client) + if client == nil { + t.Fatalf("client %q not found", tc.client) + } + for i := 0; i < 50; i++ { + loc, found, parsedOK := svc.findEntryFromBytes(*client, tc.raw) + if !parsedOK || !found { + t.Fatalf("%s: found=%v parsedOK=%v", tc.client, found, parsedOK) + } + if loc.Name != defaultServerName || !loc.PointsHere { + t.Fatalf("%s lookup %d: got entry %q (pointsHere=%v), want %q", tc.client, i, loc.Name, loc.PointsHere, defaultServerName) + } + } + } +}