Skip to content

Commit 38036ea

Browse files
authored
fix(telemetry): materialize resolved telemetry.enabled in /api/v1/config (#681)
Telemetry is opt-out (default enabled), but DefaultConfig leaves the Telemetry field nil and Enabled is a pointer with omitempty. On a fresh install the /api/v1/config response omitted the telemetry key entirely, so the web UI (SettingField !!modelValue) and macOS (?? false) coerced the missing bool to false and displayed telemetry as DISABLED — even though Config.IsTelemetryEnabled() and the CLI correctly reported it enabled. Fix at the contract/response boundary: mirror IsTelemetryEnabled() into a copy of the config before marshaling so telemetry.enabled is always present, without mutating the shared config or changing the Go default-true logic. One server-side fix covers both UI clients. Related #MCP-2477
1 parent 9794dfe commit 38036ea

2 files changed

Lines changed: 82 additions & 0 deletions

File tree

internal/contracts/converters.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,25 @@ func ConvertConfigToContract(cfg *config.Config) interface{} {
530530
return nil
531531
}
532532

533+
// Materialize the resolved telemetry.enabled value before marshaling.
534+
// Telemetry is opt-out (default enabled), but DefaultConfig leaves the
535+
// Telemetry field nil and Enabled is a pointer with `omitempty`, so a fresh
536+
// install would omit the `telemetry` key entirely. Both the web UI and macOS
537+
// clients coerce the missing bool to false, displaying telemetry as DISABLED
538+
// even though Config.IsTelemetryEnabled() reports true (MCP-2477). Mirror that
539+
// resolution into the serialized response without mutating the shared config.
540+
if cfg.Telemetry == nil || cfg.Telemetry.Enabled == nil {
541+
cfgCopy := *cfg
542+
var tel config.TelemetryConfig
543+
if cfg.Telemetry != nil {
544+
tel = *cfg.Telemetry
545+
}
546+
enabled := cfg.IsTelemetryEnabled()
547+
tel.Enabled = &enabled
548+
cfgCopy.Telemetry = &tel
549+
return &cfgCopy
550+
}
551+
533552
// Return the config as-is for JSON marshaling
534553
// The JSON tags on config.Config will handle serialization
535554
return cfg

internal/contracts/converters_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package contracts
22

33
import (
4+
"encoding/json"
45
"testing"
56
"time"
67

@@ -9,6 +10,68 @@ import (
910
"github.com/stretchr/testify/require"
1011
)
1112

13+
// telemetryEnabledFromContract marshals the contract representation of cfg and
14+
// extracts the resolved telemetry.enabled value (or reports it absent).
15+
func telemetryEnabledFromContract(t *testing.T, cfg *config.Config) (enabled, present bool) {
16+
t.Helper()
17+
raw, err := json.Marshal(ConvertConfigToContract(cfg))
18+
require.NoError(t, err)
19+
20+
var decoded struct {
21+
Telemetry *struct {
22+
Enabled *bool `json:"enabled"`
23+
} `json:"telemetry"`
24+
}
25+
require.NoError(t, json.Unmarshal(raw, &decoded))
26+
if decoded.Telemetry == nil || decoded.Telemetry.Enabled == nil {
27+
return false, false
28+
}
29+
return *decoded.Telemetry.Enabled, true
30+
}
31+
32+
// TestConvertConfigToContract_TelemetryMaterializedOnFreshDefault asserts that
33+
// /api/v1/config exposes the resolved telemetry.enabled value (true) on a fresh
34+
// install where Telemetry is nil, instead of omitting the key (which clients
35+
// coerce to false). Regression test for MCP-2477.
36+
func TestConvertConfigToContract_TelemetryMaterializedOnFreshDefault(t *testing.T) {
37+
t.Setenv("MCPPROXY_TELEMETRY", "") // ensure no env override
38+
cfg := config.DefaultConfig()
39+
require.Nil(t, cfg.Telemetry, "precondition: DefaultConfig leaves Telemetry nil")
40+
41+
enabled, present := telemetryEnabledFromContract(t, cfg)
42+
assert.True(t, present, "telemetry.enabled must be present in the API response")
43+
assert.True(t, enabled, "telemetry.enabled must serialize as true on a fresh default install")
44+
45+
// Materialization must not mutate the shared config.
46+
assert.Nil(t, cfg.Telemetry, "ConvertConfigToContract must not mutate the source config")
47+
}
48+
49+
// TestConvertConfigToContract_TelemetryEnabledNilMaterialized covers the case
50+
// where a TelemetryConfig exists but Enabled is nil — it must resolve to true.
51+
func TestConvertConfigToContract_TelemetryEnabledNilMaterialized(t *testing.T) {
52+
t.Setenv("MCPPROXY_TELEMETRY", "")
53+
cfg := config.DefaultConfig()
54+
cfg.Telemetry = &config.TelemetryConfig{AnonymousID: "abc"} // Enabled nil
55+
56+
enabled, present := telemetryEnabledFromContract(t, cfg)
57+
assert.True(t, present, "telemetry.enabled must be present when Enabled is nil")
58+
assert.True(t, enabled, "telemetry.enabled must resolve to true when Enabled is nil")
59+
assert.Nil(t, cfg.Telemetry.Enabled, "source TelemetryConfig.Enabled must remain nil")
60+
}
61+
62+
// TestConvertConfigToContract_TelemetryExplicitFalsePreserved asserts an
63+
// explicit opt-out is faithfully serialized (not overwritten by the default).
64+
func TestConvertConfigToContract_TelemetryExplicitFalsePreserved(t *testing.T) {
65+
t.Setenv("MCPPROXY_TELEMETRY", "")
66+
disabled := false
67+
cfg := config.DefaultConfig()
68+
cfg.Telemetry = &config.TelemetryConfig{Enabled: &disabled}
69+
70+
enabled, present := telemetryEnabledFromContract(t, cfg)
71+
assert.True(t, present, "telemetry.enabled must be present when explicitly set")
72+
assert.False(t, enabled, "explicit telemetry opt-out must be preserved")
73+
}
74+
1275
// TestConvertGenericServersToTyped_OAuth verifies OAuth config is properly extracted
1376
func TestConvertGenericServersToTyped_OAuth(t *testing.T) {
1477
// Simulate the map structure returned from the management service

0 commit comments

Comments
 (0)