fix(analytics): address review findings on activation-funnel events

- app_opened now also fires on the consent-grant transition (modal Allow /
  Settings toggle), not just cold start with prior consent — the true first
  session was emitting nothing and session 2 got mislabeled is_first_open.
  trackAppOpened() is guarded once-per-JS-session so revoke->regrant cannot
  double-count.
- testConnection() takes a source ('onboarding' | 'edit_test') carried on
  connection_attempted/succeeded/failed so the funnel can filter out the
  edit screen's repeat-tester noise.
- Aborted runs no longer count: abortedSessions set (in sessions.ts, read by
  events.ts which already imports it — no new import cycle), marked after a
  successful abort call, cleared on busy, and checked on busy->idle for BOTH
  response_received and recordSuccessfulSession().
- Consent revocation now DROPS buffered events instead of flushing them:
  PostHog's optOut() only blocks new captures and shutdown() drains the queue
  over the network, so ConsentGatedPostHog overrides the public fetch()
  transport to answer with a synthetic 200 post-revoke — shutdown clears the
  persisted queue and timers with zero bytes leaving the device. Re-grant
  calls optIn() to clear the persisted SDK opt-out flag.
- classifyConnectionError extracted to pure analytics-classify.ts with
  node --test coverage (same pattern as store-review-policy).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJKAQ6HAikWGQK7PGZ5Y4E
This commit is contained in:
engineer
2026-07-16 16:04:54 -07:00
parent 027c529ce5
commit c3cac2b8e5
9 changed files with 195 additions and 48 deletions

View File

@@ -84,6 +84,7 @@ export default function EditConnectionScreen() {
directory: directory.trim() || undefined, directory: directory.trim() || undefined,
username: username.trim() || undefined, username: username.trim() || undefined,
}, },
"edit_test",
password || undefined, password || undefined,
) )

View File

