diff --git a/app/connection/[id].tsx b/app/connection/[id].tsx index 4328056..c873e52 100644 --- a/app/connection/[id].tsx +++ b/app/connection/[id].tsx @@ -84,6 +84,7 @@ export default function EditConnectionScreen() { directory: directory.trim() || undefined, username: username.trim() || undefined, }, + "edit_test", password || undefined, ) diff --git a/app/connection/add.tsx b/app/connection/add.tsx index 064de6a..c8d9bb3 100644 --- a/app/connection/add.tsx +++ b/app/connection/add.tsx @@ -80,6 +80,7 @@ export default function AddConnectionScreen() { url: serverUrl, username: username.trim() || undefined, }, + "onboarding", password || undefined, ) @@ -133,7 +134,7 @@ export default function AddConnectionScreen() { // Advanced mode saves directly without a pre-flight health check (see // useConnections.addConnection), so unlike quick-connect there is no // success/failure signal to report here — only that an attempt was made. - track(AnalyticsEvent.ConnectionAttempted) + track(AnalyticsEvent.ConnectionAttempted, { source: "onboarding" }) setIsConnecting(true) await addConnection( { diff --git a/src/lib/analytics-classify.test.ts b/src/lib/analytics-classify.test.ts new file mode 100644 index 0000000..e5a69a9 --- /dev/null +++ b/src/lib/analytics-classify.test.ts @@ -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") +}) diff --git a/src/lib/analytics-classify.ts b/src/lib/analytics-classify.ts new file mode 100644 index 0000000..f05ad81 --- /dev/null +++ b/src/lib/analytics-classify.ts @@ -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" +} diff --git a/src/lib/analytics.ts b/src/lib/analytics.ts index 6325c51..4e4e106 100644 --- a/src/lib/analytics.ts +++ b/src/lib/analytics.ts @@ -6,7 +6,16 @@ // module never calls PostHog.init/capture on its own; it is only ever // driven by ./telemetry.ts, which gates BOTH Sentry and analytics behind // 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 // properties (booleans, enums, counts). // @@ -18,6 +27,8 @@ import PostHog from "posthog-react-native" import * as SecureStore from "expo-secure-store" import { log } from "./logbuffer" +export { classifyConnectionError, type ConnectionErrorClass } from "./analytics-classify" + const API_KEY = process.env.EXPO_PUBLIC_POSTHOG_KEY // 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. @@ -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" -let client: PostHog | null = null -let enabled = false +// When true (set on consent revocation), the transport answers with a +// 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 - * (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" +/** PostHog client whose transport is consent-gated: after revocation every + * request short-circuits to a fake success so nothing reaches the network. + * (Types derived from the base class to avoid importing the transitive + * @posthog/core package directly.) */ +class ConsentGatedPostHog extends PostHog { + fetch(url: string, options: Parameters[1]): ReturnType { + if (dropNetwork) { + return Promise.resolve({ + status: 200, + text: async () => "", + 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 * tracked in the product analytics dashboard. */ 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", /** User tapped Connect/Save with a non-empty server URL (quick or advanced mode). */ ConnectionFormSubmitted = "connection_form_submitted", @@ -55,10 +82,17 @@ export enum AnalyticsEvent { ConnectionFailed = "connection_failed", /** User sent a prompt/message to an agent session (excludes slash commands). */ 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", } +/** 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() { if (enabled) return if (!API_KEY) { @@ -66,11 +100,15 @@ export function initAnalytics() { return } try { - client = new PostHog(API_KEY, { + dropNetwork = false + client = new ConsentGatedPostHog(API_KEY, { host: HOST, // We call track() explicitly at each funnel step — no implicit capture. 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 log.info("analytics", "initialized", `host=${HOST}`) } 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() { if (!enabled || !client) return enabled = false const c = client client = null + dropNetwork = true 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() } catch (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 { @@ -110,11 +155,15 @@ export function track(event: AnalyticsEvent, props?: AnalyticsProps) { } } -/** Fire AppOpened with `is_first_open`. 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 either. */ +/** Fire AppOpened with `is_first_open`, at most once per JS session. + * Called both from app start (consent already granted) and from the + * 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() { - if (!enabled) return + if (!enabled || appOpenedTracked) return + appOpenedTracked = true let isFirstOpen = false try { const seen = await SecureStore.getItemAsync(FIRST_OPEN_KEY) @@ -125,15 +174,3 @@ export async function trackAppOpened() { } 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" -} diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 2f0e7e8..fdd335b 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -23,7 +23,7 @@ import * as SecureStore from "expo-secure-store" 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" @@ -82,6 +82,13 @@ async function applyTelemetryConsent(granted: boolean): Promise { _resolved = true if (!sentryEnabled()) initSentry() 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 } diff --git a/src/stores/connections.ts b/src/stores/connections.ts index f107113..ef3b12d 100644 --- a/src/stores/connections.ts +++ b/src/stores/connections.ts @@ -4,7 +4,7 @@ import * as Crypto from "expo-crypto" import type { ServerConnection, ConnectionType } from "../lib/types" import { createClient, type Client, type Project } from "../lib/sdk" 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" const CONNECTIONS_KEY = "opencode_connections" @@ -34,7 +34,13 @@ interface ConnectionsState { addConnection: (connection: Omit, password?: string) => Promise removeConnection: (id: string) => Promise setActiveConnection: (id: string) => Promise - 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) => Promise refreshProject: () => Promise // Create a one-off client pointing at a specific directory (for cross-project operations) @@ -243,8 +249,8 @@ export const useConnections = create((set, get) => ({ }) }, - testConnection: async (connection, password) => { - track(AnalyticsEvent.ConnectionAttempted) + testConnection: async (connection, source, password) => { + track(AnalyticsEvent.ConnectionAttempted, { source }) try { const client = createClient({ baseUrl: connection.url, @@ -253,11 +259,11 @@ export const useConnections = create((set, get) => ({ }) await client.global.health() - track(AnalyticsEvent.ConnectionSucceeded) + track(AnalyticsEvent.ConnectionSucceeded, { source }) return { ok: true } } catch (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 } } }, diff --git a/src/stores/events.ts b/src/stores/events.ts index 0ee33e4..875508c 100644 --- a/src/stores/events.ts +++ b/src/stores/events.ts @@ -1,6 +1,6 @@ import { create } from "zustand" import { useConnections } from "./connections" -import { useSessions } from "./sessions" +import { useSessions, abortedSessions } from "./sessions" import { send as notify } from "../lib/notifications" import { sanitizeBody } from "../lib/notify-format" import { statusFromPart } from "../lib/status-labels" @@ -168,8 +168,11 @@ export const useEvents = create((set, get) => ({ const previous = get().sessionStatus[sessionID] const completed = previous?.type === "busy" && status.type === "idle" - // A new run starts — forget any error from the previous one - if (status.type === "busy") erroredSessions.delete(sessionID) + // A new run starts — forget any error/abort from the previous one + if (status.type === "busy") { + erroredSessions.delete(sessionID) + abortedSessions.delete(sessionID) + } set((state) => ({ sessionStatus: { ...state.sessionStatus, [sessionID]: status }, @@ -190,7 +193,10 @@ export const useEvents = create((set, get) => ({ } 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) notify({ category: "completed", @@ -201,8 +207,8 @@ export const useEvents = create((set, get) => ({ // Genuinely positive moment — count it toward the one-time // store review prompt, but only if this run never errored // (session.error doesn't touch sessionStatus, so an errored - // session still lands here via busy -> idle). - if (!erroredSessions.has(sessionID)) void recordSuccessfulSession() + // session still lands here via busy -> idle) and wasn't aborted. + if (!aborted && !erroredSessions.has(sessionID)) void recordSuccessfulSession() } break } @@ -378,6 +384,7 @@ export const useEvents = create((set, get) => ({ controller?.abort() controller = null erroredSessions.clear() + abortedSessions.clear() set({ connected: false, reconnectAttempts: 0, diff --git a/src/stores/sessions.ts b/src/stores/sessions.ts index cbdc651..aee117c 100644 --- a/src/stores/sessions.ts +++ b/src/stores/sessions.ts @@ -53,6 +53,14 @@ interface SessionsState { 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() + // Get the right client for a session's directory function clientFor(directory?: string): Client | null { const connState = useConnections.getState() @@ -310,6 +318,9 @@ export const useSessions = create((set, get) => ({ try { 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 } })) } catch { set({ error: "Failed to abort session" })