Skip to content

Commit f952885

Browse files
claudeclaude
committed
feat(headers): unify delete syntax to JSON Merge Patch + add CLI patch
User feedback on the previous round: REST PATCH had two ways to delete a key (`headers_remove: ["X"]` array vs MCP tool's `{"X": null}`). The MCP tool's syntax is cleaner and is a documented standard (RFC 7396). Unify on it and add a CLI command so all three interfaces — MCP, REST, CLI — behave the same way. Backend (internal/httpapi/server.go): - AddServerRequest.Headers and .Env switch from `map[string]string` to `map[string]*string`. Go's encoding/json then maps a missing key to "no entry", a present non-null value to a non-nil `*string`, and a present `null` to a nil pointer. The merge loop reads each entry: nil pointer = delete, non-nil = upsert. - Drop the `headers_remove` / `env_remove` array fields. A single `null` in the same map carries the same intent and aligns with the MCP tool. - POST (add) ignores nil entries via the new flattenNullableMap helper; `null` on create has no meaning. - redactServerHeaders / SSE redaction unchanged. Tests (internal/httpapi/patch_server_test.go): - Rewrite the previous `*_remove`-style tests to use literal JSON null payloads via `json.RawMessage`. The raw-byte approach is independent of any Go marshaling quirks that could collapse `null` values. - New TestHandlePatchServer_HeadersEmptyStringSetsNotDeletes pins the distinction between `""` (set to empty) and `null` (delete) — JSON Merge Patch is explicit about it and a future refactor that "helpfully" collapses one to the other would silently break. - Total: 7 tests, all green. Web UI (frontend/src/views/ServerDetail.vue): - deleteKv now sends `{headers: {key: null}}` instead of the array form. JSON.stringify emits `null` literally, no special handling. - Drop the `scopeRemoveKey` helper (no longer needed). macOS Swift (native/macos/MCPProxy/MCPProxy/Views/ServerDetailView.swift): - diffKVMap returns a single `[String: Any]` patch dict where deleted keys map to `NSNull()` instead of returning the previous `(set, remove)` tuple. - saveEdits writes the patch as `updates["headers"] = patch` directly; no `headers_remove` companion field anymore. - performConvertToSecret sends a single-key patch `{field: {key: ref}}` instead of building the full map — minimal wire payload, never round-trips the redacted Authorization. - The trap was real and surprising: Swift's default `JSONEncoder` on `[String: String?]` SILENTLY DROPS nil entries from the JSON output. Using `[String: Any]` with `NSNull()` + `JSONSerialization` (the encoder our APIClient already uses) renders `null` correctly. Swift unit test (native/macos/MCPProxy/MCPProxyTests/MergePatchEncodingTests.swift): - 4 tests pinning the encoding contract: 1. NSNull encodes as literal `null` via JSONSerialization. 2. A delete-only patch round-trips through JSON and the value parses back as NSNull (not "", not absent). 3. The wrong path — `[String: String?]` + default `JSONEncoder` — does silently drop nils. Documented as a poison-pill test so a future refactor that "simplifies" to it has to explicitly delete this test and read the comment first. 4. Empty string still encodes as `""` and explicit null as `null` — the JSON Merge Patch set-vs-delete distinction is preserved. CLI (cmd/mcpproxy/upstream_cmd.go + internal/cliclient/client.go): - New `mcpproxy upstream patch <name>` subcommand with flags: --header K=V upsert (repeatable) --header-remove K delete (repeatable) --env K=V upsert (repeatable) --env-remove K delete (repeatable) - New cliclient.PatchServer(name, body) sends raw JSON to PATCH /api/v1/servers/{name}. Body shape is the same JSON Merge Patch the Web UI and macOS tray send. - Closes the CLI gap I called out in the boundaries-matrix summary — REST, MCP, and CLI now all support both write and delete on headers / env with the same semantics. Live end-to-end verification: $ mcpproxy upstream patch synapbus --header "X-Cli-Test: hello-from-cli" ✅ Patched synapbus: 1 header(s) set → on disk: { Authorization (real Bearer), X-Cli-Test } (Auth preserved) $ mcpproxy upstream patch synapbus --header-remove X-Cli-Test ✅ Patched synapbus: 1 header(s) removed → on disk: { Authorization (real Bearer) } (Auth preserved) $ mcpproxy upstream patch synapbus --header "X-Foo: v" --header-remove X-Foo Error: --header and --header-remove for "X-Foo" conflict; pick one Web UI: "+ Add header" → X-WebUI-Test=from-browser → Save → on disk: { Authorization (real Bearer), X-WebUI-Test } macOS tray: Edit → textarea pre-populated with "Authorization=***REDACTED***" → user appends "X-Mac-Test=hello-from-mac" → Save → on disk: { Authorization (REAL Bearer, preserved!), X-Mac-Test } PR #463 subagent review confirmed the unrelated "disable tool" pattern is a different domain (reversible state in BBolt vs destructive mutation in mcp_config.json) and should not be unified with this work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c23e038 commit f952885

9 files changed

Lines changed: 500 additions & 142 deletions

File tree

‎cmd/mcpproxy/upstream_cmd.go‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,39 @@ Examples:
121121
RunE: runUpstreamAddJSON,
122122
}
123123

