feat(api): expose & persist auto_approve_tool_changes in server REST API (MCP-2940)

MCP-2930 (#724) added the per-server auto_approve_tool_changes flag to
config.ServerConfig but did not extend the REST layer, so the Web UI toggle
(MCP-2932, PR #725) could neither read nor persist it.

REST request side:
- Add AutoApproveToolChanges *bool to httpapi.AddServerRequest.
- Wire it into handleAddServer (create) and handlePatchServer with *bool
  nil-preserve semantics: omitting it on PATCH preserves the existing pointer
  (which may be nil = "never set"); an explicit value (including false) is
  applied. server.UpdateServer applies the pointer only when non-nil so callers
  that don't touch it (e.g. config-to-secret) never reset it.

REST read side:
- Add AutoApproveToolChanges *bool to contracts.Server (omitempty → tri-state
  nil stays out of the payload so the UI can tell unset from explicit false).
- Emit auto_approve_tool_changes from runtime.GetAllServers (StateView path)
  and getAllServersLegacy, in parity with quarantined.
- Project it in management.ListServers and ConvertGenericServersToTyped /
  ConvertServerConfig.

Persistence:
- SaveConfiguration rebuilds the JSON config's server list from BBolt records,
  so a field absent from storage.UpstreamRecord is wiped on the next mutation.
  Add AutoApproveToolChanges to UpstreamRecord and all four selective
  conversions (Save/Get/List in manager.go, saveServerSync in async_ops.go);
  update TestSaveServerSyncFieldCoverage to require it in BBolt. Verified the
  flag now survives PATCH→GET and a full restart.

Tests: handler PATCH (set/preserve-nil/preserve-true/explicit-false) + GET
exposure; management + contracts projection; storage Save/Get/List round-trip
incl. tri-state nil. OpenAPI regenerated. Docs updated (tool-quarantine.md,
rest-api.md).

Related #725. Runtime enforcement remains MCP-2931.

Author: Dumbris

docs/api/rest-api.md

@@ -231,6 +231,7 @@ mask string.
   "protocol": "http",
   "enabled": true,
   "quarantined": false,
+  "auto_approve_tool_changes": true,
   "isolation": {"enabled": true, "image": "node:20"}
 }
 ```
@@ -258,8 +259,11 @@ curl -X PATCH -H "X-API-Key: $KEY" -H "Content-Type: application/json" \
 
 - Empty string `""` is **set-to-empty**, NOT delete. JSON Merge Patch is
   explicit about this — only the JSON `null` token deletes.
-- Boolean fields (`enabled`, `quarantined`, `reconnect_on_use`) use
-  pointer-style semantics: absent = preserve, present = explicit value.
+- Boolean fields (`enabled`, `quarantined`, `reconnect_on_use`,
+  `auto_approve_tool_changes`) use pointer-style semantics: absent = preserve,
+  present = explicit value. `auto_approve_tool_changes` is tri-state — it is
+  omitted entirely from `GET /api/v1/servers` responses when never set, so a
+  client can distinguish "unset" from an explicit `false`.
 
 #### POST /api/v1/servers/{name}/config-to-secret
 

docs/features/tool-quarantine.md

@@ -82,6 +82,8 @@ Trust specific servers by skipping per-server tool-change review:
 
 > **Note — rollout:** `auto_approve_tool_changes` is config-plumbed but its **enforcement is not yet wired** — runtime auto-approval is still governed by `skip_quarantine`. The new flag becomes the active control (and gains the richer rug-pull / trust-baseline behavior) in an upcoming release. Existing `skip_quarantine` configs are unaffected and are migrated onto the new key automatically. To auto-approve a server today, set `skip_quarantine: true`.
 
+> **REST API:** `auto_approve_tool_changes` round-trips through the server REST API. `GET /api/v1/servers` includes it on each server object (omitted when unset — the tri-state nil is preserved so the Web UI can tell "never set" from an explicit `false`), and `POST`/`PATCH /api/v1/servers/{id}` accept it. As a tri-state `*bool`, omitting it on `PATCH` leaves the stored value unchanged; sending an explicit `false` clears a prior `true`. The value is persisted to BBolt so it survives a save/restart.
+
 When the active control is enabled for a server, new tools from it are automatically approved.
 
 ### Auto-Approve Behavior

internal/contracts/converters.go

@@ -33,6 +33,9 @@ func ConvertServerConfig(cfg *config.ServerConfig, status string, connected bool
 		// can show a server's origin. Empty for manually-configured servers.
 		SourceRegistryID:         cfg.SourceRegistryID,
 		SourceRegistryProvenance: cfg.SourceRegistryProvenance,
+		// MCP-2940: surface the per-server auto-approve intent (tri-state *bool)
+		// so the Web UI toggle reflects the persisted value.
+		AutoApproveToolChanges: cfg.AutoApproveToolChanges,
 	}
 
 	// Convert OAuth config if present
@@ -177,6 +180,13 @@ func ConvertGenericServersToTyped(genericServers []map[string]interface{}) []Ser
 		if quarantined, ok := generic["quarantined"].(bool); ok {
 			server.Quarantined = quarantined
 		}
+		// MCP-2940: tri-state *bool — only set the pointer when the key is
+		// present so an unset flag stays nil (the Web UI distinguishes unset
+		// from an explicit false).
+		if autoApprove, ok := generic["auto_approve_tool_changes"].(bool); ok {
+			v := autoApprove
+			server.AutoApproveToolChanges = &v
+		}
 		if connected, ok := generic["connected"].(bool); ok {
 			server.Connected = connected
 		}

internal/contracts/converters_test.go

@@ -200,6 +200,43 @@ func TestConvertServerConfig_SourceRegistry(t *testing.T) {
 	assert.Empty(t, manual.SourceRegistryProvenance)
 }
 
+// TestConvertGenericServersToTyped_AutoApproveToolChanges verifies the
+// per-server auto_approve_tool_changes flag (MCP-2940) survives the
+// generic-map fallback projection so the Web UI toggle can read it. A server
+// that never set the flag must leave the pointer nil (tri-state), not coerce
+// to false.
+func TestConvertGenericServersToTyped_AutoApproveToolChanges(t *testing.T) {
+	genericServers := []map[string]interface{}{
+		{"id": "on", "name": "on", "enabled": true, "auto_approve_tool_changes": true},
+		{"id": "off", "name": "off", "enabled": true, "auto_approve_tool_changes": false},
+		{"id": "unset", "name": "unset", "enabled": true},
+	}
+
+	servers := ConvertGenericServersToTyped(genericServers)
+	require.Len(t, servers, 3)
+
+	require.NotNil(t, servers[0].AutoApproveToolChanges)
+	assert.True(t, *servers[0].AutoApproveToolChanges)
+
+	require.NotNil(t, servers[1].AutoApproveToolChanges)
+	assert.False(t, *servers[1].AutoApproveToolChanges)
+
+	assert.Nil(t, servers[2].AutoApproveToolChanges, "unset flag must stay nil, not coerce to false")
+}
+
+// TestConvertServerConfig_AutoApproveToolChanges verifies the direct
+// config→contracts mapper carries the auto_approve_tool_changes pointer.
+func TestConvertServerConfig_AutoApproveToolChanges(t *testing.T) {
+	on := true
+	cfg := &config.ServerConfig{Name: "on", Enabled: true, AutoApproveToolChanges: &on}
+	server := ConvertServerConfig(cfg, "ready", true, 0, false)
+	require.NotNil(t, server.AutoApproveToolChanges)
+	assert.True(t, *server.AutoApproveToolChanges)
+
+	unset := ConvertServerConfig(&config.ServerConfig{Name: "unset", Enabled: true}, "ready", true, 0, false)
+	assert.Nil(t, unset.AutoApproveToolChanges)
+}
+
 // TestConvertGenericServersToTyped_NoOAuth verifies servers without OAuth have nil OAuth field
 func TestConvertGenericServersToTyped_NoOAuth(t *testing.T) {
 	genericServers := []map[string]interface{}{

internal/contracts/types.go

@@ -41,19 +41,26 @@ type Server struct {
 	// IsolationDefaults exposes the resolved baseline values that
 	// would apply when no per-server override is set. Populated on
 	// list/get responses; never consumed on PATCH requests.
-	IsolationDefaults *IsolationDefaults   `json:"isolation_defaults,omitempty"`
-	Authenticated     bool                 `json:"authenticated"`                  // OAuth authentication status
-	OAuthStatus       string               `json:"oauth_status,omitempty"`         // OAuth status: "authenticated", "expired", "error", "none"
-	TokenExpiresAt    *time.Time           `json:"token_expires_at,omitempty"`     // When the OAuth token expires (ISO 8601)
-	ToolListTokenSize int                  `json:"tool_list_token_size,omitempty"` // Token size for this server's tools
-	ShouldRetry       bool                 `json:"should_retry,omitempty"`
-	RetryCount        int                  `json:"retry_count,omitempty"`
-	LastRetryTime     *time.Time           `json:"last_retry_time,omitempty"`
-	UserLoggedOut     bool                 `json:"user_logged_out,omitempty"`  // True if user explicitly logged out (prevents auto-reconnection)
-	Health            *HealthStatus        `json:"health,omitempty"`           // Unified health status calculated by the backend
-	Quarantine        *QuarantineStats     `json:"quarantine,omitempty"`       // Tool quarantine metrics for this server
-	ReconnectOnUse    bool                 `json:"reconnect_on_use,omitempty"` // Attempt reconnection when a tool call targets this disconnected server
-	SecurityScan      *SecurityScanSummary `json:"security_scan,omitempty"`    // Latest security scan results summary
+	IsolationDefaults *IsolationDefaults `json:"isolation_defaults,omitempty"`
+	Authenticated     bool               `json:"authenticated"`                  // OAuth authentication status
+	OAuthStatus       string             `json:"oauth_status,omitempty"`         // OAuth status: "authenticated", "expired", "error", "none"
+	TokenExpiresAt    *time.Time         `json:"token_expires_at,omitempty"`     // When the OAuth token expires (ISO 8601)
+	ToolListTokenSize int                `json:"tool_list_token_size,omitempty"` // Token size for this server's tools
+	ShouldRetry       bool               `json:"should_retry,omitempty"`
+	RetryCount        int                `json:"retry_count,omitempty"`
+	LastRetryTime     *time.Time         `json:"last_retry_time,omitempty"`
+	UserLoggedOut     bool               `json:"user_logged_out,omitempty"`  // True if user explicitly logged out (prevents auto-reconnection)
+	Health            *HealthStatus      `json:"health,omitempty"`           // Unified health status calculated by the backend
+	Quarantine        *QuarantineStats   `json:"quarantine,omitempty"`       // Tool quarantine metrics for this server
+	ReconnectOnUse    bool               `json:"reconnect_on_use,omitempty"` // Attempt reconnection when a tool call targets this disconnected server
+	// AutoApproveToolChanges mirrors config.ServerConfig.AutoApproveToolChanges
+	// (MCP-2930): the per-server intent to auto-approve new/changed tools past
+	// the trust baseline. Tri-state *bool — nil means "never set" (omitted from
+	// the payload), so the Web UI toggle (MCP-2932) can distinguish unset from
+	// an explicit false. Read-only on the GET path; PATCH/POST accept it via
+	// AddServerRequest.
+	AutoApproveToolChanges *bool                `json:"auto_approve_tool_changes,omitempty"`
+	SecurityScan           *SecurityScanSummary `json:"security_scan,omitempty"` // Latest security scan results summary
 	// Spec 044 — structured diagnostic error and stable error code. Both
 	// are populated when the server is in a failed state and the error
 	// has been classified by internal/diagnostics. Healthy servers omit

internal/httpapi/patch_server_test.go

@@ -23,6 +23,14 @@ type mockPatchServerController struct {
 	apiKey          string
 	existingServer  *config.ServerConfig
 	capturedUpdates *config.ServerConfig
+	allServers      []map[string]interface{}
+}
+
+// GetAllServers feeds the legacy GET /api/v1/servers path (used when no
+// management service is wired). Returning generic maps lets us assert how
+// the handler projects server-status fields into the JSON response.
+func (m *mockPatchServerController) GetAllServers() ([]map[string]interface{}, error) {
+	return m.allServers, nil
 }
 
 func (m *mockPatchServerController) GetCurrentConfig() any {
@@ -445,6 +453,115 @@ func TestHandleConvertConfigToSecret_ValidationErrors(t *testing.T) {
 	}
 }
 
+// TestHandlePatchServer_AutoApproveToolChanges covers the MCP-2940 gap: the
+// REST PATCH layer must accept and persist the per-server
+// auto_approve_tool_changes flag (a tri-state *bool added to ServerConfig in
+// MCP-2930). Mirrors the *bool nil-preserve semantics — omitting the field on
+// PATCH must not clobber a previously-set value, while an explicit value
+// (including false) must round-trip.
+func TestHandlePatchServer_AutoApproveToolChanges(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+
+	boolPtr := func(b bool) *bool { return &b }
+
+	newServer := func() *config.ServerConfig {
+		return &config.ServerConfig{
+			Name:        "github",
+			Protocol:    "stdio",
+			Command:     "npx",
+			Enabled:     true,
+			Quarantined: false,
+		}
+	}
+
+	patch := func(t *testing.T, existing *config.ServerConfig, body string) *config.ServerConfig {
+		t.Helper()
+		mockCtrl := &mockPatchServerController{apiKey: "test-key", existingServer: existing}
+		srv := NewServer(mockCtrl, logger, nil)
+		req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/github", bytes.NewReader([]byte(body)))
+		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-API-Key", "test-key")
+		w := httptest.NewRecorder()
+		srv.ServeHTTP(w, req)
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		require.NotNil(t, mockCtrl.capturedUpdates, "UpdateServer should have been called")
+		return mockCtrl.capturedUpdates
+	}
+
+	t.Run("explicit true sets the pointer", func(t *testing.T) {
+		updates := patch(t, newServer(), `{"auto_approve_tool_changes":true}`)
+		require.NotNil(t, updates.AutoApproveToolChanges, "pointer must be set when the request provides it")
+		assert.True(t, *updates.AutoApproveToolChanges, "value must reflect the request (true)")
+	})
+
+	t.Run("omitting preserves nil existing", func(t *testing.T) {
+		existing := newServer() // AutoApproveToolChanges is nil
+		updates := patch(t, existing, `{"args":["new-arg"]}`)
+		assert.Nil(t, updates.AutoApproveToolChanges,
+			"omitting the field must preserve the unset (nil) existing value")
+	})
+
+	t.Run("omitting preserves a prior true", func(t *testing.T) {
+		existing := newServer()
+		existing.AutoApproveToolChanges = boolPtr(true)
+		updates := patch(t, existing, `{"args":["new-arg"]}`)
+		require.NotNil(t, updates.AutoApproveToolChanges,
+			"omitting the field must preserve the existing pointer")
+		assert.True(t, *updates.AutoApproveToolChanges, "prior true must be preserved")
+	})
+
+	t.Run("explicit false overrides a prior true", func(t *testing.T) {
+		existing := newServer()
+		existing.AutoApproveToolChanges = boolPtr(true)
+		updates := patch(t, existing, `{"auto_approve_tool_changes":false}`)
+		require.NotNil(t, updates.AutoApproveToolChanges,
+			"explicit false must set the pointer, not leave it nil")
+		assert.False(t, *updates.AutoApproveToolChanges, "explicit false must override the prior true")
+	})
+}
+
+// TestHandleGetServers_ExposesAutoApproveToolChanges verifies the GET
+// /api/v1/servers payload surfaces auto_approve_tool_changes so the Web UI
+// toggle can reflect the persisted value (MCP-2940). Exercises the legacy
+// projection path (ConvertGenericServersToTyped) via a generic server map.
+func TestHandleGetServers_ExposesAutoApproveToolChanges(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	mockCtrl := &mockPatchServerController{
+		apiKey: "test-key",
+		allServers: []map[string]interface{}{
+			{
+				"id":                        "github",
+				"name":                      "github",
+				"enabled":                   true,
+				"quarantined":               false,
+				"auto_approve_tool_changes": true,
+			},
+		},
+	}
+	srv := NewServer(mockCtrl, logger, nil)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/v1/servers", http.NoBody)
+	req.Header.Set("X-API-Key", "test-key")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+
+	var resp struct {
+		Data struct {
+			Servers []struct {
+				Name                   string `json:"name"`
+				AutoApproveToolChanges *bool  `json:"auto_approve_tool_changes"`
+			} `json:"servers"`
+		} `json:"data"`
+	}
+	require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
+	require.Len(t, resp.Data.Servers, 1)
+	require.NotNil(t, resp.Data.Servers[0].AutoApproveToolChanges,
+		"auto_approve_tool_changes must appear in the GET payload")
+	assert.True(t, *resp.Data.Servers[0].AutoApproveToolChanges)
+}
+
 // TestHandleConvertConfigToSecret_ServerNotFound verifies the 404 path
 // for an unknown server name. Separate from the validation-errors table
 // because it needs an empty server list, not a single populated entry.

internal/httpapi/server.go

@@ -1326,6 +1326,13 @@ type AddServerRequest struct {
 	Enabled        *bool              `json:"enabled,omitempty"`
 	Quarantined    *bool              `json:"quarantined,omitempty"`
 	ReconnectOnUse *bool              `json:"reconnect_on_use,omitempty"`
+	// AutoApproveToolChanges is the per-server intent to auto-approve
+	// new/changed tools past the trust baseline (MCP-2930). Tri-state *bool:
+	// a nil pointer means "leave unchanged" on PATCH; a present value
+	// (including false) is applied. Mirrors config.ServerConfig's *bool
+	// semantics — do NOT collapse to a plain bool, or an omitted field would
+	// silently reset a previously-set value.
+	AutoApproveToolChanges *bool `json:"auto_approve_tool_changes,omitempty"`
 	// Isolation carries per-server Docker isolation overrides (image,
 	// network_mode, extra_args, working_dir, enabled). A nil pointer
 	// means "do not touch isolation config"; an empty-but-present
@@ -1450,6 +1457,11 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
 	if req.ReconnectOnUse != nil {
 		serverConfig.ReconnectOnUse = *req.ReconnectOnUse
 	}
+	// MCP-2940: carry the per-server auto-approve intent through on create.
+	// *bool nil-preserve semantics: only set when the caller provided it.
+	if req.AutoApproveToolChanges != nil {
+		serverConfig.AutoApproveToolChanges = req.AutoApproveToolChanges
+	}
 
 	// Add server via controller
 	logger := s.getRequestLogger(r) // T019: Use request-scoped logger
@@ -1639,6 +1651,17 @@ func (s *Server) handlePatchServer(w http.ResponseWriter, r *http.Request) {
 	} else if existingSrv != nil {
 		updates.ReconnectOnUse = existingSrv.ReconnectOnUse
 	}
+	// MCP-2940: auto_approve_tool_changes is a tri-state *bool, so unlike the
+	// non-pointer bools above we preserve the EXISTING POINTER (which may be
+	// nil = "never set") when the request omits the field — collapsing to a
+	// plain bool here would erase the unset/false distinction the trust-baseline
+	// logic (MCP-2931) relies on.
+	if req.AutoApproveToolChanges != nil {
+		updates.AutoApproveToolChanges = req.AutoApproveToolChanges
+		hasUpdates = true
+	} else if existingSrv != nil {
+		updates.AutoApproveToolChanges = existingSrv.AutoApproveToolChanges
+	}
 	if req.Isolation != nil {
 		updates.Isolation = req.Isolation.toConfig()
 		hasUpdates = true

internal/management/service.go

@@ -478,6 +478,14 @@ func (s *service) ListServers(ctx context.Context) ([]*contracts.Server, *contra
 			srv.ReconnectOnUse = reconnectOnUse
 		}
 
+		// MCP-2940: project the per-server auto-approve intent. Tri-state
+		// *bool — only set the pointer when the key is present so an unset
+		// flag stays nil and the Web UI toggle can tell unset from false.
+		if autoApprove, ok := srvRaw["auto_approve_tool_changes"].(bool); ok {
+			v := autoApprove
+			srv.AutoApproveToolChanges = &v
+		}
+
 		// Extract unified health status
 		if health, ok := srvRaw["health"].(*contracts.HealthStatus); ok {
 			srv.Health = health

internal/management/service_test.go

@@ -163,6 +163,52 @@ func TestListServers(t *testing.T) {
 		assert.Empty(t, byName["manual"].SourceRegistryProvenance)
 	})
 
+	// MCP-2940: auto_approve_tool_changes must be projected onto contracts.Server
+	// so the Web UI toggle (MCP-2932) can reflect the persisted per-server value.
+	t.Run("auto_approve_tool_changes projected", func(t *testing.T) {
+		runtime := newMockRuntime()
+		runtime.servers = []map[string]interface{}{
+			{
+				"id":                        "auto-on",
+				"name":                      "auto-on",
+				"enabled":                   true,
+				"auto_approve_tool_changes": true,
+			},
+			{
+				"id":                        "auto-off",
+				"name":                      "auto-off",
+				"enabled":                   true,
+				"auto_approve_tool_changes": false,
+			},
+			{
+				"id":      "unset",
+				"name":    "unset",
+				"enabled": true,
+			},
+		}
+
+		svc := NewService(runtime, cfg, "", emitter, nil, logger)
+		servers, _, err := svc.ListServers(context.Background())
+		require.NoError(t, err)
+		require.Len(t, servers, 3)
+
+		byName := map[string]*contracts.Server{}
+		for _, s := range servers {
+			byName[s.Name] = s
+		}
+		require.Contains(t, byName, "auto-on")
+		require.NotNil(t, byName["auto-on"].AutoApproveToolChanges)
+		assert.True(t, *byName["auto-on"].AutoApproveToolChanges)
+
+		require.Contains(t, byName, "auto-off")
+		require.NotNil(t, byName["auto-off"].AutoApproveToolChanges)
+		assert.False(t, *byName["auto-off"].AutoApproveToolChanges)
+
+		require.Contains(t, byName, "unset")
+		assert.Nil(t, byName["unset"].AutoApproveToolChanges,
+			"a server that never set the flag must omit it (nil), not coerce to false")
+	})
+
 	// T094: Test that TotalTools only counts enabled servers' tools (Issue #285 fix)
 	t.Run("TotalTools excludes disabled servers", func(t *testing.T) {
 		runtime := newMockRuntime()