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 ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -1036,5 +1036,5 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—`
| [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) |
| [107-server-edition-sso-hardening](./specs/107-server-edition-sso-hardening/) | `shipped` | 126/126 (100%) |
| [108-profiles-v3](./specs/108-profiles-v3/) | `in-flight` | 23/153 (15%) |
| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `in-flight` | 35/180 (19%) |
| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `in-flight` | 36/180 (20%) |
| [110-catalog-popularity](./specs/110-catalog-popularity/) | `in-flight` | 19/23 (83%) |
59 changes: 40 additions & 19 deletions cmd/mcpproxy/catalog_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,7 @@ func newCatalogShowCmd() *cobra.Command {
}

hit := registries.BuildCatalogHit(reg, *entry)
result := registries.ToCatalogResult(hit, catalogAddedFromConfig(cfg)(hit))
return renderCatalogShow(formatter, result)
return renderCatalogShow(formatter, catalogResultForConfig(cfg, hit))
},
}
return cmd
Expand Down Expand Up @@ -273,33 +272,43 @@ func catalogSearchInProcess(ctx context.Context, cfg *config.Config, q, source,
// httpapi.handleCatalogSearch).
hits, sections, unavailable := registries.SearchAll(ctx, q, tag, limit, registries.SearchOptions{Source: source})

added := catalogAddedFromConfig(cfg)
resp := &cliclient.CatalogSearchResponse{Query: q, Unavailable: unavailable}
if resp.Unavailable == nil {
resp.Unavailable = []registries.SourceError{}
}
for _, h := range hits {
resp.Results = append(resp.Results, registries.ToCatalogResult(h, added(h)))
resp.Results = append(resp.Results, catalogResultForConfig(cfg, h))
}
if sections != nil {
resp.Sections = &cliclient.CatalogSections{}
for _, h := range sections.Official {
resp.Sections.Official = append(resp.Sections.Official, registries.ToCatalogResult(h, added(h)))
resp.Sections.Official = append(resp.Sections.Official, catalogResultForConfig(cfg, h))
}
for _, h := range sections.Popular {
resp.Sections.Popular = append(resp.Sections.Popular, registries.ToCatalogResult(h, added(h)))
resp.Sections.Popular = append(resp.Sections.Popular, catalogResultForConfig(cfg, h))
}
}
return resp, nil
}

// catalogAddedFromConfig returns a predicate reporting whether a catalog hit
// matches an already-configured server (contracts/rest-api.md#catalog "added"):
// a registry-sourced server also needs a matching source, a manual add
// matches on install target alone.
func catalogAddedFromConfig(cfg *config.Config) func(registries.CatalogHit) bool {
byRegistryAndTarget := make(map[string]bool)
byTargetOnly := make(map[string]bool)
// catalogResultForConfig builds a CatalogResult for hit, joining it against the
// loaded config the way REST does: Added, plus AddedServerName when exactly one
// configured server matches.
func catalogResultForConfig(cfg *config.Config, hit registries.CatalogHit) registries.CatalogResult {
isAdded, name := catalogAddedFromConfig(cfg)(hit)
result := registries.ToCatalogResult(hit, isAdded)
result.AddedServerName = name
return result
}

// catalogAddedFromConfig returns a resolver reporting whether a catalog hit
// matches an already-configured server (contracts/rest-api.md#catalog "added")
// and, only for a unique match, that server's name (mirrors
// httpapi.catalogAddedResolver): a registry-sourced server also needs a
// matching source, a manual add matches on install target alone.
func catalogAddedFromConfig(cfg *config.Config) func(registries.CatalogHit) (bool, string) {
byRegistryAndTarget := make(map[string][]string)
byTargetOnly := make(map[string][]string)
if cfg != nil {
for _, s := range cfg.Servers {
if s == nil {
Expand All @@ -308,21 +317,33 @@ func catalogAddedFromConfig(cfg *config.Config) func(registries.CatalogHit) bool
target := catalogInstallTargetForConfigServer(s)
if s.SourceRegistryID == "" {
// Manual add: matches any source by install target alone.
byTargetOnly[target] = true
byTargetOnly[target] = append(byTargetOnly[target], s.Name)
} else {
// Registry-sourced: must also match its own source, so it
// never falsely matches a different source's identical
// install target (contracts/rest-api.md#catalog "added").
byRegistryAndTarget[s.SourceRegistryID+"\x00"+target] = true
key := s.SourceRegistryID + "\x00" + target
byRegistryAndTarget[key] = append(byRegistryAndTarget[key], s.Name)
}
}
}
return func(h registries.CatalogHit) bool {
return func(h registries.CatalogHit) (bool, string) {
target := registries.CatalogInstallTarget(registries.ToCatalogResult(h, false).Install)
if byRegistryAndTarget[h.Source+"\x00"+target] {
return true
names := append([]string(nil), byRegistryAndTarget[h.Source+"\x00"+target]...)
names = append(names, byTargetOnly[target]...)
if len(names) == 0 {
return false, ""
}
// De-duplicate defensively so a malformed legacy configuration with a
// repeated name is not reported as ambiguous.
unique := make(map[string]struct{}, len(names))
for _, name := range names {
unique[name] = struct{}{}
}
if len(unique) == 1 {
return true, names[0]
}
return byTargetOnly[target]
return true, ""
}
}

Expand Down
73 changes: 64 additions & 9 deletions cmd/mcpproxy/catalog_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"bytes"
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -38,34 +39,73 @@ func TestParseCatalogRef_RejectsMissingSlash(t *testing.T) {

// TestCatalogAddedFromConfig pins the join rule (contracts/rest-api.md#catalog
// "added"): a registry-sourced configured server needs source AND install
// target; a manual add matches on install target alone.
// target; a manual add matches on install target alone. The resolver also
// reports the server name, but only for a unique match (REST parity).
func TestCatalogAddedFromConfig(t *testing.T) {
cfg := &config.Config{
Servers: []*config.ServerConfig{
{Name: "manual", URL: "https://manual.example.com/mcp"},
{Name: "from-official", Command: "npx", Args: []string{"server-x"}, SourceRegistryID: "official"},
{Name: "dup-a", Command: "npx", Args: []string{"server-dup"}},
{Name: "dup-b", Command: "npx", Args: []string{"server-dup"}},
},
}
added := catalogAddedFromConfig(cfg)

manualHit := registries.CatalogHit{Source: "smithery", Entry: registries.ServerEntry{ID: "m", URL: "https://manual.example.com/mcp"}}
if !added(manualHit) {
t.Error("expected a manual add to match by install target regardless of source")
if ok, name := added(manualHit); !ok || name != "manual" {
t.Errorf("expected a manual add to match by install target regardless of source, got (%v, %q)", ok, name)
}

officialHit := registries.CatalogHit{Source: "official", Entry: registries.ServerEntry{ID: "x", InstallCmd: "npx server-x"}}
if !added(officialHit) {
t.Error("expected the registry-sourced server to match its own source + target")
if ok, name := added(officialHit); !ok || name != "from-official" {
t.Errorf("expected the registry-sourced server to match its own source + target, got (%v, %q)", ok, name)
}

wrongSourceHit := registries.CatalogHit{Source: "smithery", Entry: registries.ServerEntry{ID: "x", InstallCmd: "npx server-x"}}
if added(wrongSourceHit) {
t.Error("a registry-sourced configured server must not match a different source with the same target")
if ok, name := added(wrongSourceHit); ok || name != "" {
t.Errorf("a registry-sourced configured server must not match a different source with the same target, got (%v, %q)", ok, name)
}

noMatchHit := registries.CatalogHit{Source: "official", Entry: registries.ServerEntry{ID: "y", InstallCmd: "npx server-y"}}
if added(noMatchHit) {
t.Error("expected no match for an unrelated entry")
if ok, name := added(noMatchHit); ok || name != "" {
t.Errorf("expected no match for an unrelated entry, got (%v, %q)", ok, name)
}

ambiguousHit := registries.CatalogHit{Source: "official", Entry: registries.ServerEntry{ID: "d", InstallCmd: "npx server-dup"}}
if ok, name := added(ambiguousHit); !ok || name != "" {
t.Errorf("expected added=true with no name when two servers match, got (%v, %q)", ok, name)
}
}

// TestCatalogSearchInProcess_AddedServerName pins CLI/REST parity: the offline
// search path emits added_server_name for a uniquely matched configured server.
func TestCatalogSearchInProcess_AddedServerName(t *testing.T) {
withCatalogCLIFixture(t, `[{"id":"gh","name":"GitHub Tool","description":"desc","url":"https://x.example.com/mcp"}]`)
cfg := &config.Config{Servers: []*config.ServerConfig{{Name: "my-gh", URL: "https://x.example.com/mcp"}}}

resp, err := catalogSearchInProcess(context.Background(), cfg, "GitHub", "", "", 10)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(resp.Results) != 1 {
t.Fatalf("expected 1 result, got %+v", resp.Results)
}
if !resp.Results[0].Added || resp.Results[0].AddedServerName != "my-gh" {
t.Errorf("expected added=true added_server_name=my-gh, got %+v", resp.Results[0])
}

empty, err := catalogSearchInProcess(context.Background(), cfg, "", "", "", 10)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if empty.Sections == nil {
t.Fatal("expected sections for an empty query")
}
for _, r := range append(append([]registries.CatalogResult{}, empty.Sections.Official...), empty.Sections.Popular...) {
if r.ID == "gh" && r.AddedServerName != "my-gh" {
t.Errorf("section entry missing added_server_name: %+v", r)
}
}
}

Expand Down Expand Up @@ -207,6 +247,21 @@ func TestCatalogShow(t *testing.T) {
}
}

// TestCatalogResultForConfig pins that 'catalog show' builds its result with
// the added_server_name join, like REST.
func TestCatalogResultForConfig(t *testing.T) {
cfg := &config.Config{Servers: []*config.ServerConfig{{Name: "my-gh", URL: "https://x.example.com/mcp"}}}
hit := registries.CatalogHit{Source: "official", Entry: registries.ServerEntry{ID: "gh", URL: "https://x.example.com/mcp"}}
result := catalogResultForConfig(cfg, hit)
if !result.Added || result.AddedServerName != "my-gh" {
t.Errorf("expected added=true added_server_name=my-gh, got %+v", result)
}
data, err := json.Marshal(result)
if err != nil || !strings.Contains(string(data), `"added_server_name":"my-gh"`) {
t.Errorf("expected added_server_name in JSON, got %s (err %v)", data, err)
}
}

// TestPrintCatalogDeprecationNotice pins FR-066: 'registry search'/'registry
// add' print a deprecation note pointing at the 'catalog' equivalent.
func TestPrintCatalogDeprecationNotice(t *testing.T) {
Expand Down
Loading
Loading