fix: telemetry opt-out shown as effective state; macOS Settings names the connected core (Spec fix-usertest-telemetry-macos) - #1471
Merged
Conversation
Resolves env opt-out vs config vs default into one EffectiveState
{enabled, source, disabled_by} so UIs can show the real state. Adds a
send-nothing proof for MCPPROXY_TELEMETRY, DO_NOT_TRACK and CI.
The wizard Verify step and Home banner say telemetry is off and why when an environment variable disables it, stay silent when the user's own config disables it, and Settings locks the toggle off with the reason.
…e in Settings The first-run welcome says telemetry is off and why when the app's environment disables it. Settings locks the telemetry toggle off with the reason, names the connected core and its running listen address, adopts that address when the config omits listen, and notes a pending restart.
…gs user-test fixes Spec, tasks, parity rows 28a and 29a, acceptance scenarios US7-7 and US7-8, research D37, quickstart recipe, REST, telemetry and macOS tray docs.
A 401 before the API key is stored no longer starts the 30 s reuse window.
Deploying mcpproxy-docs with
|
| Latest commit: |
4f6aaa8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://205e27db.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-usertest-telemetry-macos.mcpproxy-docs.pages.dev |
F4.1 (macOS): the Settings listen note cached the running address from the first load only. ConfigStore now re-reads /api/v1/status when the tray lands on a new core connection and whenever a Settings tab appears, so the note follows a restarted core (FR-044b). F3.1 (web): the Raw JSON Apply path posted the whole document without checking field locks, so an env-locked telemetry.enabled could be persisted. Apply now refuses a document that changes a locked key (FR-044a). The macOS Raw tab is read-only, so it has no equivalent path.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37080784917 --repo smart-mcp-proxy/mcpproxy-go
|
11 tasks
macOS Settings: an adopted listen address follows the running core on every status refresh while the config omits listen and the field is unedited, and the pending-restart note reads the config file, never the adopted mirror (F1.1, XCTest covers a running-address change and an edited field). Web: lockedKeysChanged matches keys case-insensitively with last-key-wins, as the backend decodes Raw JSON (F1.2); Raw JSON Apply refreshes the effective telemetry state (F3.2). Tests fail without the fixes. Deferred: server-side enforcement of the telemetry lock in the apply handler (F1.2 preferred variant). The stored value is a legitimate setting that the environment overrides at runtime; rejecting it would break scripted edits. QA.mac: live macOS verification was not done in this stage; the screen was unlocked here but the live-QA stage owns it. Not faked.
Web: lockedKeysChanged no longer assumes document-order last-key-wins. The backend round-trips Raw JSON through a map (UnmaskLiveConfigDocument marshals sorted keys), so among case-variant duplicates the byte-order winner decides, and same-named objects merge. Every case-variant reading of a locked key is now collected and the document is refused when any differs from the stored value (fail toward refusal). The new test fails on the previous code: a document whose document-order last key matched the stored value but whose sorted winner turned telemetry off slipped through. Deferred: server-side enforcement of the telemetry lock in the apply handler. The stored value is a legitimate setting that the environment overrides at runtime; rejecting it would break scripted edits.
…tch (Spec 109 FR-044a) Merge origin/main and add server-side enforcement of the telemetry.enabled lock the Web and macOS UIs show while DO_NOT_TRACK, CI or MCPPROXY_TELEMETRY=false forces telemetry off. POST /config/apply and PATCH /config answer 422 and write nothing when the typed result would change the value (miscased keys included); unchanged values, including the GET-then-POST round trip of an unset setting, pass.
Renumber this PR's Spec 109 tasks to T205-T210 (main owns T199-T204 from fix-usertest-web) and research D38 to D42; Phase 20; task count 246. Keep main's FR-043 text next to FR-044/FR-044a/FR-044b. Regenerate oas/docs.go, oas/swagger.yaml and ROADMAP.md.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
2 participants
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
Two medium findings from the codex first-run user test, fixed on every surface that shows them. Both are on the Spec 109 side of the ownership rule (onboarding and the telemetry notice, Settings, macOS Settings); nothing here touches Spec 108 surfaces.
F-03: the notice and Settings now show the effective telemetry state
With
MCPPROXY_TELEMETRY=false(orDO_NOT_TRACK,CI) the core sends nothing, but the Web wizard and Home banner still said "MCPProxy sends anonymous usage statistics", the macOS first-run welcome did the same, and Settings showed the storedtelemetry.enabled: trueas on.GET /api/v1/statusgainstelemetry: {enabled, source, disabled_by}.sourceisenv,configordefault.disabled_byappears only forenvand reuses the existing reasons. The block is withheld from scoped callers (agent tokens), likeactivation. It is resolved from the running config by the newtelemetry.ResolveEffectiveState, which always agrees withEffectiveTelemetryEnabled.GET /api/v1/configis unchanged: it stays the stored, round-trippable document.MCPPROXY_TELEMETRY=false,FALSE,DO_NOT_TRACK=1andCI=truethroughStart, the shutdown flush and the opt-out beacon, and expects zero requests. A control run with no environment variable must reach the server, so the test cannot pass vacuously. Neutering both env gates makes it fail.F-10: macOS Settings names the core it is editing
The reported "wrong listen address and telemetry state" came from the dev app being attached to the user's real v0.69.0 core: the tray ignores
." with "The saved address Y takes effect after a restart." when it differs, and a config withoutHOME, andopendrops shell variables. Settings was showing the connected core's values correctly; nothing told the tester which core that was. Settings → Security now shows, under Listen address, "Connected core vX is listening onlistenshows the running address instead of the placeholder without becoming a pending change.docs/development/macos-tray.mddocuments the dev-rig trap.Audit findings fixed
F-03 (medium) and F-10 (medium) of the codex first-run user test.
Spec
specs/109-ux-navigation-consistency: Phase 17 (T172 to T177), US7-7 and US7-8, FR-044a and FR-044b, parity rows 28a and 29a, research D37, quickstart recipefix-usertest-telemetry-macos, plan row. The spec-count assertions in the traceability and parity tests move from 43 to 45 scenarios and from 32 to 34 matrix rows. Docs updated: the REST status section (status.telemetry), the telemetry feature page (how the UI shows an environment opt-out) and the macOS tray development page.Tests added
internal/telemetry/effective_state_test.go(shared fixtureinternal/telemetry/testdata/effective_state_cases.json),internal/telemetry/env_gate_send_test.go,internal/httpapi/status_telemetry_test.go.telemetry-state.spec.ts(same fixture),settings-telemetry-env-lock.spec.ts, extendedtelemetry-banner-wizard.spec.ts.TelemetryNoticeTests.swift(same fixture),SettingsEffectiveStateTests.swift.mcpproxy telemetry status.No new dependencies.
oas/swagger.yamlchanges by the one description line.Follow-ups (not in this PR)
config.IsTelemetryEnabledcompares the environment value strictly to"false"whileIsDisabledByEnvtrims and case-folds;ConvertConfigToContractmaterialises an environment-forcedfalseinto the config document when the stored value is nil; the tray could warn whenHOMEdiffers butMCPPROXY_HOMEis unset; the Web listen field could show the running address when it differs from the configured one.