fix(config): copy all ServerConfig fields in CopyServerConfig

internal/config/merge.go

@@ -528,25 +528,36 @@ func CopyServerConfig(src *ServerConfig) *ServerConfig {
 	}
 
 	dst := &ServerConfig{
-		Name:                src.Name,
-		URL:                 src.URL,
-		Protocol:            src.Protocol,
-		Command:             src.Command,
-		WorkingDir:          src.WorkingDir,
-		Enabled:             src.Enabled,
-		Quarantined:         src.Quarantined,
-		SkipQuarantine:      src.SkipQuarantine,
-		Shared:              src.Shared,
-		Created:             src.Created,
-		Updated:             src.Updated,
-		LauncherWaitTimeout: src.LauncherWaitTimeout,
+		Name:                     src.Name,
+		URL:                      src.URL,
+		Protocol:                 src.Protocol,
+		Command:                  src.Command,
+		WorkingDir:               src.WorkingDir,
+		Enabled:                  src.Enabled,
+		Quarantined:              src.Quarantined,
+		SkipQuarantine:           src.SkipQuarantine,
+		Shared:                   src.Shared,
+		Created:                  src.Created,
+		Updated:                  src.Updated,
+		LauncherWaitTimeout:      src.LauncherWaitTimeout,
+		ReconnectOnUse:           src.ReconnectOnUse,
+		SourceRegistryID:         src.SourceRegistryID,
+		SourceRegistryProvenance: src.SourceRegistryProvenance,
 	}
 
 	// Copy slices
 	if src.Args != nil {
 		dst.Args = make([]string, len(src.Args))
 		copy(dst.Args, src.Args)
 	}
+	if src.EnabledTools != nil {
+		dst.EnabledTools = make([]string, len(src.EnabledTools))
+		copy(dst.EnabledTools, src.EnabledTools)
+	}
+	if src.DisabledTools != nil {
+		dst.DisabledTools = make([]string, len(src.DisabledTools))
+		copy(dst.DisabledTools, src.DisabledTools)
+	}
 
 	// Copy maps
 	if src.Env != nil {
@@ -568,6 +579,24 @@ func CopyServerConfig(src *ServerConfig) *ServerConfig {
 		dst.AutoApproveToolChanges = &autoApprove
 	}
 
+	// Copy *Duration overrides by value to avoid shared pointer state (spec 074)
+	if src.HealthCheckInterval != nil {
+		hc := *src.HealthCheckInterval
+		dst.HealthCheckInterval = &hc
+	}
+	if src.ToolDiscoveryInterval != nil {
+		td := *src.ToolDiscoveryInterval
+		dst.ToolDiscoveryInterval = &td
+	}
+
+	// Copy the per-upstream auth-broker block by value (spec 074, server edition).
+	// In the personal edition AuthBrokerConfig is an empty stub struct, so this is
+	// a no-op there; copying by value keeps the pointer from being shared.
+	if src.AuthBroker != nil {
+		broker := *src.AuthBroker
+		dst.AuthBroker = &broker
+	}
+
 	// Copy nested structs
 	dst.Isolation = copyIsolationConfig(src.Isolation)
 	dst.OAuth = copyOAuthConfig(src.OAuth)

internal/config/merge_test.go

@@ -997,3 +997,74 @@ func TestCopyServerConfig_AllFields(t *testing.T) {
 		t.Error("CopyServerConfig(nil) should return nil")
 	}
 }
+
+// TestCopyServerConfig_PreservesAllowlistAndOverrides guards the fields that
+// were silently dropped by CopyServerConfig before this fix: the per-tool
+// allowlist/denylist, ReconnectOnUse, the spec-074 *Duration overrides, and the
+// source-registry provenance. A drop here loses a server's disabled_tools and
+// per-server cadence overrides on any config merge/patch round-trip.
+func TestCopyServerConfig_PreservesAllowlistAndOverrides(t *testing.T) {
+	hc := Duration(30 * time.Second)
+	td := Duration(5 * time.Minute)
+	src := &ServerConfig{
+		Name:                     "srv",
+		ReconnectOnUse:           true,
+		EnabledTools:             []string{"a", "b"},
+		DisabledTools:            []string{"c"},
+		HealthCheckInterval:      &hc,
+		ToolDiscoveryInterval:    &td,
+		SourceRegistryID:         "reg-1",
+		SourceRegistryProvenance: "official",
+	}
+
+	dst := CopyServerConfig(src)
+
+	if !dst.ReconnectOnUse {
+		t.Error("ReconnectOnUse was dropped")
+	}
+	if dst.SourceRegistryID != "reg-1" {
+		t.Errorf("SourceRegistryID: got %q, want %q", dst.SourceRegistryID, "reg-1")
+	}
+	if dst.SourceRegistryProvenance != "official" {
+		t.Errorf("SourceRegistryProvenance: got %q, want %q", dst.SourceRegistryProvenance, "official")
+	}
+	if !reflect.DeepEqual(dst.EnabledTools, src.EnabledTools) {
+		t.Errorf("EnabledTools: got %v, want %v", dst.EnabledTools, src.EnabledTools)
+	}
+	if !reflect.DeepEqual(dst.DisabledTools, src.DisabledTools) {
+		t.Errorf("DisabledTools: got %v, want %v", dst.DisabledTools, src.DisabledTools)
+	}
+	if dst.HealthCheckInterval == nil || *dst.HealthCheckInterval != hc {
+		t.Errorf("HealthCheckInterval: got %v, want %v", dst.HealthCheckInterval, hc)
+	}
+	if dst.ToolDiscoveryInterval == nil || *dst.ToolDiscoveryInterval != td {
+		t.Errorf("ToolDiscoveryInterval: got %v, want %v", dst.ToolDiscoveryInterval, td)
+	}
+
+	// Deep copy: mutating the source must not affect the copy.
+	src.DisabledTools[0] = "mutated"
+	if dst.DisabledTools[0] == "mutated" {
+		t.Error("DisabledTools is aliased, not deep-copied")
+	}
+	*src.HealthCheckInterval = Duration(time.Hour)
+	if *dst.HealthCheckInterval == Duration(time.Hour) {
+		t.Error("HealthCheckInterval pointer is shared, not copied by value")
+	}
+}
+
+// TestMergeServerConfig_PreservesDisabledToolsOnUnrelatedPatch is the
+// end-to-end regression: patching an unrelated field on a server that has a
+// disabled_tools denylist must not wipe the denylist (it previously did,
+// because MergeServerConfig starts from CopyServerConfig(base)).
+func TestMergeServerConfig_PreservesDisabledToolsOnUnrelatedPatch(t *testing.T) {
+	base := &ServerConfig{Name: "srv", DisabledTools: []string{"danger"}}
+	patch := &ServerConfig{Env: map[string]string{"K": "V"}}
+
+	merged, _, err := MergeServerConfig(base, patch, MergeOptions{})
+	if err != nil {
+		t.Fatalf("MergeServerConfig: %v", err)
+	}
+	if len(merged.DisabledTools) != 1 || merged.DisabledTools[0] != "danger" {
+		t.Errorf("DisabledTools dropped on unrelated patch: got %v, want [danger]", merged.DisabledTools)
+	}
+}