diff --git a/AGENTS.md b/AGENTS.md index f32254633..b6df81956 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,13 +32,13 @@ by a human, report the exact validator error rather than editing them 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 and eagerly initializes the service. It 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 8a0211ef9..bfe4248b3 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(() => { @@ -229,6 +241,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 e98e4c489..2a0552ce2 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 8fae45e91..d2b0cf01d 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), })); import { useTelemetryIdentity } from "#/hooks/use-telemetry-identity"; @@ -51,6 +54,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", () => { @@ -91,14 +97,19 @@ describe("useTelemetryIdentity", () => { renderHook(() => useTelemetryIdentity()); expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).toHaveBeenCalledWith(null); }); it("clears Cloud user context while a local backend is active", () => { - useActiveBackendMock.mockReturnValue({ backend: localBackend, orgId: null }); + useActiveBackendMock.mockReturnValue({ + backend: localBackend, + orgId: null, + }); renderHook(() => useTelemetryIdentity()); expect(setTelemetryCloudContextMock).toHaveBeenCalledWith(null); + expect(setTelemetryIdentityMock).not.toHaveBeenCalled(); }); it("declares a changed Cloud account", () => { @@ -114,5 +125,23 @@ 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", + }); }); }); diff --git a/__tests__/services/telemetry-bootstrap.test.ts b/__tests__/services/telemetry-bootstrap.test.ts index 8b9be45fc..0f0ad38f4 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 11383c6bb..fe2f6911e 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: | { 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, + isTelemetryEnabled, setTelemetryBackendContext, + setTelemetryCloudContext, + setTelemetryConsent, + setTelemetryIdentity, subscribeTelemetryConsent, - isTelemetryEnabled, - trackInstall, - trackSessionStart, 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({}); }); @@ -155,8 +167,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({ @@ -164,8 +176,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({ @@ -182,6 +198,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"); @@ -488,6 +560,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(); @@ -497,6 +570,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 0911124a2..6b8cb2f50 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 ec23fddc3..c58a7d8d7 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 eb74b4c68..daec34641 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 3a3c2e861..33e0bbac9 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 a96c67ac6..3fb25dcee 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; } @@ -87,7 +93,7 @@ export function ActiveBackendProvider({ // @spec BM-001 — Auto-switch to newly connected backend const addBackend = React.useCallback( - (backend: Omit): Backend => { + (backend: BackendInput): Backend => { const next: Backend = { ...backend, id: generateId() }; const list = [...getRegisteredBackends(), next]; setRegisteredBackends(list); @@ -99,14 +105,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 +119,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 +149,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 351ebcc08..d462eb21a 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 4c76d029a..48387c5c8 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 7768a3819..b9cca2d99 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 1b2ee294f..c597f28cd 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 adb7f4dcb..1adf11f05 100644 --- a/src/hooks/use-telemetry-identity.ts +++ b/src/hooks/use-telemetry-identity.ts @@ -2,7 +2,10 @@ import React from "react"; 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 { setTelemetryCloudContext } from "#/services/telemetry"; +import { + setTelemetryCloudContext, + setTelemetryIdentity, +} from "#/services/telemetry"; /** Keep Cloud user event context aligned with the active backend. */ export const useTelemetryIdentity = () => { @@ -24,9 +27,19 @@ export const useTelemetryIdentity = () => { if (!userId) { setTelemetryCloudContext(null); + void setTelemetryIdentity(null); return; } setTelemetryCloudContext({ userId, email, orgId }); - }, [backend.kind, email, isIdentityLoading, orgId, userId]); + void setTelemetryIdentity(userId, email ? { email } : {}); + }, [ + backend.connectionRevision, + backend.id, + backend.kind, + email, + isIdentityLoading, + orgId, + userId, + ]); }; diff --git a/src/services/telemetry.ts b/src/services/telemetry.ts index fba8c7036..e98419933 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"; @@ -92,6 +94,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, @@ -140,15 +154,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]) + ); } /** @@ -177,6 +223,7 @@ export function configureTelemetry(config: TelemetryConfiguration): void { if (wasDisabled) { if (posthogInstance) { restorePostHogConsent(posthogInstance); + applyDesiredTelemetryIdentity(posthogInstance); } notifyTelemetryConsentListeners(); } @@ -294,7 +341,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, { @@ -308,7 +354,7 @@ export async function initializePostHogClient( consent_persistence_name: `${POSTHOG_INSTANCE_NAME}-consent`, person_profiles: "always", disable_session_recording: true, - bootstrap, + bootstrap: pendingBootstrap, before_send: addCanvasEventProperties, }, POSTHOG_INSTANCE_NAME, @@ -317,14 +363,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(); } @@ -388,6 +432,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) => { @@ -424,6 +479,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 { @@ -471,7 +557,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 { @@ -491,6 +579,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) */ @@ -633,6 +752,8 @@ async function getPostHogForConsentedCapture(): Promise { posthog.opt_in_capturing(); } + applyDesiredTelemetryIdentity(posthog); + return posthog; } @@ -668,7 +789,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; @@ -707,6 +835,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 { @@ -721,6 +852,9 @@ export async function clearTelemetryData(): Promise { telemetryBackendContext = getBackendTelemetryProperties({}); telemetryCloudContext = getCloudTelemetryProperties(); + desiredTelemetryIdentity = null; + desiredIdentityRevision += 1; + appliedIdentityRevision = -1; try { if (posthogInstance) { @@ -733,5 +867,7 @@ export async function clearTelemetryData(): Promise { } catch { // Telemetry failures must not break the application. } + } finally { + notifyTelemetryConsentListeners(); } }