From bd248915a7ce8298cf869e73e60e3bf112c96ffa Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Thu, 30 Jul 2026 12:27:10 -0400 Subject: [PATCH] fix: identify PostHog users with Cloud identity (#16087) Co-authored-by: neubig Co-authored-by: openhands --- AGENTS.md | 4 +- .../contexts/active-backend-context.test.tsx | 139 ++++++++++++++++ ...-sync-automation-telemetry-consent.test.ts | 12 ++ .../hooks/use-telemetry-identity.test.ts | 26 +++ .../services/telemetry-bootstrap.test.ts | 4 +- __tests__/services/telemetry.test.ts | 106 ++++++++++-- .../automation-service.api.test.ts | 6 + .../automation-service.api.ts | 4 + src/api/backend-registry/storage.ts | 6 +- src/api/backend-registry/types.ts | 2 + src/contexts/active-backend-context.tsx | 81 ++++++++-- src/hooks/query/use-cloud-current-user-id.ts | 21 ++- src/hooks/query/use-cloud-organizations.ts | 6 +- src/hooks/use-can-manage-org-profiles.ts | 12 +- .../use-sync-automation-telemetry-consent.ts | 18 ++- src/hooks/use-telemetry-identity.ts | 8 +- src/services/telemetry.ts | 152 +++++++++++++++++- 17 files changed, 556 insertions(+), 51 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9e5dd80c9c..328e97b847 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,13 +31,13 @@ by a human, report the exact validator error rather than editing it yourself. One Canvas-owned PostHog client owns telemetry and app analytics. -- `src/services/telemetry.ts` is the only module that accesses the named `agent-canvas` PostHog client. The name isolates Canvas identity, persistence, configuration, and consent from an embedding host's default singleton. React code declares Cloud user event context through `setTelemetryCloudContext()` and captures through the service; it never receives, identifies, or resets the SDK client. +- `src/services/telemetry.ts` is the only module that accesses the named `agent-canvas` PostHog client. The name isolates Canvas identity, persistence, configuration, and consent from an embedding host's default singleton. React code declares Cloud user identity and event context through the service and captures through the service; it never receives, identifies, or resets the SDK client directly. - `TelemetryProvider` configures bootstrap/runtime options, eagerly initializes the service, and is the sole owner of the `useTelemetry()` lifecycle that emits install/session events. Do not mount that lifecycle hook separately in Canvas routes or internal components. The provider does not expose PostHog context or maintain a second client lifecycle. - The default PostHog key and direct ingestion host live in `config/defaults.json` under `telemetry`. Local launchers (`dev-with-automation`, `dev-static`, published binary path) and Docker default `AUTOMATION_POSTHOG_API_KEY` from explicit automation env, then `VITE_POSTHOG_API_KEY`, then that shared default key, so the automation backend can emit local consent-gated telemetry without extra user config. Keep `VITE_DO_NOT_TRACK=1` disabling the zero-config default. - Unconfigured source builds use the staging key and route through `https://z.openhands.dev`. Release workflows pass the public production key through `VITE_POSTHOG_API_KEY`. Precompiled npm consumers override `apiKey`, `apiHost`, and `uiHost` at runtime through `AgentServerUIProviders.analytics` or `configureTelemetry()`. - `setTelemetryConsent` is the only user-consent controller; `configureTelemetry(false)` is the embedding host's hard disable. An explicit first-run browser decision remains pending across local backends until `useSyncTelemetryConsent` persists it to Cloud; a stale/default backend value must not overwrite that newer choice during login or navigation. Once Cloud confirms the choice, backend `user_consents_to_analytics` changes are authoritative and mirrored to the client. No other hook or component should call `opt_in_capturing` / `opt_out_capturing` directly. - `subscribeTelemetryConsent` is the sole React-facing consent store. Hooks that render consent state must use `useSyncExternalStore`; do not mirror consent in component state or gate events outside `telemetry.ts`. -- `canvas_install` fires once, pre-consent, with the client's anonymous distinct ID. Consent does not identify PostHog as the Cloud user; Canvas keeps the browser/install distinct ID stable across local/Cloud backend switches, creates anonymous person profiles for analytics, and attaches Cloud account context as event properties (`cloud_user_id`, `cloud_user_email`) only while a Cloud backend is active. Legacy identified users are reset only when the PostHog client initializes without OAuth bootstrap IDs, and the reset immediately restores the canonical consent state that the PostHog SDK clears. +- `canvas_install` fires once, pre-consent, with the client's anonymous distinct ID. After consent and Cloud authentication, Canvas identifies PostHog with the stable Cloud user ID so PostHog joins the earlier anonymous activity to that person. Merely switching to a local backend clears Cloud event context without resetting the identified person; a resolved logout/account change, consent revocation, or privacy clear owns the reset. Local-only and never-authenticated traffic remains on the anonymous browser/install ID. Cloud account context (`cloud_user_id`, `cloud_user_email`, `cloud_org_id`) is attached as event properties only while a Cloud backend is active. - `telemetry.ts` adds immutable `client_source`, `client_version`, `package_name`, and `package_version` properties in `before_send`, so reset cannot remove attribution and event producers cannot override it. Repeated business milestones use deterministic PostHog `$insert_id` values instead of process-local caches. - `trackEvent` and `useTelemetry` remain the public library telemetry API for npm consumers (the `TelemetryConsentBanner` component was removed; hosts needing a consent UI build their own on `useTelemetry`). Non-React state machines use typed functions in `cloud-funnel-analytics.ts`; they do not call `trackEvent` directly. - React app events use typed functions in `src/hooks/use-tracking.ts`; components never call `posthog.capture()` raw. The hook attaches `current_url` automatically and captures through the telemetry service; Cloud account email is attached centrally as `cloud_user_email` while Cloud context is active. It may read backend settings for event properties, but must never gate capture on a settings snapshot: `useSyncTelemetryConsent` has already mirrored the authoritative decision to the telemetry service, and settings can be stale during a backend transition. diff --git a/__tests__/contexts/active-backend-context.test.tsx b/__tests__/contexts/active-backend-context.test.tsx index 8a0211ef98..a4771d6ceb 100644 --- a/__tests__/contexts/active-backend-context.test.tsx +++ b/__tests__/contexts/active-backend-context.test.tsx @@ -2,6 +2,17 @@ import React from "react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { act, render, renderHook } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { setTelemetryCloudContextMock, setTelemetryIdentityMock } = vi.hoisted( + () => ({ + setTelemetryCloudContextMock: vi.fn(), + setTelemetryIdentityMock: vi.fn(), + }), +); +vi.mock("#/services/telemetry", () => ({ + setTelemetryCloudContext: setTelemetryCloudContextMock, + setTelemetryIdentity: setTelemetryIdentityMock, +})); import { __resetActiveStoreForTests, NO_BACKEND_ID, @@ -35,6 +46,7 @@ beforeEach(() => { vi.stubEnv("VITE_SESSION_API_KEY", "session-key"); __resetActiveStoreForTests(); __resetHealthStoreForTests(); + vi.clearAllMocks(); }); afterEach(() => { @@ -139,6 +151,85 @@ describe("ActiveBackendProvider", () => { expect(queryClient.getQueryData(["dummy"])).toEqual({ value: 1 }); }); + it("clears Cloud context but preserves identity when switching from Cloud to local", () => { + const { result } = renderHook(() => useActiveBackendContext(), { + wrapper: makeWrapper(), + }); + act(() => { + result.current.addBackend({ + name: "Cloud", + host: "https://app.all-hands.dev", + apiKey: "cloud-key", + kind: "cloud", + }); + }); + vi.clearAllMocks(); + + act(() => { + result.current.setActive(SEEDED_DEFAULT_BACKEND_ID); + }); + + expect(result.current.active.backend.id).toBe(SEEDED_DEFAULT_BACKEND_ID); + expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).not.toHaveBeenCalled(); + }); + + it("clears Cloud context without resetting identity when switching Cloud orgs", () => { + const { result } = renderHook(() => useActiveBackendContext(), { + wrapper: makeWrapper(), + }); + let cloudId = ""; + act(() => { + cloudId = result.current.addBackend({ + name: "Cloud", + host: "https://app.all-hands.dev", + apiKey: "cloud-key", + kind: "cloud", + }).id; + }); + vi.clearAllMocks(); + + act(() => { + result.current.setActive(cloudId, "org-2"); + }); + + expect(result.current.active.orgId).toBe("org-2"); + expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).not.toHaveBeenCalled(); + }); + + it("clears Cloud context and defers identity when switching Cloud backends", () => { + const { result } = renderHook(() => useActiveBackendContext(), { + wrapper: makeWrapper(), + }); + let firstCloudId = ""; + act(() => { + firstCloudId = result.current.addBackend({ + name: "Cloud 1", + host: "https://app.all-hands.dev", + apiKey: "cloud-key-1", + kind: "cloud", + }).id; + }); + act(() => { + result.current.addBackend({ + name: "Cloud 2", + host: "https://app.all-hands.dev", + apiKey: "cloud-key-2", + kind: "cloud", + }); + }); + vi.clearAllMocks(); + + act(() => { + result.current.setActive(firstCloudId); + }); + + expect(result.current.active.backend.id).toBe(firstCloudId); + expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith(null); + }); + // @spec BM-003 — Fallback on active backend removal it("removeBackend falls back to the seeded default when the active backend is removed", () => { const { result } = renderHook(() => useActiveBackendContext(), { @@ -229,6 +320,54 @@ describe("ActiveBackendProvider", () => { // Assert expect(getBackendHealthEntry(id)).toBeNull(); + expect( + result.current.backends.find((backend) => backend.id === id), + ).toHaveProperty("connectionRevision", 1); + }); + + it("clears identity and re-keys data when active Cloud credentials change", () => { + const { result } = renderHook(() => useActiveBackendContext(), { + wrapper: makeWrapper(), + }); + let id = ""; + act(() => { + id = result.current.addBackend({ + name: "Cloud", + host: "https://app.all-hands.dev", + apiKey: "old-key", + kind: "cloud", + }).id; + }); + + act(() => { + result.current.updateBackend(id, { apiKey: "new-key" }); + }); + + expect(result.current.active.backend.connectionRevision).toBe(1); + expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith(null); + }); + + it("clears identity when the active Cloud backend is removed", () => { + const { result } = renderHook(() => useActiveBackendContext(), { + wrapper: makeWrapper(), + }); + let id = ""; + act(() => { + id = result.current.addBackend({ + name: "Cloud", + host: "https://app.all-hands.dev", + apiKey: "key", + kind: "cloud", + }).id; + }); + + act(() => { + result.current.removeBackend(id); + }); + + expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith(null); }); it("removeBackend drops the backend's persisted health entry", () => { diff --git a/__tests__/hooks/use-sync-automation-telemetry-consent.test.ts b/__tests__/hooks/use-sync-automation-telemetry-consent.test.ts index e98e4c4896..2a0552ce22 100644 --- a/__tests__/hooks/use-sync-automation-telemetry-consent.test.ts +++ b/__tests__/hooks/use-sync-automation-telemetry-consent.test.ts @@ -8,6 +8,7 @@ const state = { backendHost: "http://localhost:8000", backendApiKey: "key-1", consent: "pending" as "pending" | "granted" | "denied", + pendingRevocationId: null as string | null, }; const listeners = new Set<() => void>(); @@ -30,6 +31,7 @@ vi.mock("#/contexts/active-backend-context", () => ({ })); vi.mock("#/services/telemetry", () => ({ + getPendingLocalTelemetryRevocationId: () => state.pendingRevocationId, getTelemetryConsent: () => state.consent, subscribeTelemetryConsent: (listener: () => void) => { listeners.add(listener); @@ -51,6 +53,7 @@ describe("useSyncAutomationTelemetryConsent", () => { state.backendKind = "local"; state.consent = "pending"; + state.pendingRevocationId = null; }); it("does not call the local automation consent API while consent is pending", () => { @@ -59,6 +62,15 @@ describe("useSyncAutomationTelemetryConsent", () => { expect(syncTelemetryConsentMock).not.toHaveBeenCalled(); }); + it("revokes the pre-reset actor after a privacy clear", () => { + state.pendingRevocationId = "user-before-reset"; + + renderHook(() => useSyncAutomationTelemetryConsent()); + + expect(syncTelemetryConsentMock).toHaveBeenCalledOnce(); + expect(syncTelemetryConsentMock).toHaveBeenCalledWith("denied"); + }); + it("does not call the local automation consent API for cloud backends", () => { state.backendKind = "cloud"; state.consent = "granted"; diff --git a/__tests__/hooks/use-telemetry-identity.test.ts b/__tests__/hooks/use-telemetry-identity.test.ts index 9dbded927e..8cf0385f2d 100644 --- a/__tests__/hooks/use-telemetry-identity.test.ts +++ b/__tests__/hooks/use-telemetry-identity.test.ts @@ -17,9 +17,12 @@ vi.mock("#/hooks/query/use-cloud-current-user-id", () => ({ })); const setTelemetryCloudContextMock = vi.fn(); +const setTelemetryIdentityMock = vi.fn(); vi.mock("#/services/telemetry", () => ({ setTelemetryCloudContext: (...args: unknown[]) => setTelemetryCloudContextMock(...args), + setTelemetryIdentity: (...args: unknown[]) => + setTelemetryIdentityMock(...args), })); const trackBackendAddedMock = vi.fn(); @@ -62,6 +65,9 @@ describe("useTelemetryIdentity", () => { email: "user@example.com", orgId: "org-123", }); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith("user-123", { + email: "user@example.com", + }); }); it("falls back to the git email and omits an absent email", () => { @@ -102,6 +108,7 @@ describe("useTelemetryIdentity", () => { renderHook(() => useTelemetryIdentity()); expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith(null); }); it("clears Cloud user context while a local backend is active", () => { @@ -113,6 +120,7 @@ describe("useTelemetryIdentity", () => { renderHook(() => useTelemetryIdentity()); expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).not.toHaveBeenCalled(); }); it("declares a changed Cloud account", () => { @@ -128,6 +136,24 @@ describe("useTelemetryIdentity", () => { email: "user@example.com", orgId: "org-123", }); + expect(setTelemetryIdentityMock).toHaveBeenLastCalledWith("user-456", { + email: "user@example.com", + }); + }); + + it("redeclares the user after Cloud connection credentials change", () => { + const { rerender } = renderHook(() => useTelemetryIdentity()); + vi.clearAllMocks(); + useActiveBackendMock.mockReturnValue({ + backend: { ...cloudBackend, connectionRevision: 1 }, + orgId: "org-123", + }); + + rerender(); + + expect(setTelemetryIdentityMock).toHaveBeenCalledWith("user-123", { + email: "user@example.com", + }); }); it("tracks cookie-auth Cloud backend addition after identity and email resolve", () => { diff --git a/__tests__/services/telemetry-bootstrap.test.ts b/__tests__/services/telemetry-bootstrap.test.ts index 8b9be45fcd..0f0ad38f47 100644 --- a/__tests__/services/telemetry-bootstrap.test.ts +++ b/__tests__/services/telemetry-bootstrap.test.ts @@ -51,12 +51,12 @@ describe("Telemetry bootstrap identity migration", () => { .__AGENT_CANVAS_LOCK_TO_CLOUD__; }); - it("clears a legacy identified user when no OAuth bootstrap is present", async () => { + it("preserves an identified user while Cloud identity is unresolved", async () => { const { telemetry, mockPosthog } = await loadTelemetryWithLegacyUser(); await telemetry.setTelemetryConsent("granted"); - expect(mockPosthog.reset).toHaveBeenCalledWith(false); + expect(mockPosthog.reset).not.toHaveBeenCalled(); expect(mockPosthog.opt_in_capturing).toHaveBeenCalled(); expect(mockPosthog.identify).not.toHaveBeenCalled(); }); diff --git a/__tests__/services/telemetry.test.ts b/__tests__/services/telemetry.test.ts index 0a0425d3c1..db233122bc 100644 --- a/__tests__/services/telemetry.test.ts +++ b/__tests__/services/telemetry.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; // Mock posthog-js before importing telemetry service let identifiedUserId: string | undefined; +let anonymousDistinctId = "ph-test-distinct-id"; let latestPostHogConfig: | (Record & { before_send: (event: unknown) => unknown }) | undefined; @@ -18,9 +19,12 @@ const mockPosthog = { get_property: vi.fn((property: string) => property === "$user_id" ? identifiedUserId : undefined, ), - get_distinct_id: vi.fn(() => "ph-test-distinct-id"), - reset: vi.fn(() => { + get_distinct_id: vi.fn(() => identifiedUserId ?? anonymousDistinctId), + reset: vi.fn((resetDeviceId = false) => { identifiedUserId = undefined; + anonymousDistinctId = resetDeviceId + ? "ph-reset-device-id" + : "ph-reset-distinct-id"; }), }; mockPosthog.init.mockImplementation((_, config) => { @@ -34,21 +38,25 @@ vi.mock("posthog-js", () => ({ import { clearPendingCloudTelemetryConsent, + clearPendingLocalTelemetryRevocation, + clearTelemetryData, configureTelemetry, + getPendingCloudTelemetryConsent, + getPendingLocalTelemetryRevocationId, getTelemetryConsent, getTelemetryDistinctId, - getPendingCloudTelemetryConsent, + getTelemetryDistinctIdForConsentSync, initializePostHogClient, - setTelemetryConsent, - setTelemetryCloudContext, - setTelemetryBackendContext, - subscribeTelemetryConsent, isTelemetryEnabled, - trackInstall, - trackSessionStart, + setTelemetryBackendContext, + setTelemetryCloudContext, + setTelemetryConsent, + setTelemetryIdentity, + subscribeTelemetryConsent, trackEvent, trackException, - clearTelemetryData, + trackInstall, + trackSessionStart, } from "#/services/telemetry"; // Mock import.meta.env for tests @@ -60,7 +68,7 @@ vi.stubGlobal("import.meta", { }); describe("Telemetry Service", () => { - beforeEach(() => { + beforeEach(async () => { // Clear localStorage before each test localStorage.clear(); sessionStorage.clear(); @@ -69,8 +77,12 @@ describe("Telemetry Service", () => { // Reset mock vi.clearAllMocks(); identifiedUserId = undefined; + anonymousDistinctId = "ph-test-distinct-id"; mockPosthog.has_opted_out_capturing.mockReturnValue(false); - mockPosthog.get_distinct_id.mockReturnValue("ph-test-distinct-id"); + mockPosthog.get_distinct_id.mockImplementation( + () => identifiedUserId ?? anonymousDistinctId, + ); + await setTelemetryIdentity(null); setTelemetryBackendContext({}); }); @@ -157,8 +169,8 @@ describe("Telemetry Service", () => { }); }); - describe("Cloud context", () => { - it("adds Cloud user context without identifying PostHog as the Cloud user", async () => { + describe("Cloud identity and context", () => { + it("identifies a consented Cloud user and adds Cloud event context", async () => { await setTelemetryConsent("granted"); setTelemetryCloudContext({ @@ -166,8 +178,12 @@ describe("Telemetry Service", () => { email: "a@example.com", orgId: "org-a", }); + await setTelemetryIdentity("user-a", { email: "a@example.com" }); - expect(mockPosthog.identify).not.toHaveBeenCalled(); + expect(mockPosthog.identify).toHaveBeenCalledWith("user-a", { + email: "a@example.com", + }); + await expect(getTelemetryDistinctId()).resolves.toBe("user-a"); expect(latestPostHogConfig).toBeDefined(); expect( latestPostHogConfig!.before_send({ @@ -184,6 +200,62 @@ describe("Telemetry Service", () => { }); }); + it("resets before switching Cloud accounts and restores consent", async () => { + await setTelemetryConsent("granted"); + await setTelemetryIdentity("user-a"); + vi.clearAllMocks(); + + await setTelemetryIdentity("user-b"); + + expect(mockPosthog.reset).toHaveBeenCalledWith(false); + expect(mockPosthog.opt_in_capturing).toHaveBeenCalledOnce(); + expect(mockPosthog.identify).toHaveBeenCalledWith("user-b", {}); + }); + + it("clears identity on logout without changing the device", async () => { + await setTelemetryConsent("granted"); + await setTelemetryIdentity("user-a"); + vi.clearAllMocks(); + + await setTelemetryIdentity(null); + + expect(mockPosthog.reset).toHaveBeenCalledWith(false); + expect(mockPosthog.opt_in_capturing).toHaveBeenCalledOnce(); + expect(mockPosthog.identify).not.toHaveBeenCalled(); + }); + + it("removes identity on denial and reapplies it after consent returns", async () => { + await setTelemetryConsent("granted"); + await setTelemetryIdentity("user-a"); + vi.clearAllMocks(); + + await setTelemetryConsent("denied"); + + expect(mockPosthog.reset).toHaveBeenCalledWith(false); + expect(mockPosthog.opt_out_capturing).toHaveBeenCalled(); + + vi.clearAllMocks(); + await setTelemetryConsent("granted"); + + expect(mockPosthog.identify).toHaveBeenCalledWith("user-a", {}); + }); + + it("syncs denial for the actor that existed before PostHog reset", async () => { + await setTelemetryConsent("granted"); + await setTelemetryIdentity("user-a"); + + await setTelemetryConsent("denied"); + + expect(mockPosthog.get_distinct_id()).toBe("ph-reset-distinct-id"); + expect(getPendingLocalTelemetryRevocationId()).toBe("user-a"); + await expect(getTelemetryDistinctIdForConsentSync()).resolves.toBe( + "user-a", + ); + + clearPendingLocalTelemetryRevocation("user-a"); + expect(getPendingLocalTelemetryRevocationId()).toBeNull(); + }); + it("clears Cloud user context for local events", async () => { await setTelemetryConsent("granted"); @@ -490,6 +562,7 @@ describe("Telemetry Service", () => { describe("clearTelemetryData", () => { it("clears all telemetry data from localStorage", async () => { await setTelemetryConsent("granted"); + await setTelemetryIdentity("user-a"); localStorage.setItem("openhands-telemetry-first-use", "true"); await clearTelemetryData(); @@ -499,6 +572,9 @@ describe("Telemetry Service", () => { expect(localStorage.getItem("openhands-telemetry-first-use")).toBeNull(); expect(mockPosthog.reset).toHaveBeenCalledWith(true); expect(mockPosthog.opt_out_capturing).toHaveBeenCalled(); + await expect(getTelemetryDistinctIdForConsentSync()).resolves.toBe( + "user-a", + ); }); it("falls back to opting out if the SDK cannot reset", async () => { diff --git a/src/api/automation-service/automation-service.api.test.ts b/src/api/automation-service/automation-service.api.test.ts index 0911124a23..6b8cb2f50c 100644 --- a/src/api/automation-service/automation-service.api.test.ts +++ b/src/api/automation-service/automation-service.api.test.ts @@ -10,6 +10,7 @@ import AutomationService from "./automation-service.api"; const { localAxios, callCloudProxy, + clearPendingLocalTelemetryRevocation, getTelemetryConsent, getTelemetryDistinctId, getTelemetryDistinctIdForConsentSync, @@ -22,6 +23,7 @@ const { delete: vi.fn(), }, callCloudProxy: vi.fn(), + clearPendingLocalTelemetryRevocation: vi.fn(), getTelemetryConsent: vi.fn(), getTelemetryDistinctId: vi.fn(), getTelemetryDistinctIdForConsentSync: vi.fn(), @@ -39,6 +41,7 @@ vi.mock("#/api/cloud/proxy", () => ({ })); vi.mock("#/services/telemetry", () => ({ + clearPendingLocalTelemetryRevocation, getTelemetryConsent, getTelemetryDistinctId, getTelemetryDistinctIdForConsentSync, @@ -175,6 +178,9 @@ describe("AutomationService.syncTelemetryConsent", () => { }, { timeout: 5000 }, ); + expect(clearPendingLocalTelemetryRevocation).toHaveBeenCalledWith( + "ph-fe-sync", + ); }); it("skips cloud backends because cloud consent is handled by auth", async () => { diff --git a/src/api/automation-service/automation-service.api.ts b/src/api/automation-service/automation-service.api.ts index ec23fddc35..c58a7d8d71 100644 --- a/src/api/automation-service/automation-service.api.ts +++ b/src/api/automation-service/automation-service.api.ts @@ -1,5 +1,6 @@ import axios from "axios"; import { + clearPendingLocalTelemetryRevocation, getTelemetryConsent, getTelemetryDistinctId, getTelemetryDistinctIdForConsentSync, @@ -211,6 +212,9 @@ class AutomationService { }, { timeout: 5000 }, ); + if (consent !== "granted" && frontendDistinctId) { + clearPendingLocalTelemetryRevocation(frontendDistinctId); + } } static async getSdkVersion(): Promise { diff --git a/src/api/backend-registry/storage.ts b/src/api/backend-registry/storage.ts index eb74b4c68c..daec346417 100644 --- a/src/api/backend-registry/storage.ts +++ b/src/api/backend-registry/storage.ts @@ -31,7 +31,11 @@ function isValidBackend(value: unknown): value is Backend { typeof v.host === "string" && typeof v.apiKey === "string" && isValidKind(v.kind) && - isValidAuthMode(v.authMode) + isValidAuthMode(v.authMode) && + (v.connectionRevision === undefined || + (typeof v.connectionRevision === "number" && + Number.isSafeInteger(v.connectionRevision) && + v.connectionRevision >= 0)) ); } diff --git a/src/api/backend-registry/types.ts b/src/api/backend-registry/types.ts index 3a3c2e861f..33e0bbac9c 100644 --- a/src/api/backend-registry/types.ts +++ b/src/api/backend-registry/types.ts @@ -8,6 +8,8 @@ export interface Backend { apiKey: string; kind: BackendKind; authMode?: BackendAuthMode; + /** Changes whenever connection credentials change, invalidating keyed data. */ + connectionRevision?: number; } export interface BackendSelection { diff --git a/src/contexts/active-backend-context.tsx b/src/contexts/active-backend-context.tsx index a96c67ac60..2127e5a456 100644 --- a/src/contexts/active-backend-context.tsx +++ b/src/contexts/active-backend-context.tsx @@ -21,13 +21,19 @@ import { } from "#/api/backend-registry/types"; import { QUERY_KEYS } from "#/hooks/query/query-keys"; import { queryClient } from "#/query-client-config"; +import { + setTelemetryCloudContext, + setTelemetryIdentity, +} from "#/services/telemetry"; + +type BackendInput = Omit; interface ActiveBackendContextValue { backends: Backend[]; active: ResolvedActiveBackend; setActive: (backendId: string, orgId?: string | null) => void; - addBackend: (backend: Omit) => Backend; - updateBackend: (id: string, patch: Partial>) => void; + addBackend: (backend: BackendInput) => Backend; + updateBackend: (id: string, patch: Partial) => void; removeBackend: (id: string) => void; } @@ -44,6 +50,24 @@ function generateId(): string { return `backend-${Date.now()}-${Math.random().toString(36).slice(2, 10)}`; } +function applyTelemetrySelectionBoundary( + previous: ResolvedActiveBackend, + next: ResolvedActiveBackend, +): void { + const previousWasCloud = previous.backend.kind === "cloud"; + const nextIsCloud = next.backend.kind === "cloud"; + if (!previousWasCloud && !nextIsCloud) return; + + setTelemetryCloudContext(null); + + if ( + nextIsCloud && + (!previousWasCloud || previous.backend.id !== next.backend.id) + ) { + void setTelemetryIdentity(null); + } +} + export function ActiveBackendProvider({ children, }: { @@ -64,12 +88,23 @@ export function ActiveBackendProvider({ const setActive = React.useCallback( (backendId: string, orgId?: string | null) => { + const currentSnapshot = getSnapshot(); const prevBackendId = getActiveSelection()?.backendId ?? null; const prevOrgId = getActiveSelection()?.orgId ?? null; const nextOrgId = orgId ?? null; if (backendId === prevBackendId && nextOrgId === prevOrgId) return; + const registeredBackends = getRegisteredBackends(); + const selectedBackend = registeredBackends.find( + (b) => b.id === backendId, + ); + const nextActive: ResolvedActiveBackend = { + backend: selectedBackend ?? registeredBackends[0] ?? NO_BACKEND, + orgId: selectedBackend ? nextOrgId : null, + }; + applyTelemetrySelectionBoundary(currentSnapshot.active, nextActive); + const next: BackendSelection = { backendId, orgId: nextOrgId }; setActiveSelection(next); retryBootstrapProbe(); @@ -87,9 +122,14 @@ export function ActiveBackendProvider({ // @spec BM-001 — Auto-switch to newly connected backend const addBackend = React.useCallback( - (backend: Omit): Backend => { + (backend: BackendInput): Backend => { + const previousActive = getSnapshot().active; const next: Backend = { ...backend, id: generateId() }; const list = [...getRegisteredBackends(), next]; + applyTelemetrySelectionBoundary(previousActive, { + backend: next, + orgId: null, + }); setRegisteredBackends(list); setActiveSelection({ backendId: next.id }); retryBootstrapProbe(); @@ -99,14 +139,9 @@ export function ActiveBackendProvider({ ); const updateBackend = React.useCallback( - (id: string, patch: Partial>) => { + (id: string, patch: Partial) => { const prev = getRegisteredBackends().find((b) => b.id === id); - const activeBeforeUpdate = getActiveSelection()?.backendId ?? null; - const list = getRegisteredBackends().map((b) => - b.id === id ? { ...b, ...patch } : b, - ); - setRegisteredBackends(list); - + const activeBeforeUpdate = getSnapshot().active.backend.id; // Re-arm health polling when the user edits the fields that // actually drive the probe. Cosmetic edits (name) shouldn't // re-enable a backend that was disabled for being unreachable. @@ -118,9 +153,27 @@ export function ActiveBackendProvider({ patch.apiKey !== undefined && prev !== undefined && patch.apiKey !== prev.apiKey; + const list = getRegisteredBackends().map((b) => + b.id === id + ? { + ...b, + ...patch, + connectionRevision: + hostChanged || apiKeyChanged + ? (b.connectionRevision ?? 0) + 1 + : b.connectionRevision, + } + : b, + ); + setRegisteredBackends(list); + if (hostChanged || apiKeyChanged) { resetBackendHealth(id); if (activeBeforeUpdate === id) { + if (prev?.kind === "cloud") { + setTelemetryCloudContext(null); + void setTelemetryIdentity(null); + } retryBootstrapProbe(); } } @@ -130,8 +183,16 @@ export function ActiveBackendProvider({ const removeBackend = React.useCallback( (id: string) => { + const removed = getRegisteredBackends().find((b) => b.id === id); + const wasActiveCloud = + removed?.kind === "cloud" && + getSnapshot().active.backend.id === removed.id; const list = getRegisteredBackends().filter((b) => b.id !== id); setRegisteredBackends(list); + if (wasActiveCloud) { + setTelemetryCloudContext(null); + void setTelemetryIdentity(null); + } dropBackendHealth(id); retryBootstrapProbe(); // If the active selection pointed at this backend, the active diff --git a/src/hooks/query/use-cloud-current-user-id.ts b/src/hooks/query/use-cloud-current-user-id.ts index 351ebcc080..d462eb21ab 100644 --- a/src/hooks/query/use-cloud-current-user-id.ts +++ b/src/hooks/query/use-cloud-current-user-id.ts @@ -35,7 +35,11 @@ export function useCloudCurrentUserId(): Record< const active = useActiveBackend(); const cloudOrgs = useAllCloudOrganizations(); - const targets: { backendId: string; orgIdForMe: string }[] = []; + const targets: { + backendId: string; + connectionRevision: number; + orgIdForMe: string; + }[] = []; for (const backend of backends) { if (backend.kind === "cloud") { const entry = cloudOrgs[backend.id]; @@ -47,19 +51,28 @@ export function useCloudCurrentUserId(): Record< ? active.orgId : (entry?.orgs[0]?.id ?? null); if (preferredOrgId) { - targets.push({ backendId: backend.id, orgIdForMe: preferredOrgId }); + targets.push({ + backendId: backend.id, + connectionRevision: backend.connectionRevision ?? 0, + orgIdForMe: preferredOrgId, + }); } } } const results = useQueries({ - queries: targets.map(({ backendId, orgIdForMe }) => { + queries: targets.map(({ backendId, connectionRevision, orgIdForMe }) => { const backend = backends.find((b) => b.id === backendId); return { // `orgIdForMe` is included in the query key so re-resolving the // active org also re-keys this query → React Query refetches // automatically without relying on explicit invalidation. - queryKey: ["cloud-current-user", backendId, orgIdForMe] as const, + queryKey: [ + "cloud-current-user", + backendId, + orgIdForMe, + connectionRevision, + ] as const, queryFn: async () => { if (!backend) return { orgId: orgIdForMe, userId: "" }; return getCloudOrganizationMe(orgIdForMe, backend); diff --git a/src/hooks/query/use-cloud-organizations.ts b/src/hooks/query/use-cloud-organizations.ts index 4c76d029a1..48387c5c86 100644 --- a/src/hooks/query/use-cloud-organizations.ts +++ b/src/hooks/query/use-cloud-organizations.ts @@ -19,7 +19,11 @@ export function useAllCloudOrganizations() { const queries = useQueries({ queries: cloudBackends.map((backend) => ({ - queryKey: ["cloud-organizations", backend.id], + queryKey: [ + "cloud-organizations", + backend.id, + backend.connectionRevision ?? 0, + ], // Filter the user's full org membership down to the single org the // backend's API key is bound to. The cloud enforces one-key-one-org // server-side (HTTP 403 otherwise); without this filter the diff --git a/src/hooks/use-can-manage-org-profiles.ts b/src/hooks/use-can-manage-org-profiles.ts index 7768a38199..b9cca2d990 100644 --- a/src/hooks/use-can-manage-org-profiles.ts +++ b/src/hooks/use-can-manage-org-profiles.ts @@ -30,12 +30,16 @@ export function useCanManageOrgProfiles(): boolean { const { backend, orgId } = useActiveBackend(); const isCloud = backend.kind === "cloud"; - // `backend` is identified by `backend.id`, which is already in the key; we - // keep the key byte-identical to useCloudCurrentUserId so React Query shares - // the cached /me result instead of firing a second request. + // Keep the key byte-identical to useCloudCurrentUserId so React Query shares + // the cached /me result, including its connection-credential revision. // eslint-disable-next-line @tanstack/query/exhaustive-deps const { data } = useQuery({ - queryKey: ["cloud-current-user", backend.id, orgId], + queryKey: [ + "cloud-current-user", + backend.id, + orgId, + backend.connectionRevision ?? 0, + ], queryFn: () => getCloudOrganizationMe(orgId!, backend), enabled: isCloud && !!orgId, staleTime: 1000 * 60 * 5, diff --git a/src/hooks/use-sync-automation-telemetry-consent.ts b/src/hooks/use-sync-automation-telemetry-consent.ts index 1b2ee294f0..c597f28cde 100644 --- a/src/hooks/use-sync-automation-telemetry-consent.ts +++ b/src/hooks/use-sync-automation-telemetry-consent.ts @@ -2,6 +2,7 @@ import React from "react"; import AutomationService from "#/api/automation-service/automation-service.api"; import { useActiveBackend } from "#/contexts/active-backend-context"; import { + getPendingLocalTelemetryRevocationId, getTelemetryConsent, subscribeTelemetryConsent, } from "#/services/telemetry"; @@ -16,12 +17,23 @@ export function useSyncAutomationTelemetryConsent() { const lastSyncKeyRef = React.useRef(null); React.useEffect(() => { - if (backend.kind !== "local" || consent === "pending") return; + const pendingRevocationId = + consent === "pending" ? getPendingLocalTelemetryRevocationId() : null; + if ( + backend.kind !== "local" || + (consent === "pending" && !pendingRevocationId) + ) { + return; + } - const syncKey = `${backend.id}:${backend.host}:${backend.apiKey ?? ""}:${consent}`; + const resolvedConsent = consent === "granted" ? "granted" : "denied"; + const pendingActor = consent === "pending" ? pendingRevocationId : ""; + const syncKey = `${backend.id}:${backend.host}:${backend.apiKey ?? ""}:${resolvedConsent}:${pendingActor}`; if (lastSyncKeyRef.current === syncKey) return; lastSyncKeyRef.current = syncKey; - void AutomationService.syncTelemetryConsent(consent).catch(() => {}); + void AutomationService.syncTelemetryConsent(resolvedConsent).catch( + () => {}, + ); }, [backend.apiKey, backend.host, backend.id, backend.kind, consent]); } diff --git a/src/hooks/use-telemetry-identity.ts b/src/hooks/use-telemetry-identity.ts index 8b407efc89..340de8b53e 100644 --- a/src/hooks/use-telemetry-identity.ts +++ b/src/hooks/use-telemetry-identity.ts @@ -3,7 +3,10 @@ import { useActiveBackend } from "#/contexts/active-backend-context"; import { useCloudCurrentUserId } from "#/hooks/query/use-cloud-current-user-id"; import { useSettings } from "#/hooks/query/use-settings"; import { useTracking } from "#/hooks/use-tracking"; -import { setTelemetryCloudContext } from "#/services/telemetry"; +import { + setTelemetryCloudContext, + setTelemetryIdentity, +} from "#/services/telemetry"; const CLOUD_COOKIE_BACKEND_ADDED_SESSION_PREFIX = "agent_canvas_cloud_cookie_backend_added"; @@ -74,10 +77,12 @@ export const useTelemetryIdentity = () => { if (!userId) { setTelemetryCloudContext(null); + void setTelemetryIdentity(null); return; } setTelemetryCloudContext({ userId, email, orgId }); + void setTelemetryIdentity(userId, email ? { email } : {}); if ( backend.authMode === "cookie" && @@ -94,6 +99,7 @@ export const useTelemetryIdentity = () => { } }, [ backend.authMode, + backend.connectionRevision, backend.id, backend.kind, email, diff --git a/src/services/telemetry.ts b/src/services/telemetry.ts index 447057c09c..e3dde89b7f 100644 --- a/src/services/telemetry.ts +++ b/src/services/telemetry.ts @@ -45,6 +45,8 @@ import { const TELEMETRY_CONSENT_KEY = "openhands-telemetry-consent"; const TELEMETRY_CONSENT_PENDING_CLOUD_SYNC_KEY = "openhands-telemetry-consent-pending-cloud-sync"; +const TELEMETRY_CONSENT_PENDING_LOCAL_REVOCATION_KEY = + "openhands-telemetry-consent-pending-local-revocation"; const TELEMETRY_CONSENT_CHANGE_EVENT = "openhands-telemetry-consent-change"; const TELEMETRY_FIRST_USE_KEY = "openhands-telemetry-first-use"; const TELEMETRY_SESSION_KEY = "openhands-telemetry-session"; @@ -93,6 +95,18 @@ let pendingBootstrap: BootstrapConfig | undefined; let telemetryConfig: TelemetryConfig = {}; let telemetryDisabled = false; +interface TelemetryIdentity { + distinctId: string; + properties: Record; +} + +// undefined means that Cloud identity has not resolved yet; null means that it +// resolved without a user. This distinction prevents startup from resetting a +// persisted identity while the current account is still loading. +let desiredTelemetryIdentity: TelemetryIdentity | null | undefined; +let desiredIdentityRevision = 0; +let appliedIdentityRevision = -1; + const CANVAS_EVENT_PROPERTIES = Object.freeze({ client_source: AGENT_CANVAS_CLIENT_SOURCE, client_version: AGENT_CANVAS_CLIENT_VERSION, @@ -141,15 +155,47 @@ function restorePostHogConsent(posthog: PostHog): void { function resetPostHogIdentity(posthog: PostHog, resetDeviceId = false): void { posthog.reset(resetDeviceId); + appliedIdentityRevision = -1; // PostHog reset clears its own consent persistence, so immediately restore // the canonical Canvas decision kept in localStorage. restorePostHogConsent(posthog); } -function clearLegacyIdentifiedUser(posthog: PostHog): void { - if (posthog.get_property?.("$user_id") != null) { +function applyDesiredTelemetryIdentity(posthog: PostHog): void { + if (desiredTelemetryIdentity === undefined || !isTelemetryEnabled()) return; + + const desiredId = desiredTelemetryIdentity?.distinctId; + const currentId = posthog.get_property("$user_id"); + if (currentId != null && currentId !== desiredId) { resetPostHogIdentity(posthog); } + + if (desiredTelemetryIdentity === null) { + appliedIdentityRevision = desiredIdentityRevision; + return; + } + + if ( + posthog.get_property("$user_id") !== desiredTelemetryIdentity.distinctId || + appliedIdentityRevision !== desiredIdentityRevision + ) { + posthog.identify( + desiredTelemetryIdentity.distinctId, + desiredTelemetryIdentity.properties, + ); + appliedIdentityRevision = desiredIdentityRevision; + } +} + +function propertiesEqual( + left: Record, + right: Record, +): boolean { + const keys = Object.keys(left); + return ( + keys.length === Object.keys(right).length && + keys.every((key) => left[key] === right[key]) + ); } /** @@ -178,6 +224,7 @@ export function configureTelemetry(config: TelemetryConfiguration): void { if (wasDisabled) { if (posthogInstance) { restorePostHogConsent(posthogInstance); + applyDesiredTelemetryIdentity(posthogInstance); } notifyTelemetryConsentListeners(); } @@ -295,7 +342,6 @@ export async function initializePostHogClient( // A named instance isolates Canvas configuration, consent, identity, and // persistence from a host application's default PostHog singleton. - const bootstrap = pendingBootstrap; const initializedPostHog = posthog.init( config.apiKey, { @@ -309,7 +355,7 @@ export async function initializePostHogClient( capture_pageview: POSTHOG_PAGEVIEW_CAPTURE_MODE, autocapture: true, disable_session_recording: true, - bootstrap, + bootstrap: pendingBootstrap, before_send: addCanvasEventProperties, }, POSTHOG_INSTANCE_NAME, @@ -318,14 +364,12 @@ export async function initializePostHogClient( posthogInstance = initializedPostHog; pendingBootstrap = undefined; - if (!bootstrap) { - clearLegacyIdentifiedUser(posthogInstance); - } if (telemetryDisabled) { posthogInstance.opt_out_capturing(); } else if (getTelemetryConsent() === "granted") { posthogInstance.opt_in_capturing(); + applyDesiredTelemetryIdentity(posthogInstance); } else if (!enableCapturing) { posthogInstance.opt_out_capturing(); } @@ -389,6 +433,17 @@ export function getPendingCloudTelemetryConsent(): ResolvedTelemetryConsent | nu } } +/** Return the pre-reset actor whose local Automation consent must be revoked. */ +export function getPendingLocalTelemetryRevocationId(): string | null { + if (!isBrowser()) return null; + + try { + return localStorage.getItem(TELEMETRY_CONSENT_PENDING_LOCAL_REVOCATION_KEY); + } catch { + return null; + } +} + export function subscribeTelemetryConsent(listener: () => void): () => void { if (!isBrowser()) return () => {}; const handleStorage = (event: StorageEvent) => { @@ -425,6 +480,37 @@ function markTelemetryConsentForCloudSync( } } +function markPendingLocalTelemetryRevocation(distinctId: string | null): void { + if (!isBrowser() || !distinctId) return; + + try { + localStorage.setItem( + TELEMETRY_CONSENT_PENDING_LOCAL_REVOCATION_KEY, + distinctId, + ); + } catch { + // Ignore storage errors; browser capture is still disabled below. + } +} + +export function clearPendingLocalTelemetryRevocation( + expectedDistinctId: string, +): void { + if (!isBrowser()) return; + + try { + if ( + localStorage.getItem(TELEMETRY_CONSENT_PENDING_LOCAL_REVOCATION_KEY) !== + expectedDistinctId + ) { + return; + } + localStorage.removeItem(TELEMETRY_CONSENT_PENDING_LOCAL_REVOCATION_KEY); + } catch { + // Ignore storage errors. + } +} + export function clearPendingCloudTelemetryConsent( expected?: ResolvedTelemetryConsent, ): void { @@ -472,7 +558,9 @@ export async function setTelemetryConsent( if (consent === "granted") { posthog.opt_in_capturing(); + applyDesiredTelemetryIdentity(posthog); } else { + markPendingLocalTelemetryRevocation(posthog.get_distinct_id?.() ?? null); if (posthog.get_property?.("$user_id") != null) { resetPostHogIdentity(posthog); } else { @@ -492,6 +580,37 @@ export async function setTelemetryConsent( } } +/** + * Declare the current Cloud identity. The telemetry service applies it only + * after consent and owns all reset/account-switch semantics. + */ +export async function setTelemetryIdentity( + distinctId: string | null, + properties: Record = {}, +): Promise { + const nextIdentity = distinctId === null ? null : { distinctId, properties }; + const unchanged = + desiredTelemetryIdentity === nextIdentity || + (desiredTelemetryIdentity !== undefined && + desiredTelemetryIdentity !== null && + nextIdentity !== null && + desiredTelemetryIdentity.distinctId === nextIdentity.distinctId && + propertiesEqual(desiredTelemetryIdentity.properties, properties)); + if (unchanged) return; + + desiredTelemetryIdentity = nextIdentity; + desiredIdentityRevision += 1; + appliedIdentityRevision = -1; + + if (!isTelemetryEnabled()) return; + const posthog = posthogInstance ?? (await initializePostHogClient()); + if (posthog && isTelemetryEnabled()) { + // Read the desired identity after the await so a newer account always wins + // if identity changes while the SDK is loading. + applyDesiredTelemetryIdentity(posthog); + } +} + /** * Check if telemetry is enabled (user has granted consent) */ @@ -634,6 +753,8 @@ async function getPostHogForConsentedCapture(): Promise { posthog.opt_in_capturing(); } + applyDesiredTelemetryIdentity(posthog); + return posthog; } @@ -669,7 +790,14 @@ export async function getTelemetryDistinctId(): Promise { export async function getTelemetryDistinctIdForConsentSync(): Promise< string | null > { - if (!isBrowser() || telemetryDisabled || isDoNotTrackEnabled()) return null; + if (!isBrowser()) return null; + + if (getTelemetryConsent() !== "granted") { + const pendingRevocationId = getPendingLocalTelemetryRevocationId(); + if (pendingRevocationId) return pendingRevocationId; + } + + if (telemetryDisabled || isDoNotTrackEnabled()) return null; const posthog = await initializePostHogClient(); return posthog?.get_distinct_id?.() ?? null; @@ -708,6 +836,9 @@ export async function clearTelemetryData(): Promise { } try { + markPendingLocalTelemetryRevocation( + posthogInstance?.get_distinct_id?.() ?? null, + ); localStorage.removeItem(TELEMETRY_CONSENT_KEY); localStorage.removeItem(TELEMETRY_FIRST_USE_KEY); } catch { @@ -722,6 +853,9 @@ export async function clearTelemetryData(): Promise { telemetryBackendContext = getBackendTelemetryProperties({}); telemetryCloudContext = getCloudTelemetryProperties(); + desiredTelemetryIdentity = null; + desiredIdentityRevision += 1; + appliedIdentityRevision = -1; try { if (posthogInstance) { @@ -734,5 +868,7 @@ export async function clearTelemetryData(): Promise { } catch { // Telemetry failures must not break the application. } + } finally { + notifyTelemetryConsentListeners(); } }