Skip to content

Commit d4f736e

Browse files
authored
feat(registry): add remove path for user-added custom registries (#592)
Custom registry sources added via `registry add-source` could not be removed through any supported surface — the only recourse was hand-editing config.db and restarting the daemon. Add the inverse remove operation: - DELETE /api/v1/registries/{id} — removes a custom/unverified source, refusing built-ins via the same registry_shadows_builtin guard and returning registry_not_found (404) for unknown ids. Persisted copy-on-write like add-source. - `mcpproxy registry remove <id>` CLI (aliases: rm, remove-source), daemon-required, with remove-specific error guidance. - cliclient.RemoveRegistrySource wrapping the DELETE endpoint. - Shared server-side derivation (removeRegistrySourceFromConfig) so CLI and REST produce identical persisted config. Docs (registries.md, rest-api.md) and OpenAPI spec updated in the same PR. The Web UI affordance is tracked as a separate frontend follow-up. Related #MCP-1057
1 parent b250a1c commit d4f736e

14 files changed

Lines changed: 562 additions & 6 deletions

cmd/mcpproxy/registry_cmd.go

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,88 @@ registries directly.`,
6565
}
6666

6767
cmd.PersistentFlags().StringVarP(&registryConfigPath, "config", "c", "", "Path to MCP configuration file")
68-
cmd.AddCommand(newRegistryListCmd(), newRegistrySearchCmd(), newRegistryAddCmd(), newRegistryAddSourceCmd())
68+
cmd.AddCommand(newRegistryListCmd(), newRegistrySearchCmd(), newRegistryAddCmd(), newRegistryAddSourceCmd(), newRegistryRemoveCmd())
6969
return cmd
7070
}
7171

72+
func newRegistryRemoveCmd() *cobra.Command {
73+
cmd := &cobra.Command{
74+
Use: "remove <id>",
75+
Aliases: []string{"remove-source", "rm"},
76+
Short: "Remove a user-added custom registry source",
77+
Long: `Remove a custom MCP registry source you previously added with
78+
'registry add-source'. Use 'registry list' to see the ids.
79+
80+
Only custom/unverified registries can be removed — the shipped built-in
81+
registries (official, reference, docker-mcp-catalog, pulse, smithery) cannot be
82+
removed. Removing a source does not touch any upstream servers you already added
83+
from it.`,
84+
Args: cobra.ExactArgs(1),
85+
RunE: func(_ *cobra.Command, args []string) error {
86+
registryID := args[0]
87+
88+
cfg, err := loadRegistryConfig()
89+
if err != nil {
90+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeConfigNotFound, err.Error()).
91+
WithRecoveryCommand("mcpproxy doctor"), clioutput.ErrCodeConfigNotFound)
92+
}
93+
94+
// remove MUST go through the daemon: the registry list lives on the
95+
// runtime config snapshot and is updated copy-on-write via UpdateConfig.
96+
if !shouldUseUpstreamDaemon(cfg.DataDir) {
97+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeConnectionFailed,
98+
"removing a registry source requires a running mcpproxy daemon").
99+
WithGuidance("Start the daemon, then retry").
100+
WithRecoveryCommand("mcpproxy serve"), clioutput.ErrCodeConnectionFailed)
101+
}
102+
103+
ctx, cancel := registryContext()
104+
defer cancel()
105+
106+
client := cliclient.NewClient(socket.DetectSocketPath(cfg.DataDir), nil)
107+
reg, err := client.RemoveRegistrySource(ctx, registryID)
108+
if err != nil {
109+
return registryRemoveErrorOutput(err)
110+
}
111+
112+
outputFormat := ResolveOutputFormat()
113+
if outputFormat == "json" || outputFormat == "yaml" {
114+
formatter, _ := GetOutputFormatter()
115+
out, _ := formatter.Format(reg)
116+
fmt.Println(out)
117+
return nil
118+
}
119+
120+
fmt.Printf("✅ Removed registry source '%s'\n", reg.ID)
121+
return nil
122+
},
123+
}
124+
return cmd
125+
}
126+
127+
// registryRemoveErrorOutput maps a *cliclient.RegistryAddError from a remove op
128+
// to a structured CLI error with remove-specific guidance (MCP-1057).
129+
func registryRemoveErrorOutput(err error) error {
130+
var addErr *cliclient.RegistryAddError
131+
if !errors.As(err, &addErr) {
132+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeOperationFailed, err.Error()), clioutput.ErrCodeOperationFailed)
133+
}
134+
135+
switch addErr.Code {
136+
case "registry_not_found":
137+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeServerNotFound, addErr.Message).
138+
WithGuidance("Check the ids with 'mcpproxy registry list' — only custom/unverified registries can be removed"), clioutput.ErrCodeServerNotFound)
139+
case "registry_shadows_builtin":
140+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeInvalidInput, addErr.Message).
141+
WithGuidance("Built-in registries cannot be removed"), clioutput.ErrCodeInvalidInput)
142+
case "registries_locked":
143+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeOperationFailed, addErr.Message).
144+
WithGuidance("Registry changes are disabled by policy (registries_locked)"), clioutput.ErrCodeOperationFailed)
145+
default:
146+
return outputError(clioutput.NewStructuredError(clioutput.ErrCodeOperationFailed, addErr.Message), clioutput.ErrCodeOperationFailed)
147+
}
148+
}
149+
72150
func newRegistryAddSourceCmd() *cobra.Command {
73151
cmd := &cobra.Command{
74152
Use: "add-source <https-url>",

docs/api/rest-api.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,21 @@ for the full feature guide (CLI, REST, MCP).
545545

546546
List configured registries.
547547

548+
#### POST /api/v1/registries
549+
550+
Add a user-supplied custom registry source. JSON body:
551+
`{ "url": "https://…", "protocol": "…", "id": "…", "name": "…" }` (only `url`
552+
required). The source is always tagged `custom/unverified`. Errors share a stable
553+
code: `invalid_registry_url` (400), `registries_locked` (403),
554+
`registry_shadows_builtin` / `duplicate_registry` (409).
555+
556+
#### DELETE /api/v1/registries/{id}
557+
558+
Remove a user-added custom registry source. Returns `data.registry` echoing the
559+
removed entry. Built-in registries are refused with `registry_shadows_builtin`
560+
(409); an unknown id returns `registry_not_found` (404); a `registries_locked`
561+
policy returns 403.
562+
548563
#### GET /api/v1/registries/{id}/servers
549564

550565
Search a registry's servers (`?search=`, `?tag=`, `?limit=`).

docs/registries.md

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,36 @@ Errors share a stable code across surfaces: `invalid_registry_url` (400),
8585
`registries_locked` (403), `registry_shadows_builtin` / `duplicate_registry` (409).
8686
The Web UI maps each code to an actionable message.
8787

88+
### Removing a registry source
89+
90+
`mcpproxy registry remove <id>` deletes a custom registry you added earlier. Only
91+
`custom/unverified` registries can be removed — the shipped built-in defaults are
92+
refused via the same shadow guard as add-source. Removing a source does not touch
93+
any upstream servers you already added from it.
94+
95+
```bash
96+
mcpproxy registry list # find the id
97+
mcpproxy registry remove acme # delete the custom source (aliases: rm, remove-source)
98+
```
99+
100+
Like add-source, this requires a running daemon — the change is applied
101+
copy-on-write on the runtime config snapshot and persisted to `mcp_config.json`.
102+
103+
Equivalent surfaces:
104+
105+
- **REST:** `DELETE /api/v1/registries/{id}``{ "registry": { … } }` echoing the removed entry.
106+
- **CLI:** `mcpproxy registry remove <id>`.
107+
108+
Errors share a stable code across surfaces: `registry_not_found` (404),
109+
`registry_shadows_builtin` (409, built-in cannot be removed),
110+
`registries_locked` (403).
111+
88112
### Enterprise: `registries_locked` (stub)
89113

90114
Setting `"registries_locked": true` in `mcp_config.json` disables runtime registry
91-
additions (`registry add-source` and the REST/MCP add-source surface return
92-
`registries_locked`). Built-in defaults are unaffected. This is a forward-looking
93-
stub for enterprise policy pinning.
115+
changes (`registry add-source` / `registry remove` and the REST add-source and
116+
remove surfaces return `registries_locked`). Built-in defaults are unaffected.
117+
This is a forward-looking stub for enterprise policy pinning.
94118

95119
## Official v0.1 protocol
96120

internal/cliclient/client.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,3 +1915,50 @@ func (c *Client) AddRegistrySource(ctx context.Context, sourceURL, protocol, id,
19151915
}
19161916
return &apiResp.Data.Registry, nil
19171917
}
1918+
1919+
// RemoveRegistrySource removes a user-added custom registry source via the daemon
1920+
// (MCP-1057). DELETE /api/v1/registries/{id} → data.registry. On failure it
1921+
// returns a *RegistryAddError carrying the stable cross-surface code
1922+
// (registry_not_found, registry_shadows_builtin, registries_locked).
1923+
func (c *Client) RemoveRegistrySource(ctx context.Context, id string) (*contracts.RegistrySummary, error) {
1924+
u := c.baseURL + "/api/v1/registries/" + url.PathEscape(id)
1925+
req, err := http.NewRequestWithContext(ctx, http.MethodDelete, u, http.NoBody)
1926+
if err != nil {
1927+
return nil, fmt.Errorf("failed to create request: %w", err)
1928+
}
1929+
c.prepareRequest(ctx, req)
1930+
1931+
resp, err := c.httpClient.Do(req)
1932+
if err != nil {
1933+
return nil, fmt.Errorf("failed to call remove-registry-source API: %w", err)
1934+
}
1935+
defer resp.Body.Close()
1936+
1937+
respBytes, err := io.ReadAll(resp.Body)
1938+
if err != nil {
1939+
return nil, fmt.Errorf("failed to read response: %w", err)
1940+
}
1941+
1942+
var apiResp struct {
1943+
Success bool `json:"success"`
1944+
Data *contracts.RemoveRegistrySourceData `json:"data"`
1945+
Error string `json:"error"`
1946+
Code string `json:"code"`
1947+
RequestID string `json:"request_id"`
1948+
}
1949+
if err := json.Unmarshal(respBytes, &apiResp); err != nil {
1950+
return nil, fmt.Errorf("failed to parse response (status %d): %s", resp.StatusCode, string(respBytes))
1951+
}
1952+
1953+
if !apiResp.Success || resp.StatusCode != http.StatusOK {
1954+
msg := apiResp.Error
1955+
if msg == "" {
1956+
msg = fmt.Sprintf("API returned status %d", resp.StatusCode)
1957+
}
1958+
return nil, &RegistryAddError{Code: apiResp.Code, Message: msg, RequestID: apiResp.RequestID}
1959+
}
1960+
if apiResp.Data == nil {
1961+
return nil, fmt.Errorf("daemon returned success with no registry data")
1962+
}
1963+
return &apiResp.Data.Registry, nil
1964+
}

internal/contracts/types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,12 @@ type AddRegistrySourceData struct {
10071007
Registry RegistrySummary `json:"registry"`
10081008
}
10091009

1010+
// RemoveRegistrySourceData is the success `data` payload for remove-source
1011+
// (MCP-1057, DELETE /api/v1/registries/{id}). It echoes the removed registry.
1012+
type RemoveRegistrySourceData struct {
1013+
Registry RegistrySummary `json:"registry"`
1014+
}
1015+
10101016
// SuccessResponse is the standard success response wrapper for API endpoints.
10111017
type SuccessResponse struct {
10121018
Success bool `json:"success"`

internal/httpapi/code_exec_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ func (m *mockController) AddServerFromRegistryRef(_ context.Context, _, _, _ str
100100
func (m *mockController) AddRegistrySourceRef(_, _, _, _ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
101101
return nil, nil, nil
102102
}
103+
func (m *mockController) RemoveRegistrySourceRef(_ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
104+
return nil, nil, nil
105+
}
103106
func (m *mockController) GetManagementService() interface{} { return nil }
104107
func (m *mockController) GetRuntime() interface{} { return nil }
105108
func (m *mockController) GetSessions(limit, offset int) (interface{}, int, error) { return nil, 0, nil }

internal/httpapi/contracts_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,9 @@ func (m *MockServerController) AddServerFromRegistryRef(_ context.Context, _, _,
318318
func (m *MockServerController) AddRegistrySourceRef(_, _, _, _ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
319319
return nil, nil, nil
320320
}
321+
func (m *MockServerController) RemoveRegistrySourceRef(_ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
322+
return nil, nil, nil
323+
}
321324

322325
// Version and updates
323326
func (m *MockServerController) GetVersionInfo() *updatecheck.VersionInfo {

internal/httpapi/registry_resilience_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package httpapi
22

33
import (
44
"encoding/json"
5+
"errors"
56
"net/http"
67
"net/http/httptest"
78
"testing"
89

10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
911
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
1012
"github.com/smart-mcp-proxy/mcpproxy-go/internal/registries"
1113
"go.uber.org/zap/zaptest"
@@ -199,3 +201,72 @@ func TestListRegistries_SurfacesProvenanceAndTrusted(t *testing.T) {
199201
t.Error("no-provenance-reg trusted: want false, got true")
200202
}
201203
}
204+
205+
// removeController simulates the server-side remove-source op: it removes the
206+
// custom "acme" registry, refuses the built-in "official", and reports
207+
// registry_not_found for anything else (MCP-1057).
208+
type removeController struct {
209+
*MockServerController
210+
}
211+
212+
func (c *removeController) RemoveRegistrySourceRef(id string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
213+
switch id {
214+
case "acme":
215+
return &config.RegistryEntry{
216+
ID: "acme",
217+
Name: "Acme",
218+
URL: "https://acme.example/",
219+
Provenance: config.RegistryProvenanceCustom,
220+
}, nil, nil
221+
case "official":
222+
rerr := &contracts.RegistryAddError{Code: "registry_shadows_builtin", Message: `"official" is a built-in registry and cannot be removed`}
223+
return nil, rerr, errors.New(rerr.Message)
224+
default:
225+
rerr := &contracts.RegistryAddError{Code: "registry_not_found", Message: "no custom registry with id " + id}
226+
return nil, rerr, errors.New(rerr.Message)
227+
}
228+
}
229+
230+
// MCP-1057: DELETE removes a custom registry and echoes it with trusted=false.
231+
func TestRemoveRegistrySource_RemovesCustom(t *testing.T) {
232+
srv := NewServer(&removeController{&MockServerController{}}, zaptest.NewLogger(t).Sugar(), nil)
233+
req := httptest.NewRequest(http.MethodDelete, "/api/v1/registries/acme", http.NoBody)
234+
w := httptest.NewRecorder()
235+
srv.ServeHTTP(w, req)
236+
237+
if w.Code != http.StatusOK {
238+
t.Fatalf("want 200, got %d (body=%s)", w.Code, w.Body.String())
239+
}
240+
var resp contracts.RemoveRegistrySourceData
241+
decodeData(t, w, &resp)
242+
if resp.Registry.ID != "acme" {
243+
t.Errorf("expected removed id=acme, got %q", resp.Registry.ID)
244+
}
245+
if resp.Registry.Trusted {
246+
t.Error("a custom registry must report trusted=false")
247+
}
248+
}
249+
250+
// MCP-1057: removing a built-in is refused with 409 registry_shadows_builtin.
251+
func TestRemoveRegistrySource_RefusesBuiltin(t *testing.T) {
252+
srv := NewServer(&removeController{&MockServerController{}}, zaptest.NewLogger(t).Sugar(), nil)
253+
req := httptest.NewRequest(http.MethodDelete, "/api/v1/registries/official", http.NoBody)
254+
w := httptest.NewRecorder()
255+
srv.ServeHTTP(w, req)
256+
257+
if w.Code != http.StatusConflict {
258+
t.Fatalf("want 409, got %d (body=%s)", w.Code, w.Body.String())
259+
}
260+
}
261+
262+
// MCP-1057: removing an unknown registry yields 404 registry_not_found.
263+
func TestRemoveRegistrySource_NotFound(t *testing.T) {
264+
srv := NewServer(&removeController{&MockServerController{}}, zaptest.NewLogger(t).Sugar(), nil)
265+
req := httptest.NewRequest(http.MethodDelete, "/api/v1/registries/ghost", http.NoBody)
266+
w := httptest.NewRecorder()
267+
srv.ServeHTTP(w, req)
268+
269+
if w.Code != http.StatusNotFound {
270+
t.Fatalf("want 404, got %d (body=%s)", w.Code, w.Body.String())
271+
}
272+
}

internal/httpapi/security_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,9 @@ func (m *baseController) AddServerFromRegistryRef(_ context.Context, _, _, _ str
300300
func (m *baseController) AddRegistrySourceRef(_, _, _, _ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
301301
return nil, nil, nil
302302
}
303+
func (m *baseController) RemoveRegistrySourceRef(_ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
304+
return nil, nil, nil
305+
}
303306
func (m *baseController) CallTool(ctx context.Context, toolName string, args map[string]interface{}) (interface{}, error) {
304307
return nil, nil
305308
}

0 commit comments

Comments
 (0)