124+
upstreamPatchCmd = &cobra.Command{
125+
Use: "patch <name>",
126+
Short: "Update headers / env on an existing upstream server",
127+
Long: `Update HTTP headers and stdio environment variables on an existing upstream server.
128+
129+
The PATCH endpoint uses JSON Merge Patch semantics: keys you specify are
130+
upserted, keys you delete with -remove flags are explicitly removed, and
131+
every other key on the stored config is preserved. So you can safely
132+
rotate a single header without seeing or touching the rest — including
133+
sensitive values the backend redacts from list / inspect responses.
134+
135+
Examples:
136+
# rotate the Authorization header on the synapbus server
137+
mcpproxy upstream patch synapbus --header "Authorization: Bearer new-token"
138+
139+
# add a custom header without disturbing existing ones
140+
mcpproxy upstream patch synapbus --header "X-Trace: on"
141+
142+
# remove a header
143+
mcpproxy upstream patch synapbus --header-remove "X-Stale"
144+
145+
# set + remove in a single round-trip
146+
mcpproxy upstream patch synapbus --header "X-New: v" --header-remove "X-Old"
147+
148+
# update env vars on a stdio server
149+
mcpproxy upstream patch obsidian-pilot --env "LOG_LEVEL=debug" --env-remove "OLD_VAR"
150+
151+
Flags are repeatable. The corresponding null in the JSON Merge Patch body
152+
is constructed automatically — you never have to think about wire format.`,
153+
Args: cobra.ExactArgs(1),
154+
RunE: runUpstreamPatch,
155+
}
156+
124157
upstreamInspectCmd = &cobra.Command{
125158
Use: "inspect <server-name>",
126159
Short: "Inspect tool approval status for a server",
@@ -248,6 +281,12 @@ Examples:
248281
// Inspect command flags
249282
upstreamInspectTool string
250283

284+
// Patch command flags
285+
upstreamPatchHeaders []string
286+
upstreamPatchHeaderRemove []string
287+
upstreamPatchEnvs []string
288+
upstreamPatchEnvRemove []string
289+
251290
// Import command flags
252291
upstreamImportServer string
253292
upstreamImportFormat string
@@ -272,6 +311,7 @@ func init() {
272311
upstreamCmd.AddCommand(upstreamAddCmd)
273312
upstreamCmd.AddCommand(upstreamRemoveCmd)
274313
upstreamCmd.AddCommand(upstreamAddJSONCmd)
314+
upstreamCmd.AddCommand(upstreamPatchCmd)
275315
upstreamCmd.AddCommand(upstreamInspectCmd)
276316
upstreamCmd.AddCommand(upstreamApproveCmd)
277317
upstreamCmd.AddCommand(upstreamImportCmd)
@@ -316,6 +356,12 @@ func init() {
316356
upstreamRemoveCmd.Flags().BoolVarP(&upstreamRemoveYes, "y", "y", false, "Skip confirmation prompt (short form)")
317357
upstreamRemoveCmd.Flags().BoolVar(&upstreamRemoveIfExists, "if-exists", false, "Don't error if server doesn't exist")
318358

359+
// Patch command flags
360+
upstreamPatchCmd.Flags().StringArrayVar(&upstreamPatchHeaders, "header", nil, "HTTP header to upsert in 'Name: value' format (repeatable)")
361+
upstreamPatchCmd.Flags().StringArrayVar(&upstreamPatchHeaderRemove, "header-remove", nil, "HTTP header name to delete (repeatable)")
362+
upstreamPatchCmd.Flags().StringArrayVar(&upstreamPatchEnvs, "env", nil, "Environment variable to upsert in KEY=value format (repeatable)")
363+
upstreamPatchCmd.Flags().StringArrayVar(&upstreamPatchEnvRemove, "env-remove", nil, "Environment variable name to delete (repeatable)")
364+
319365
// Inspect command flags
320366
upstreamInspectCmd.Flags().StringVar(&upstreamInspectTool, "tool", "", "Show details for a specific tool")
321367

@@ -2006,3 +2052,125 @@ func applyImportedServersConfigMode(imported []*configimport.ImportedServer, glo
20062052

20072053
return nil
20082054
}
2055+
2056+
// runUpstreamPatch handles the 'upstream patch' command. Translates the
2057+
// repeatable --header / --env / --header-remove / --env-remove flags into
2058+
// a JSON Merge Patch (RFC 7396) body and POSTs it to PATCH /api/v1/servers/{name}.
2059+
//
2060+
// Upserts encode as {"headers": {"X-Foo": "bar"}}; deletes encode as
2061+
// {"headers": {"X-Stale": null}}. Both shapes go through encoding/json
2062+
// (`map[string]*string`) where a nil pointer renders as the literal
2063+
// `null` token — verified by the backend's PATCH tests.
2064+
func runUpstreamPatch(_ *cobra.Command, args []string) error {
2065+
serverName := strings.TrimSpace(args[0])
2066+
if serverName == "" {
2067+
return fmt.Errorf("server name is required")
2068+
}
2069+
2070+
if len(upstreamPatchHeaders) == 0 && len(upstreamPatchHeaderRemove) == 0 &&
2071+
len(upstreamPatchEnvs) == 0 && len(upstreamPatchEnvRemove) == 0 {
2072+
return fmt.Errorf("at least one of --header / --header-remove / --env / --env-remove must be specified")
2073+
}
2074+
2075+
headers := map[string]*string{}
2076+
for _, h := range upstreamPatchHeaders {
2077+
parts := strings.SplitN(h, ":", 2)
2078+
if len(parts) != 2 {
2079+
return fmt.Errorf("invalid --header format: %q (expected 'Name: value')", h)
2080+
}
2081+
key := strings.TrimSpace(parts[0])
2082+
val := strings.TrimSpace(parts[1])
2083+
if key == "" {
2084+
return fmt.Errorf("invalid --header: empty header name in %q", h)
2085+
}
2086+
headers[key] = &val
2087+
}
2088+
for _, k := range upstreamPatchHeaderRemove {
2089+
key := strings.TrimSpace(k)
2090+
if key == "" {
2091+
return fmt.Errorf("invalid --header-remove: empty name")
2092+
}
2093+
if _, dupe := headers[key]; dupe {
2094+
return fmt.Errorf("--header and --header-remove for %q conflict; pick one", key)
2095+
}
2096+
headers[key] = nil // JSON Merge Patch: null = delete
2097+
}
2098+
2099+
envs := map[string]*string{}
2100+
for _, e := range upstreamPatchEnvs {
2101+
parts := strings.SplitN(e, "=", 2)
2102+
if len(parts) != 2 {
2103+
return fmt.Errorf("invalid --env format: %q (expected 'KEY=value')", e)
2104+
}
2105+
key := strings.TrimSpace(parts[0])
2106+
val := parts[1]
2107+
if key == "" {
2108+
return fmt.Errorf("invalid --env: empty key in %q", e)
2109+
}
2110+
envs[key] = &val
2111+
}
2112+
for _, k := range upstreamPatchEnvRemove {
2113+
key := strings.TrimSpace(k)
2114+
if key == "" {
2115+
return fmt.Errorf("invalid --env-remove: empty name")
2116+
}
2117+
if _, dupe := envs[key]; dupe {
2118+
return fmt.Errorf("--env and --env-remove for %q conflict; pick one", key)
2119+
}
2120+
envs[key] = nil
2121+
}
2122+
2123+
body := map[string]interface{}{}
2124+
if len(headers) > 0 {
2125+
body["headers"] = headers
2126+
}
2127+
if len(envs) > 0 {
2128+
body["env"] = envs
2129+
}
2130+
2131+
bodyBytes, err := json.Marshal(body)
2132+
if err != nil {
2133+
return fmt.Errorf("failed to marshal patch body: %w", err)
2134+
}
2135+
2136+
ctx, cancel := context.WithTimeout(reqcontext.WithMetadata(context.Background(), reqcontext.SourceCLI), 15*time.Second)
2137+
defer cancel()
2138+
2139+
globalConfig, err := loadUpstreamConfig()
2140+
if err != nil {
2141+
return fmt.Errorf("failed to load config: %w", err)
2142+
}
2143+
if !shouldUseUpstreamDaemon(globalConfig.DataDir) {
2144+
return fmt.Errorf("mcpproxy daemon is not running — start it with `mcpproxy serve` first.\n\nThe `patch` subcommand requires a live backend so configuration changes are\napplied with full deep-merge semantics and propagated to running upstream\nconnections immediately. Editing the config file by hand only works while\nthe daemon is offline.")
2145+
}
2146+
logger, err := createUpstreamLogger("warn")
2147+
if err != nil {
2148+
return fmt.Errorf("failed to create logger: %w", err)
2149+
}
2150+
socketPath := socket.DetectSocketPath(globalConfig.DataDir)
2151+
client := cliclient.NewClient(socketPath, logger.Sugar())
2152+
2153+
if err := client.PatchServer(ctx, serverName, bodyBytes); err != nil {
2154+
return err
2155+
}
2156+
2157+
fmt.Printf("✅ Patched %s", serverName)
2158+
parts := []string{}
2159+
if n := len(upstreamPatchHeaders); n > 0 {
2160+
parts = append(parts, fmt.Sprintf("%d header(s) set", n))
2161+
}
2162+
if n := len(upstreamPatchHeaderRemove); n > 0 {
2163+
parts = append(parts, fmt.Sprintf("%d header(s) removed", n))
2164+
}
2165+
if n := len(upstreamPatchEnvs); n > 0 {
2166+
parts = append(parts, fmt.Sprintf("%d env var(s) set", n))
2167+
}
2168+
if n := len(upstreamPatchEnvRemove); n > 0 {
2169+
parts = append(parts, fmt.Sprintf("%d env var(s) removed", n))
2170+
}
2171+
if len(parts) > 0 {
2172+
fmt.Printf(": %s", strings.Join(parts, ", "))
2173+
}
2174+
fmt.Println()
2175+
return nil
2176+
}

‎frontend/src/views/ServerDetail.vue‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,18 +2395,22 @@ async function patchServerDiff(patch: Record<string, unknown>, action: string):
23952395
function scopeKey(scope: 'header' | 'env'): 'headers' | 'env' {
23962396
return scope === 'header' ? 'headers' : 'env'
23972397
}
2398-
function scopeRemoveKey(scope: 'header' | 'env'): 'headers_remove' | 'env_remove' {
2399-
return scope === 'header' ? 'headers_remove' : 'env_remove'
2400-
}
24012398
24022399
async function saveEdit(scope: 'header' | 'env', k: string, val: string) {
24032400
const ok = await patchServerDiff({ [scopeKey(scope)]: { [k]: val } }, `Updated ${k}`)
24042401
if (ok) editingKey.value = null
24052402
}
24062403
2404+
// Deletion uses JSON Merge Patch (RFC 7396): a null value on the key
2405+
// signals "delete this key" to the backend. JSON.stringify emits `null`
2406+
// as the literal token, so the patch body becomes `{"headers": {"X-Old":
2407+
// null}}` on the wire — symmetric with the MCP `upstream_servers patch`
2408+
// tool's `{"X-Old": null}` convention. Note the explicit `null` literal:
2409+
// passing `undefined` would be stripped by JSON.stringify (no key in the
2410+
// output) and the backend would interpret that as "preserve".
24072411
async function deleteKv(scope: 'header' | 'env', k: string) {
24082412
if (!confirm(`Delete ${scope === 'header' ? 'header' : 'env variable'} "${k}"?`)) return
2409-
await patchServerDiff({ [scopeRemoveKey(scope)]: [k] }, `Deleted ${k}`)
2413+
await patchServerDiff({ [scopeKey(scope)]: { [k]: null } }, `Deleted ${k}`)
24102414
}
24112415
24122416
async function commitNewHeader() {

‎internal/cliclient/client.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,62 @@ func (c *Client) AddServer(ctx context.Context, req *AddServerRequest) (*AddServ
12241224
return apiResp.Data, nil
12251225
}
12261226

1227+
// PatchServer issues a partial update against PATCH /api/v1/servers/{name}.
1228+
//
1229+
// The body is sent as raw JSON so callers can include JSON null values
1230+
// for header / env deletion under JSON Merge Patch semantics (RFC 7396).
1231+
// Building the body as `map[string]any{"X": nil}` would round-trip through
1232+
// encoding/json fine, but `map[string]*string` is cleaner for fixed-shape
1233+
// callers that want type safety — both forms are supported because
1234+
// callers pass already-marshaled bytes.
1235+
//
1236+
// Example body shapes:
1237+
//
1238+
// {"headers": {"X-New": "value"}} -- upsert
1239+
// {"headers": {"X-Stale": null}} -- delete
1240+
// {"headers": {"X-Set": "v", "X-Old": null}} -- combined
1241+
// {"env": {"API_KEY": null}, "enabled": true} -- env delete + enable
1242+
func (c *Client) PatchServer(ctx context.Context, serverName string, body []byte) error {
1243+
url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, serverName)
1244+
req, err := http.NewRequestWithContext(ctx, http.MethodPatch, url, bytes.NewReader(body))
1245+
if err != nil {
1246+
return fmt.Errorf("failed to create request: %w", err)
1247+
}
1248+
req.Header.Set("Content-Type", "application/json")
1249+
c.prepareRequest(ctx, req)
1250+
1251+
resp, err := c.httpClient.Do(req)
1252+
if err != nil {
1253+
return fmt.Errorf("failed to call patch API: %w", err)
1254+
}
1255+
defer resp.Body.Close()
1256+
1257+
respBytes, err := io.ReadAll(resp.Body)
1258+
if err != nil {
1259+
return fmt.Errorf("failed to read response: %w", err)
1260+
}
1261+
1262+
if resp.StatusCode == http.StatusNotFound {
1263+
return fmt.Errorf("server '%s' not found", serverName)
1264+
}
1265+
if resp.StatusCode != http.StatusOK {
1266+
return fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(respBytes))
1267+
}
1268+
1269+
var apiResp struct {
1270+
Success bool `json:"success"`
1271+
Error string `json:"error"`
1272+
RequestID string `json:"request_id"`
1273+
}
1274+
if err := json.Unmarshal(respBytes, &apiResp); err != nil {
1275+
return fmt.Errorf("failed to parse response: %w", err)
1276+
}
1277+
if !apiResp.Success {
1278+
return parseAPIError(apiResp.Error, apiResp.RequestID)
1279+
}
1280+
return nil
1281+
}
1282+
12271283
// RemoveServer removes an upstream server via the daemon API.
12281284
func (c *Client) RemoveServer(ctx context.Context, serverName string) error {
12291285
url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, serverName)

0 commit comments

Comments
 (0)