feat(telemetry): wire up the analytics catalog, identify users, fix opt-out precedence - #367
Merged
Conversation
Ten of the seventeen events in `Analytics.Event` had no call site, so PostHog only ever received startup, error, slow-call, and diagnostic events. Wire up the container, image, Kubernetes, terminal, and settings events at their real success points. Conventions applied throughout: - fire on success only; failures already emit `error_occurred` - container events carry `backend` (grpc/docker) and `batch`, so the two engine paths and bulk actions stay separable - terminal events carry `surface`, captured where the session actually reaches `.connected` rather than at the open request - properties stay low-cardinality: no IDs, names, image references, or paths Drop `app_launched`: the SDK's `captureApplicationLifecycleEvents` already emits Application Installed/Opened/Backgrounded on macOS.
Events were entirely anonymous: `identify` was never called, so with `personProfiles = .identifiedOnly` PostHog built no person profiles, and no super properties meant nothing could be segmented by environment. Identity mirrors `AuthSession` from the coordinator via `withObservationTracking`, following the existing daemon/startup idiom — `ArcBoxAuth` stays free of any analytics dependency. distinct_id is the OIDC subject; email, name, and email_verified go in as person properties. Sign-out calls `reset()` so a second account on the same Mac is not merged into the first. Re-identify is keyed on the whole `AuthIdentity` because `loadUserInfo()` enriches it after sign-in. Super properties: arcbox_profile and update_channel at setup, system_vm_backend from `SystemVmBackendModel` (funnelled through one setter so the three assignment sites cannot drift), k8s_active from `KubernetesState`. The SDK already supplies $app_version, $os_version, and $device_type. The Privacy toggle's opt-in/opt-out moves from `GeneralSettingsView` into `defaultsDidChange`. Both used to be driven by the same UserDefaults write with no ordering guarantee, and the SDK drops `identify` while opted out — so a user who signed in before enabling telemetry would have stayed anonymous forever. Opting back in now re-runs identify. The toggle's caption claimed "No personal data is collected", which is no longer true once an account is linked; it now says so.
`setup()` lets the SDK's own persisted `.optOut` flag override `config.optOut` (PostHogSDK.swift:128), so the value derived from `telemetryEnabled` was only a fallback for a fresh install. Once the two diverged, PostHog's copy silently won on every launch and Settings > Privacy no longer described what was happening. Restate the preference right after setup so the default — opted in — actually holds. Also restore super properties after `reset()`: it deletes the registered-properties key along with the identity (PostHogStorage.swift:403), which dropped arcbox_profile, update_channel, system_vm_backend, and k8s_active from every event for the rest of a session after sign-out. They describe the install, not the person, so Analytics now keeps its own mirror and re-registers. No behavior change for signed-out users: `capture` never required person processing, so their events were always collected — only the person profile depends on signing in. Said so in the doc comment, since the previous wording invited the opposite reading.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThe PR connects the analytics event catalog to feature operations, synchronizes PostHog identity with authentication state, registers installation-level dimensions, and makes the ArcBox telemetry preference authoritative.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| ArcBox/Analytics.swift | Adds centralized identity, opt-in, reset, and super-property APIs alongside the expanded analytics catalog. |
| ArcBox/App/AppDelegate+Telemetry.swift | Makes the application preference authoritative after PostHog setup and registers initial installation dimensions. |
| ArcBox/App/ApplicationCoordinator.swift | Observes authentication and preference state to synchronize analytics identity, update-channel properties, and telemetry enablement. |
| ArcBox/Integrations/Terminal/ExternalTerminalLauncher.swift | Records external-terminal activity only after the asynchronous launch path returns successfully. |
| ArcBox/ViewModels/KubernetesState.swift | Captures successful Kubernetes transitions and keeps the active-state super property synchronized. |
| ArcBox/Views/Settings/GeneralSettingsView.swift | Updates telemetry disclosure wording and delegates preference side effects to the coordinator. |
Sequence Diagram
sequenceDiagram
participant Defaults as UserDefaults
participant App as AppDelegate
participant Coordinator as ApplicationCoordinator
participant Auth as AuthSession
participant Analytics
participant PostHog
App->>Defaults: Register preference defaults
App->>PostHog: Setup SDK
App->>Analytics: Apply opt-in or opt-out
App->>Analytics: Register install properties
Coordinator->>Auth: Observe status and identity
alt Signed in
Coordinator->>Analytics: identify(subject, profile properties)
Analytics->>PostHog: Identify person
else Signed out after prior identity
Coordinator->>Analytics: reset()
Analytics->>PostHog: Reset identity
Analytics->>PostHog: Restore super properties
end
Defaults-->>Coordinator: Telemetry preference changed
Coordinator->>Analytics: Apply new preference
Reviews (3): Last reviewed commit: "Merge origin/master into feat/telemetry-..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…erminal Two real defects from review. The signed-out reset was skipped on launch. `identifiedAs` starts nil, so the dedupe guard returned before reaching `reset()` — but PostHog's identity persists across launches, and a session can end while the app is closed (revoked refresh token, cleared Keychain). Those launches kept reporting under the previous account. The signed-out branch now runs before the dedupe and consults `analyticsIdentified` in UserDefaults, which outlives the process, rather than the in-process cache. `terminal_opened` for external terminals fired before the launch was attempted, counting failures as opens. Every path does have a success signal — `NSWorkspace.open` returns Bool, the completion handler reports an error, and NSAppleScript fills its error out-param — so the capture is now threaded through as an `onOpened` callback invoked only by whichever path actually launches a terminal. This matches how the in-app sessions already report at `.connected`. The third comment, reordering `import os` above `import PostHog`, is declined: OrderedImports in .swift-format sorts one alphabetical block and rewrites it back, and DockerTerminalSession.swift:3-4 already has third-party SwiftTerm before os.
Master moved substantially under this branch (#369 in particular), and the PR went DIRTY — which is why `pull_request` CI stopped running on the branch entirely: GitHub cannot compute a merge commit for a conflicted PR, so the last push was never built or tested. Merged rather than rebased: the drift touches the same lines this branch changes, so a rebase would have re-resolved the same conflicts once per commit. Three files conflicted. ExternalTerminalLauncher — master rewrote it as `async throws` with typed `ExternalTerminalLaunchError`. This supersedes the `onOpened` callback added in efe8ff5 for the same review comment: every failure path now propagates, so a plain capture after the launch returns is both correct and simpler. The callback threading is gone. DockerTerminalSession — master funnelled both image paths through `launchImageSession` and added `imageContainerName`. That already discriminates the surface, so the `surface` parameter is dropped and the value derived at the capture site; two parameters encoding the same fact could only drift. ImagesViewModel+Docker — master replaced `try response.ok` with `applyImageDeletion(_:id:)` returning success; `image_removed` moves inside that branch. All 15 catalog events still have call sites. pbxproj and project.yml are byte-identical to master.
KafuChino123
added a commit
that referenced
this pull request
Aug 12, 2026
The 1.35.0 release PR merged with an empty `### Highlights` section, so `cargo xtask release notes` now fails on every PR against master. That step runs before Lint, Build and Test, so no PR gets as far as compiling. The release DMG extracts the same section to feed Sparkle's update dialog and refuses to build without it, so 1.35.0 cannot be packaged either. Write the section from what 1.35.0 actually shipped: the Fleet device auth, enrollment and runner management in #320, the authoritative sandbox port reconciliation in #374, and the telemetry identification and Privacy toggle ordering in #367.
AprilNEA
pushed a commit
that referenced
this pull request
Aug 12, 2026
* fix(ui): slide the detail tab indicator instead of morphing glass The detail tab bar played an exaggerated stretch on every tab switch, worst on the four-tab container view, and the first switch after a cold launch was worse still. GlassEffectContainer's spacing is a blend threshold, not a layout gap: the higher it is, the sooner shapes start merging as they approach. It was set to tabCount * detailTabSegment -- 320pt for containers, 480pt for sandboxes -- against an HStack spacing of 2pt, so the blend field spanned the whole bar. Apple documents that a container spacing larger than the interior stack's spacing blends the effects together at rest. The indicator also toggled glassEffect between .regular and .identity per segment under one shared glassEffectID. That ID stays constant while the segment beneath it changes, which is the documented trigger for GlassEffectTransition.matchedGeometry to apply "additional scale and offset effects to content" -- on top of the oversized blend. Render one persistent glass capsule instead and position it with matchedGeometryEffect against the selected segment. The pill now moves rather than appearing and disappearing, so there is no morph to overshoot and no appearance transition to misfire on first mount. GlassEffectContainer is no longer needed for a single effect. * chore(release): curate the 1.35.0 highlights The 1.35.0 release PR merged with an empty `### Highlights` section, so `cargo xtask release notes` now fails on every PR against master. That step runs before Lint, Build and Test, so no PR gets as far as compiling. The release DMG extracts the same section to feed Sparkle's update dialog and refuses to build without it, so 1.35.0 cannot be packaged either. Write the section from what 1.35.0 actually shipped: the Fleet device auth, enrollment and runner management in #320, the authoritative sandbox port reconciliation in #374, and the telemetry identification and Privacy toggle ordering in #367.
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
1 participant
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.
Why
PostHog has been connected and sending since the Info.plist injection fix (#246) — the shipped 1.34.0 bundle carries a real
PostHogAPIKey, and the SDK's local queue drains againstus.i.posthog.com. But what it was sending was close to useless:Analytics.Eventhad no call site. Only startup, error, slow-call, and diagnostic events ever fired. No container, image, Kubernetes, terminal, or settings usage was visible at all.identify()was never called, so withpersonProfiles = .identifiedOnlyPostHog built no person profiles.error_occurredvolume couldn't be attributed to a hypervisor backend, release channel, or environment.What changed
1. Instrument the dead events (
0390d10)Every event in the catalog now has a call site.
container_started/stopped/removedcontainer_createdContainersViewModel+GRPC(Docker API path)image_pulled/image_removedImagesViewModel+Dockerk8s_enabled/k8s_disabledKubernetesStateterminal_openedsettings_openedApplicationCoordinator.showSettingsConventions:
error_occurredviaErrorReporting, so double-reporting would make success rates uncomputable.backend(grpc/docker) andbatch, keeping the two engine paths and bulk actions separable.terminal_openedfires where the session actually reaches.connected, not at the open request — otherwise "Docker CLI not found" would count as a terminal open.app_launchedis dropped: the SDK'scaptureApplicationLifecycleEventsalready emits Application Installed/Opened/Backgrounded on macOS (ApplicationLifecyclePublisher.swift:78).2. Identity and super properties (
f1f9b83)Identity mirrors
AuthSessionfrom the coordinator viawithObservationTracking, following the existing daemon/startup idiom —ArcBoxAuthstays free of any analytics dependency. distinct_id is the OIDC subject;email,name, andemail_verifiedbecome person properties. Sign-out callsreset(). Re-identify is keyed on the wholeAuthIdentitybecauseloadUserInfo()enriches it after sign-in.Super properties:
arcbox_profileandupdate_channelat setup,system_vm_backendfromSystemVmBackendModel(funnelled through one setter so the three assignment sites can't drift),k8s_activefromKubernetesState.$app_version,$os_version, and$device_typecome from the SDK already.This also fixes a race: the Privacy toggle's opt-in/opt-out moves from
GeneralSettingsViewintodefaultsDidChange. Both used to be driven by the same UserDefaults write with no ordering guarantee, and the SDK dropsidentifywhile opted out — so a user who signed in before enabling telemetry would have stayed anonymous forever.3. Opt-out precedence (
5647d0d)setup()lets the SDK's own persisted.optOutflag overrideconfig.optOut(PostHogSDK.swift:128-130), so the value derived fromtelemetryEnabledwas only a fallback for a fresh install. Once the two diverged, PostHog's copy silently won on every launch. The preference is now restated right after setup.reset()also deletes the registered-properties key (PostHogStorage.swift:403), which dropped all four super properties from every event for the rest of a session after sign-out. They describe the install, not the person, soAnalyticskeeps its own mirror and re-registers.Privacy — needs a second opinion
The toggle previously read "Share anonymous usage data … No personal data is collected." That is no longer true once an account is linked, so it now reads:
The wording should be reviewed, and the privacy policy needs PostHog listed as a processor along with the
email/namefields. That part is outside this PR.Worth stating explicitly since it's easy to misread: signing in was never a gate on collection.
capturedoes not require person processing (PostHogSDK.swift:906-912), so events from users who never sign in were always collected as anonymous events. Signing in only adds a person profile.One migration note: users who turned telemetry off in an older build but whose
telemetryEnableddefault readstruewill flip back to opted in. The two should never have diverged — the toggle always wrote both — but the alternative (letting the SDK's persisted value win and back-filling UserDefaults) is a real option if stale state is a concern.Verification
make lint— 0 violations in 285 filesmake test— 227 tests, 0 failuresmake buildin both Debug and Release. This mattered: the branch that actually sends telemetry sits behind#if !DEBUG, so a Debug-only build never type-checks it.Analytics.swift.No files added or removed, so
project.pbxprojis untouched and xcodegen does not need to re-run.No tests
These are straight-line
capture/registercalls with no branching or transformation, against thePostHogSDK.sharedsingleton with no injection seam — tests would only restate the implementation. The one piece with real logic,syncAnalyticsIdentity's dedupe, depends on a live@MainActor AuthSession; making it testable means givingAuthSessiona test double, which is an auth-package refactor rather than part of this change. Happy to do that separately if it's wanted.