@@ -80,6 +80,7 @@ export default function AddConnectionScreen() {
url: serverUrl, url: serverUrl,
username: username.trim() || undefined, username: username.trim() || undefined,
}, },
"onboarding",
password || undefined, password || undefined,
) )
@@ -133,7 +134,7 @@ export default function AddConnectionScreen() {
// Advanced mode saves directly without a pre-flight health check (see // Advanced mode saves directly without a pre-flight health check (see
// useConnections.addConnection), so unlike quick-connect there is no // useConnections.addConnection), so unlike quick-connect there is no
// success/failure signal to report here — only that an attempt was made. // success/failure signal to report here — only that an attempt was made.
track(AnalyticsEvent.ConnectionAttempted) track(AnalyticsEvent.ConnectionAttempted, { source: "onboarding" })
setIsConnecting(true) setIsConnecting(true)
await addConnection( await addConnection(
{ {

View File

@@ -0,0 +1,49 @@
import { test } from "node:test"
import assert from "node:assert/strict"
import { classifyConnectionError } from "./analytics-classify.ts"
test("401 / unauthorized errors -> unauthorized (the known connect bug)", () => {
assert.equal(classifyConnectionError("API Error: 401 - Unauthorized"), "unauthorized")
assert.equal(classifyConnectionError("401"), "unauthorized")
assert.equal(classifyConnectionError("Request failed: unauthorized"), "unauthorized")
assert.equal(classifyConnectionError("HTTP 401 Unauthorised"), "unauthorized")
})
test("TLS / certificate errors -> tls-error", () => {
assert.equal(classifyConnectionError("SSL handshake failed"), "tls-error")
assert.equal(classifyConnectionError("certificate verify failed"), "tls-error")
assert.equal(classifyConnectionError("TLS connection error"), "tls-error")
})
test("timeouts -> timeout", () => {
assert.equal(classifyConnectionError("timeout after 8000ms"), "timeout")
assert.equal(classifyConnectionError("Connection timed out"), "timeout")
})
test("network-level failures -> server-unreachable", () => {
assert.equal(classifyConnectionError("Network request failed"), "server-unreachable")
assert.equal(classifyConnectionError("connect ECONNREFUSED 192.0.2.1:4096"), "server-unreachable")
assert.equal(classifyConnectionError("host unreachable"), "server-unreachable")
assert.equal(classifyConnectionError("fetch failed"), "server-unreachable")
})
test("URL parse failures -> malformed-url", () => {
assert.equal(classifyConnectionError("Malformed URL"), "malformed-url")
assert.equal(classifyConnectionError("Invalid URL: htp:/oops"), "malformed-url")
})
test("anything else (or missing) -> unknown", () => {
assert.equal(classifyConnectionError("some new error"), "unknown")
assert.equal(classifyConnectionError(""), "unknown")
assert.equal(classifyConnectionError(undefined), "unknown")
})
test("classification is case-insensitive", () => {
assert.equal(classifyConnectionError("UNAUTHORIZED"), "unauthorized")
assert.equal(classifyConnectionError("TIMEOUT"), "timeout")
})
test("precedence: 401 wins over other matches in a combined message", () => {
// A 401 behind a TLS proxy should surface as auth, the actionable bucket.
assert.equal(classifyConnectionError("401 Unauthorized (tls terminated)"), "unauthorized")
})

View File

@@ -0,0 +1,28 @@
// Pure connection-failure classification for analytics, extracted from
// analytics.ts (which imports posthog/expo) so it is unit-testable under
// plain `node --test` — same pattern as diagnostics-classify.ts and
// store-review-policy.ts.
/** Coarse, non-identifying failure buckets — never include the raw error string
* (it may embed hostnames/tokens/paths). Reuses the vocabulary already
* established by diagnostics-classify.ts's Classification type. */
export type ConnectionErrorClass =
| "malformed-url"
| "no-internet"
| "server-unreachable"
| "unauthorized"
| "tls-error"
| "timeout"
| "unknown"
/** Classify a connection failure into a coarse bucket without leaking the
* raw error message (which can contain hostnames/IPs). */
export function classifyConnectionError(message: string | undefined): ConnectionErrorClass {
const m = (message || "").toLowerCase()
if (/401|unauthoriz/.test(m)) return "unauthorized"
if (/ssl|tls|certificate|handshake/.test(m)) return "tls-error"
if (/timeout|timed out/.test(m)) return "timeout"
if (/network request failed|unreachable|econnrefused|fetch failed/.test(m)) return "server-unreachable"
if (/malformed|invalid url/.test(m)) return "malformed-url"
return "unknown"
}

View File

@@ -6,7 +6,16 @@
// module never calls PostHog.init/capture on its own; it is only ever // module never calls PostHog.init/capture on its own; it is only ever
// driven by ./telemetry.ts, which gates BOTH Sentry and analytics behind // driven by ./telemetry.ts, which gates BOTH Sentry and analytics behind
// the exact same "opencode_telemetry_consent" flag. // the exact same "opencode_telemetry_consent" flag.
// 3. No PII in event properties: never pass server URLs, tokens, prompts, // 3. On consent REVOCATION, buffered-but-unsent events are DROPPED, not
// flushed: PostHog's shutdown() normally drains the queue over the
// network, and optOut() only blocks NEW captures (already-queued events
// would still be sent by the next flush). So ConsentGatedPostHog
// overrides the client's public fetch() transport; once revoked it
// answers every SDK request with a synthetic 200 without touching the
// network. shutdown() then "drains" the queue into that stub — clearing
// the persisted queue and stopping timers — while zero bytes leave the
// device.
// 4. No PII in event properties: never pass server URLs, tokens, prompts,
// or file contents. Only coarse, enumerated event names + small typed // or file contents. Only coarse, enumerated event names + small typed
// properties (booleans, enums, counts). // properties (booleans, enums, counts).
// //
@@ -18,6 +27,8 @@ import PostHog from "posthog-react-native"
import * as SecureStore from "expo-secure-store" import * as SecureStore from "expo-secure-store"
import { log } from "./logbuffer" import { log } from "./logbuffer"
export { classifyConnectionError, type ConnectionErrorClass } from "./analytics-classify"
const API_KEY = process.env.EXPO_PUBLIC_POSTHOG_KEY const API_KEY = process.env.EXPO_PUBLIC_POSTHOG_KEY
// EU by default (GDPR-friendly region for opencode's mostly-EU/self-hosted user base). // EU by default (GDPR-friendly region for opencode's mostly-EU/self-hosted user base).
// Override with EXPO_PUBLIC_POSTHOG_HOST for a self-hosted instance. // Override with EXPO_PUBLIC_POSTHOG_HOST for a self-hosted instance.
@@ -25,25 +36,41 @@ const HOST = process.env.EXPO_PUBLIC_POSTHOG_HOST || "https://eu.i.posthog.com"
const FIRST_OPEN_KEY = "opencode_analytics_first_open_done" const FIRST_OPEN_KEY = "opencode_analytics_first_open_done"
let client: PostHog | null = null // When true (set on consent revocation), the transport answers with a
let enabled = false // synthetic 200 instead of hitting the network, so queued events are
// discarded rather than uploaded. Reset when a new client is created.
let dropNetwork = false
/** Coarse, non-identifying failure buckets — never include the raw error string /** PostHog client whose transport is consent-gated: after revocation every
* (it may embed hostnames/tokens/paths). Reuses the vocabulary already * request short-circuits to a fake success so nothing reaches the network.
* established by diagnostics-classify.ts's Classification type. */ * (Types derived from the base class to avoid importing the transitive
export type ConnectionErrorClass = * @posthog/core package directly.) */
| "malformed-url" class ConsentGatedPostHog extends PostHog {
| "no-internet" fetch(url: string, options: Parameters<PostHog["fetch"]>[1]): ReturnType<PostHog["fetch"]> {
| "server-unreachable" if (dropNetwork) {
| "unauthorized" return Promise.resolve({
| "tls-error" status: 200,
| "timeout" text: async () => "",
| "unknown" json: async () => ({ status: 1 }),
})
}
return super.fetch(url, options)
}
}
let client: ConsentGatedPostHog | null = null
let enabled = false
// app_opened must fire at most once per JS session, whichever path enables
// analytics first (cold start with prior consent, or the consent modal /
// Settings toggle mid-session). Also prevents a revoke -> re-grant in the
// same session from double-counting.
let appOpenedTracked = false
/** Activation-funnel events. Keep this list in 1:1 sync with the funnel steps /** Activation-funnel events. Keep this list in 1:1 sync with the funnel steps
* tracked in the product analytics dashboard. */ * tracked in the product analytics dashboard. */
export enum AnalyticsEvent { export enum AnalyticsEvent {
/** App process started and the user has an existing telemetry decision of "granted". */ /** Fired once per app session, as soon as analytics is enabled (either at
* cold start with prior consent, or right after consent is granted). */
AppOpened = "app_opened", AppOpened = "app_opened",
/** User tapped Connect/Save with a non-empty server URL (quick or advanced mode). */ /** User tapped Connect/Save with a non-empty server URL (quick or advanced mode). */
ConnectionFormSubmitted = "connection_form_submitted", ConnectionFormSubmitted = "connection_form_submitted",
@@ -55,10 +82,17 @@ export enum AnalyticsEvent {
ConnectionFailed = "connection_failed", ConnectionFailed = "connection_failed",
/** User sent a prompt/message to an agent session (excludes slash commands). */ /** User sent a prompt/message to an agent session (excludes slash commands). */
MessageSent = "message_sent", MessageSent = "message_sent",
/** An agent response finished streaming (session transitioned busy -> idle). */ /** An agent response finished streaming (session transitioned busy -> idle),
* excluding user-aborted runs. */
ResponseReceived = "response_received", ResponseReceived = "response_received",
} }
/** Where a connection test was initiated from. The activation funnel filters
* to source=onboarding; edit_test covers the Test button on the existing-
* connection edit screen, which would otherwise pollute the funnel with
* repeat-tester noise. */
export type ConnectionTestSource = "onboarding" | "edit_test"
export function initAnalytics() { export function initAnalytics() {
if (enabled) return if (enabled) return
if (!API_KEY) { if (!API_KEY) {
@@ -66,11 +100,15 @@ export function initAnalytics() {
return return
} }
try { try {
client = new PostHog(API_KEY, { dropNetwork = false
client = new ConsentGatedPostHog(API_KEY, {
host: HOST, host: HOST,
// We call track() explicitly at each funnel step — no implicit capture. // We call track() explicitly at each funnel step — no implicit capture.
captureAppLifecycleEvents: false, captureAppLifecycleEvents: false,
}) })
// A previous revoke persisted the SDK-level opt-out flag; clear it so the
// re-granted client can enqueue again. No-op on a fresh install.
void client.optIn()
enabled = true enabled = true
log.info("analytics", "initialized", `host=${HOST}`) log.info("analytics", "initialized", `host=${HOST}`)
} catch (e) { } catch (e) {
@@ -78,17 +116,24 @@ export function initAnalytics() {
} }
} }
/** Consent revoked: block new captures, DROP anything buffered (see header
* note 3 — the gated fetch turns shutdown's drain into a no-network discard),
* and tear the client down. */
export async function shutdownAnalytics() { export async function shutdownAnalytics() {
if (!enabled || !client) return if (!enabled || !client) return
enabled = false enabled = false
const c = client const c = client
client = null client = null
dropNetwork = true
try { try {
// Persist SDK-level opt-out first so even a re-created client (without
// consent) could not capture, then let shutdown clear queue + timers.
await c.optOut()
await c.shutdown() await c.shutdown()
} catch (e) { } catch (e) {
log.warn("analytics", "shutdown failed", String(e)) log.warn("analytics", "shutdown failed", String(e))
} }
log.info("analytics", "disabled by user") log.info("analytics", "disabled by user — buffered events dropped")
} }
export function analyticsEnabled(): boolean { export function analyticsEnabled(): boolean {
@@ -110,11 +155,15 @@ export function track(event: AnalyticsEvent, props?: AnalyticsProps) {
} }
} }
/** Fire AppOpened with `is_first_open`. The "seen before" flag is only ever /** Fire AppOpened with `is_first_open`, at most once per JS session.
* read/written once consent is granted (this function is itself a no-op * Called both from app start (consent already granted) and from the
* without consent), so nothing is recorded locally pre-consent either. */ * consent-grant transition (modal "Allow" / Settings toggle) — the session
* guard makes whichever happens first win. The "seen before" flag is only
* ever read/written once consent is granted (this function is itself a
* no-op without consent), so nothing is recorded locally pre-consent. */
export async function trackAppOpened() { export async function trackAppOpened() {
if (!enabled) return if (!enabled || appOpenedTracked) return
appOpenedTracked = true
let isFirstOpen = false let isFirstOpen = false
try { try {
const seen = await SecureStore.getItemAsync(FIRST_OPEN_KEY) const seen = await SecureStore.getItemAsync(FIRST_OPEN_KEY)
@@ -125,15 +174,3 @@ export async function trackAppOpened() {
} }
track(AnalyticsEvent.AppOpened, { is_first_open: isFirstOpen }) track(AnalyticsEvent.AppOpened, { is_first_open: isFirstOpen })
} }
/** Classify a connection failure into a coarse bucket without leaking the
* raw error message (which can contain hostnames/IPs). */
export function classifyConnectionError(message: string | undefined): ConnectionErrorClass {
const m = (message || "").toLowerCase()
if (/401|unauthoriz/.test(m)) return "unauthorized"
if (/ssl|tls|certificate|handshake/.test(m)) return "tls-error"
if (/timeout|timed out/.test(m)) return "timeout"
if (/network request failed|unreachable|econnrefused|fetch failed/.test(m)) return "server-unreachable"
if (/malformed|invalid url/.test(m)) return "malformed-url"
return "unknown"
}

View File

@@ -23,7 +23,7 @@
import * as SecureStore from "expo-secure-store" import * as SecureStore from "expo-secure-store"
import { disableSentry, initSentry, sentryEnabled } from "./sentry" import { disableSentry, initSentry, sentryEnabled } from "./sentry"
import { initAnalytics, shutdownAnalytics, analyticsEnabled } from "./analytics" import { initAnalytics, shutdownAnalytics, analyticsEnabled, trackAppOpened } from "./analytics"
const CONSENT_KEY = "opencode_telemetry_consent" const CONSENT_KEY = "opencode_telemetry_consent"
@@ -82,6 +82,13 @@ async function applyTelemetryConsent(granted: boolean): Promise<void> {
_resolved = true _resolved = true
if (!sentryEnabled()) initSentry() if (!sentryEnabled()) initSentry()
if (!analyticsEnabled()) initAnalytics() if (!analyticsEnabled()) initAnalytics()
// First-ever session reaches here via the consent modal's "Allow" (app
// start skipped init because consent was still unknown), so app_opened
// must also fire on the grant transition — otherwise the true first
// session emits nothing and session 2 gets mislabeled is_first_open.
// trackAppOpened() is internally once-per-session, so a mid-session
// revoke -> re-grant cannot double-count.
void trackAppOpened()
return return
} }

View File

@@ -4,7 +4,7 @@ import * as Crypto from "expo-crypto"
import type { ServerConnection, ConnectionType } from "../lib/types" import type { ServerConnection, ConnectionType } from "../lib/types"
import { createClient, type Client, type Project } from "../lib/sdk" import { createClient, type Client, type Project } from "../lib/sdk"
import { addBreadcrumb } from "../lib/sentry" import { addBreadcrumb } from "../lib/sentry"
import { AnalyticsEvent, classifyConnectionError, track } from "../lib/analytics" import { AnalyticsEvent, classifyConnectionError, track, type ConnectionTestSource } from "../lib/analytics"
import { buildAuth } from "../lib/auth" import { buildAuth } from "../lib/auth"
const CONNECTIONS_KEY = "opencode_connections" const CONNECTIONS_KEY = "opencode_connections"
@@ -34,7 +34,13 @@ interface ConnectionsState {
addConnection: (connection: Omit<ServerConnection, "id">, password?: string) => Promise<void> addConnection: (connection: Omit<ServerConnection, "id">, password?: string) => Promise<void>
removeConnection: (id: string) => Promise<void> removeConnection: (id: string) => Promise<void>
setActiveConnection: (id: string) => Promise<void> setActiveConnection: (id: string) => Promise<void>
testConnection: (connection: ServerConnection, password?: string) => Promise<{ ok: boolean; error?: string }> // `source` distinguishes the activation funnel (onboarding) from the edit
// screen's Test button (edit_test) in analytics.
testConnection: (
connection: ServerConnection,
source: ConnectionTestSource,
password?: string,
) => Promise<{ ok: boolean; error?: string }>
updateConnection: (id: string, updates: Partial<ServerConnection>) => Promise<void> updateConnection: (id: string, updates: Partial<ServerConnection>) => Promise<void>
refreshProject: () => Promise<void> refreshProject: () => Promise<void>
// Create a one-off client pointing at a specific directory (for cross-project operations) // Create a one-off client pointing at a specific directory (for cross-project operations)
@@ -243,8 +249,8 @@ export const useConnections = create<ConnectionsState>((set, get) => ({
}) })
}, },
testConnection: async (connection, password) => { testConnection: async (connection, source, password) => {
track(AnalyticsEvent.ConnectionAttempted) track(AnalyticsEvent.ConnectionAttempted, { source })
try { try {
const client = createClient({ const client = createClient({
baseUrl: connection.url, baseUrl: connection.url,
@@ -253,11 +259,11 @@ export const useConnections = create<ConnectionsState>((set, get) => ({
}) })
await client.global.health() await client.global.health()
track(AnalyticsEvent.ConnectionSucceeded) track(AnalyticsEvent.ConnectionSucceeded, { source })
return { ok: true } return { ok: true }
} catch (error) { } catch (error) {
const message = error instanceof Error ? error.message : String(error) const message = error instanceof Error ? error.message : String(error)
track(AnalyticsEvent.ConnectionFailed, { error_class: classifyConnectionError(message) }) track(AnalyticsEvent.ConnectionFailed, { source, error_class: classifyConnectionError(message) })
return { ok: false, error: message } return { ok: false, error: message }
} }
}, },

View File

@@ -1,6 +1,6 @@
import { create } from "zustand" import { create } from "zustand"
import { useConnections } from "./connections" import { useConnections } from "./connections"
import { useSessions } from "./sessions" import { useSessions, abortedSessions } from "./sessions"
import { send as notify } from "../lib/notifications" import { send as notify } from "../lib/notifications"
import { sanitizeBody } from "../lib/notify-format" import { sanitizeBody } from "../lib/notify-format"
import { statusFromPart } from "../lib/status-labels" import { statusFromPart } from "../lib/status-labels"
@@ -168,8 +168,11 @@ export const useEvents = create<EventsState>((set, get) => ({
const previous = get().sessionStatus[sessionID] const previous = get().sessionStatus[sessionID]
const completed = previous?.type === "busy" && status.type === "idle" const completed = previous?.type === "busy" && status.type === "idle"
// A new run starts — forget any error from the previous one // A new run starts — forget any error/abort from the previous one
if (status.type === "busy") erroredSessions.delete(sessionID) if (status.type === "busy") {
erroredSessions.delete(sessionID)
abortedSessions.delete(sessionID)
}
set((state) => ({ set((state) => ({
sessionStatus: { ...state.sessionStatus, [sessionID]: status }, sessionStatus: { ...state.sessionStatus, [sessionID]: status },
@@ -190,7 +193,10 @@ export const useEvents = create<EventsState>((set, get) => ({
} }
if (completed) { if (completed) {
track(AnalyticsEvent.ResponseReceived) // A user-cancelled run still ends busy -> idle; don't count it
// as a received response or a review-worthy success.
const aborted = abortedSessions.has(sessionID)
if (!aborted) track(AnalyticsEvent.ResponseReceived)
const match = useSessions.getState().sessions.find((s) => s.id === sessionID) const match = useSessions.getState().sessions.find((s) => s.id === sessionID)
notify({ notify({
category: "completed", category: "completed",
@@ -201,8 +207,8 @@ export const useEvents = create<EventsState>((set, get) => ({
// Genuinely positive moment — count it toward the one-time // Genuinely positive moment — count it toward the one-time
// store review prompt, but only if this run never errored // store review prompt, but only if this run never errored
// (session.error doesn't touch sessionStatus, so an errored // (session.error doesn't touch sessionStatus, so an errored
// session still lands here via busy -> idle). // session still lands here via busy -> idle) and wasn't aborted.
if (!erroredSessions.has(sessionID)) void recordSuccessfulSession() if (!aborted && !erroredSessions.has(sessionID)) void recordSuccessfulSession()
} }
break break
} }
@@ -378,6 +384,7 @@ export const useEvents = create<EventsState>((set, get) => ({
controller?.abort() controller?.abort()
controller = null controller = null
erroredSessions.clear() erroredSessions.clear()
abortedSessions.clear()
set({ set({
connected: false, connected: false,
reconnectAttempts: 0, reconnectAttempts: 0,

View File

@@ -53,6 +53,14 @@ interface SessionsState {
handleEvent: (event: Event) => void handleEvent: (event: Event) => void
} }
// Sessions the user aborted since they last went busy. Mirrors events.ts's
// erroredSessions: SessionStatus has no "aborted" variant — an aborted run
// still ends with a busy -> idle transition — so without this mark a
// user-cancelled run would count as response_received in analytics and as a
// success toward the store review prompt. events.ts (which already imports
// this module) clears entries on busy and checks them on busy -> idle.
export const abortedSessions = new Set<string>()
// Get the right client for a session's directory // Get the right client for a session's directory
function clientFor(directory?: string): Client | null { function clientFor(directory?: string): Client | null {
const connState = useConnections.getState() const connState = useConnections.getState()
@@ -310,6 +318,9 @@ export const useSessions = create<SessionsState>((set, get) => ({
try { try {
await client.session.abort(session.id) await client.session.abort(session.id)
// Mark only after the abort request succeeded — if it failed, the run
// continues and any eventual completion is a genuine response.
abortedSessions.add(session.id)
set((state) => ({ sending: { ...state.sending, [session.id]: false } })) set((state) => ({ sending: { ...state.sending, [session.id]: false } }))
} catch { } catch {
set({ error: "Failed to abort session" }) set({ error: "Failed to abort session" })