feat(api): expose & persist auto_approve_tool_changes in server REST API (MCP-2940)#727
Merged
Merged
Conversation
…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.
Deploying mcpproxy-docs with
|
| Latest commit: |
3e7f20d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://211e9748.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2940-rest-auto-appro.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27821934238 --repo smart-mcp-proxy/mcpproxy-go
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the REST gap from MCP-2940 (parent MCP-2916). MCP-2930 added the per-server
auto_approve_tool_changesflag toconfig.ServerConfigbut did not extend the REST layer, so the Web-UI "Auto-approve tool changes" toggle (MCP-2932, PR #725) could neither read nor persist it. This wires the flag through the REST request/response paths and through BBolt persistence.Changes
REST request side (
internal/httpapi,internal/server)AutoApproveToolChanges *booltoAddServerRequest.handleAddServer(create) andhandlePatchServerwith*boolnil-preserve semantics: omitting on PATCH preserves the existing pointer (may be nil = "never set"); an explicit value — includingfalse— is applied.server.UpdateServerapplies the pointer only when non-nil, so callers that don't touch it (e.g. config-to-secret) never reset it.REST read side (
internal/contracts,internal/runtime,internal/management)AutoApproveToolChanges *booltocontracts.Server(omitempty→ tri-state nil stays out of the payload, so the UI can tell unset from explicitfalse).auto_approve_tool_changesfromruntime.GetAllServers(StateView) +getAllServersLegacy, project it inmanagement.ListServersandConvertGenericServersToTyped/ConvertServerConfig— in parity withquarantined.Persistence (
internal/storage) — discovered during verificationSaveConfigurationrebuilds the JSON config's server list from BBolt records, so a field absent fromUpstreamRecordis wiped on the next mutation. AddedAutoApproveToolChangestoUpstreamRecord+ all four selective conversions, and updatedTestSaveServerSyncFieldCoverageto require it in BBolt. Without this the toggle would not survive a save/restart (the exact "always shows OFF after reload" symptom the issue describes).Verification
go test ./internal/{storage,httpapi,contracts,management}/... -racegreen; targetedinternal/servertests green.golangci-lint run --config .github/.golangci.yml→ 0 issues.make swagger); docs updated (tool-quarantine.md,rest-api.md) per ENG-9.mcpproxy serve: POSTtrue→ GETtrue; PATCHfalse→ GETfalse; PATCH args-only → GET preservesfalse; a server added without the flag → omitted/nil; all values survive a full restart; config file persistsauto_approve_tool_changes: false.Notes
*boolthroughout — nil-preserve semantics mirror the config field; no collapse to a plain bool.Related #725