fix(config): copy all ServerConfig fields in CopyServerConfig

[electrolobzik]

Problem

CopyServerConfig (internal/config/merge.go) hand-copies a subset of ServerConfig fields and has drifted as new fields were added. It silently omits 8 fields: EnabledTools, DisabledTools, ReconnectOnUse, HealthCheckInterval, ToolDiscoveryInterval, SourceRegistryID, SourceRegistryProvenance, and AuthBroker (only AutoApproveToolChanges was kept current).

Because MergeServerConfig starts from CopyServerConfig(base), any config merge/patch (e.g. upstream_servers patch, the Web UI server-config edit) round-trips the server through CopyServerConfig and drops these fields.

User-visible symptom: editing an unrelated field on a server that has a disabled_tools / enabled_tools allowlist (or reconnect_on_use, or per-server health/discovery cadence overrides) silently wipes those settings.

Fix

Copy the missing fields:

• scalars (ReconnectOnUse, SourceRegistryID, SourceRegistryProvenance) in the struct literal;
• the allowlist slices (EnabledTools, DisabledTools) deep-copied like Args;
• the *Duration overrides (HealthCheckInterval, ToolDiscoveryInterval) by value, matching the existing AutoApproveToolChanges pattern;
• the AuthBroker block by value (a no-op empty stub in the personal edition).

Tests

• TestCopyServerConfig_PreservesAllowlistAndOverrides — asserts each field is copied, and that the slices/pointers are independent of the source (deep copy).
• TestMergeServerConfig_PreservesDisabledToolsOnUnrelatedPatch — end-to-end regression: an unrelated patch must not wipe a server's disabled_tools.

Follow-up (not in this PR)

The same record↔config field mapping is hand-duplicated in the storage layer (SaveUpstreamServer / GetUpstreamServer / ListUpstreamServers / ListQuarantinedUpstreamServers), and ListQuarantinedUpstreamServers has drifted similarly (missing AutoApproveToolChanges, LauncherWaitTimeout, and the spec-074 intervals). Collapsing all of these onto a single helper would prevent the next field from drifting again — happy to do that in a separate PR if you'd like.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/config/merge.go	90.00%	2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[mcpproxy-gatekeeper]

✅ Gatekeeper approval — Codex review verdict: ACCEPT.

This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.

Auto-approved per Model B (MCP-1249).

[Dumbris]

Thank you, @electrolob — really nice fix. 🙏

A reviewer enumerated all 26 ServerConfig fields and confirmed the copy is now complete and correct:

• Thoughtful deep-copy discipline — you copied the *Duration / *bool fields by value (not by sharing the pointer), deep-copied the slices/maps with make+copy, and handled the build-tagged AuthBroker stub-vs-real distinction with an accurate comment. That subtlety trips up a lot of contributors.
• Excellent tests with intent — they don't just check field presence, they assert non-aliasing (mutate the source, confirm the copy is unaffected) and include an end-to-end MergeServerConfig regression proving a disabled_tools denylist survives an unrelated patch.

Verified locally: go build ./... clean, config + storage tests green (incl. -tags server), gofmt clean.

Merging now. Two optional, non-blocking notes for the future: (1) in the server edition, AuthBrokerConfig has a Scopes []string, so dst.AuthBroker.Scopes still aliases the source — a make+copy there would give full parity (no-op in the personal edition where it's struct{}); (2) a reflect-based completeness test (like TestSaveServerSyncFieldCoverage) would catch the next added field automatically. Neither blocks this — thanks again for the careful work!

[mcpproxy-gatekeeper]

APPROVE — Claude Code review. All 26 ServerConfig fields copied; slices/maps/pointers deep-copied (non-aliasing asserted by new tests); *Duration/*bool by value; build-tagged AuthBroker handled. go build + config/storage tests (incl -tags server) pass, gofmt clean. Non-blocking nits: server-edition AuthBroker.Scopes shallow (low impact); reflection completeness test suggested. Community PR by @electrolob.