fix(analytics): harden analytics opt-out and feedback surfaces (#423)

Server stops phoning Sentry home after opt-out (release-health sessions + client reports off); settings saves diff-send only changed keys so a stale tab cannot revert an instance-wide opt-out; disabling analytics hides the feedback UI immediately; optIn resumes PostHog after re-enable; onboarding survey writes time out at 15s; inline tool-feedback prompt arms a shown-cooldown.

apps/api/src/instrument.ts

@@ -27,6 +27,11 @@ if (ANALYTICS_BAKED.sentryDsn) {
       environment: process.env.NODE_ENV || "production",
       tracesSampleRate: ANALYTICS_BAKED.sampleRate,
       sendDefaultPii: false,
+      // Release-health request-sessions and client-report envelopes are sent outside
+      // beforeSend/beforeSendTransaction, so the runtime opt-out below would not stop
+      // them. Disable both so an opted-out instance truly stops phoning home.
+      integrations: [Sentry.httpIntegration({ trackIncomingRequestsAsSessions: false })],
+      sendClientReports: false,
       // Runtime opt-out: drop the whole transaction when analytics is off.
       tracesSampler: () => (sentryActive() ? ANALYTICS_BAKED.sampleRate : 0),
       beforeSend(event) {

apps/web/src/components/feedback/tool-feedback-prompt.tsx

@@ -19,10 +19,15 @@ interface ToolFeedbackPromptProps {
 }
 
 const GLOBAL_LAST_PROMPT_KEY = "snapotter-feedback-last-prompt-at";
+const GLOBAL_LAST_SHOWN_KEY = "snapotter-feedback-last-shown-at";
 const PROMPTS_DISABLED_KEY = "snapotter-feedback-prompts-disabled";
 const TOOL_PROMPT_PREFIX = "snapotter-feedback-tool-prompt:";
 const GLOBAL_PROMPT_COOLDOWN_MS = 30 * 24 * 60 * 60 * 1000;
 const TOOL_PROMPT_COOLDOWN_MS = 90 * 24 * 60 * 60 * 1000;
+// A prompt that was merely shown (not acted on) still arms a short cooldown so an
+// ignored card does not reappear on the very next result. Interacting arms the much
+// longer cooldowns above.
+const SHOWN_PROMPT_COOLDOWN_MS = 3 * 24 * 60 * 60 * 1000;
 
 function readNumber(key: string): number {
   try {
@@ -61,6 +66,8 @@ function disablePrompts(): void {
 function shouldShowPrompt(toolId: string): boolean {
   if (!toolId || promptsDisabled()) return false;
   const now = Date.now();
+  const lastShown = readNumber(GLOBAL_LAST_SHOWN_KEY);
+  if (lastShown && now - lastShown < SHOWN_PROMPT_COOLDOWN_MS) return false;
   const lastGlobal = readNumber(GLOBAL_LAST_PROMPT_KEY);
   if (lastGlobal && now - lastGlobal < GLOBAL_PROMPT_COOLDOWN_MS) return false;
   const lastTool = readNumber(`${TOOL_PROMPT_PREFIX}${toolId}`);
@@ -73,6 +80,10 @@ function markPromptHandled(toolId: string): void {
   writeNow(`${TOOL_PROMPT_PREFIX}${toolId}`);
 }
 
+function markPromptShown(): void {
+  writeNow(GLOBAL_LAST_SHOWN_KEY);
+}
+
 export function ToolFeedbackPrompt({
   toolId,
   jobStatus = "completed",
@@ -88,7 +99,9 @@ export function ToolFeedbackPrompt({
 
   useEffect(() => {
     if (!analyticsLoaded || !analyticsConfig?.enabled) return;
-    setVisible(shouldShowPrompt(toolId));
+    const show = shouldShowPrompt(toolId);
+    if (show) markPromptShown();
+    setVisible(show);
   }, [analyticsLoaded, analyticsConfig?.enabled, toolId]);
 
   if (!analyticsLoaded || !analyticsConfig?.enabled) return null;

apps/web/src/components/onboarding/usage-survey-overlay.tsx

@@ -26,8 +26,14 @@ import {
   surveyIdForSource,
 } from "@/lib/feedback";
 import { cn } from "@/lib/utils";
+import { withTimeout } from "@/lib/with-timeout";
 import { useAnalyticsStore } from "@/stores/analytics-store";
 
+// A hung write (connection black-holed, never erroring) would otherwise leave both
+// buttons disabled forever with no exit but a page reload; time it out so the overlay
+// re-enables and the admin can retry or dismiss.
+const WRITE_TIMEOUT_MS = 15_000;
+
 const USAGE_TYPES: { value: FeedbackUsageType; Icon: typeof User; wide?: boolean }[] = [
   { value: "personal", Icon: User },
   { value: "team_internal", Icon: Users },
@@ -97,7 +103,7 @@ export function UsageSurveyOverlay() {
 
   async function recordSettingsKey(key: string) {
     const value = new Date().toISOString();
-    await apiPut("/v1/settings", { [key]: value });
+    await withTimeout(apiPut("/v1/settings", { [key]: value }), WRITE_TIMEOUT_MS);
     setSettings((current) => ({ ...(current ?? {}), [key]: value }));
   }
 
@@ -107,13 +113,16 @@ export function UsageSurveyOverlay() {
     const answerKey = JSON.stringify({ usageType, importantAreas: [...importantAreas].sort() });
     try {
       if (submittedAnswerKeyRef.current !== answerKey) {
-        await submitFeedback({
+        await withTimeout(
-          source: "onboarding",
+          submitFeedback({
-          surveyId: surveyIdForSource("onboarding"),
+            source: "onboarding",
-          promptVariant: promptVariantForSource("onboarding"),
+            surveyId: surveyIdForSource("onboarding"),
-          usageType,
+            promptVariant: promptVariantForSource("onboarding"),
-          importantAreas,
+            usageType,
-        });
+            importantAreas,
+          }),
+          WRITE_TIMEOUT_MS,
+        );
         submittedAnswerKeyRef.current = answerKey;
       }
       await recordSettingsKey("onboarding.usageSurvey.answeredAt");

apps/web/src/components/settings/settings-dialog.tsx

@@ -35,6 +35,7 @@ import { useMobile } from "@/hooks/use-mobile";
 import { apiDelete, apiGet, apiPost, apiPut, clearToken, formatHeaders } from "@/lib/api";
 import { shouldShowInstallFeedbackCard } from "@/lib/feedback";
 import { format, plural } from "@/lib/format";
+import { changedSettings, writableSettings } from "@/lib/settings-payload";
 import { getCategoryName, getToolDescription, getToolName } from "@/lib/tool-i18n";
 import { cn, copyToClipboard } from "@/lib/utils";
 import { useAnalyticsStore } from "@/stores/analytics-store";
@@ -529,25 +530,15 @@ function GeneralSection() {
 
 /* ────────────────────── System ────────────────────── */
 
-// PUT /v1/settings rejects server-managed read-only keys (instance_id, cookie_secret)
-// with 400 READONLY_SETTING, and GET returns redacted secrets as the literal "********"
-// (cookie_secret, oidc_client_secret, siem_webhook_auth). Echoing either back breaks the
-// save or overwrites a real secret with the mask, so strip both before any bulk save.
-const READONLY_SETTING_KEYS = new Set(["instance_id", "cookie_secret"]);
-function writableSettings(settings: Record<string, string>): Record<string, string> {
-  return Object.fromEntries(
-    Object.entries(settings).filter(
-      ([key, value]) => !READONLY_SETTING_KEYS.has(key) && value !== "********",
-    ),
-  );
-}
-
 function SystemSection() {
   const { t } = useTranslation();
   const { role } = useAuth();
   const analyticsConfig = useAnalyticsStore((s) => s.config);
   const analyticsConfigLoaded = useAnalyticsStore((s) => s.configLoaded);
   const [settings, setSettings] = useState<Record<string, string>>({});
+  // Snapshot of the last server state, so a save sends only the fields this tab
+  // changed (never a stale value that could clobber another admin's concurrent edit).
+  const originalSettingsRef = useRef<Record<string, string>>({});
   const [loading, setLoading] = useState(true);
   const [saving, setSaving] = useState(false);
   const [saveMsg, setSaveMsg] = useState<string | null>(null);
@@ -557,15 +548,20 @@ function SystemSection() {
 
   useEffect(() => {
     apiGet<{ settings: Record<string, string> }>("/v1/settings")
-      .then((data) => setSettings(data.settings))
+      .then((data) => {
+        setSettings(data.settings);
+        originalSettingsRef.current = data.settings;
+      })
       .catch(() => {
         // Fallback defaults if endpoint not ready
-        setSettings({
+        const fallback = {
           fileUploadLimitMb: "100",
           defaultTheme: "system",
           defaultLocale: "en",
           loginAttemptLimit: "5",
-        });
+        };
+        setSettings(fallback);
+        originalSettingsRef.current = fallback;
       })
       .finally(() => setLoading(false));
   }, []);
@@ -596,18 +592,22 @@ function SystemSection() {
     setSaving(true);
     setSaveMsg(null);
     try {
-      await apiPut("/v1/settings", writableSettings(settings));
+      const changed = changedSettings(originalSettingsRef.current, settings);
-      if (settings.analyticsEnabled === "false") {
+      await apiPut("/v1/settings", writableSettings(changed));
-        const { optOut } = await import("@/lib/analytics");
+      if ("analyticsEnabled" in changed) {
-        optOut();
+        const { optIn, optOut } = await import("@/lib/analytics");
-      } else {
+        if (settings.analyticsEnabled === "false") optOut();
-        // Re-enabling takes effect on the next config refetch / reload.
+        else optIn();
+        // Refresh the cached config so this tab converges immediately: optOut()
+        // stops the SDKs but leaves the store enabled, which would keep feedback
+        // surfaces visible (and silently drop their submissions) until a refocus.
         useAnalyticsStore.getState().fetchConfig();
       }
       if (settings.defaultTheme) {
         const theme = settings.defaultTheme as "light" | "dark" | "system";
         useThemeStore.getState().setTheme(theme);
       }
+      originalSettingsRef.current = { ...settings };
       setSaveMsg(t.settings.system.saveSuccess);
     } catch {
       setSaveMsg(t.settings.system.saveFailed);
@@ -1076,13 +1076,20 @@ function SecuritySection() {
 function AdminSecuritySettings() {
   const { t } = useTranslation();
   const [settings, setSettings] = useState<Record<string, string>>({});
+  // Snapshot of the last server state; a save sends only the fields this tab changed
+  // so an unrelated edit here can never echo (and revert) another admin's change,
+  // such as an instance-wide analytics opt-out.
+  const originalSettingsRef = useRef<Record<string, string>>({});
   const [loading, setLoading] = useState(true);
   const [saving, setSaving] = useState(false);
   const [saveMsg, setSaveMsg] = useState<string | null>(null);
 
   useEffect(() => {
     apiGet<{ settings: Record<string, string> }>("/v1/settings")
-      .then((data) => setSettings(data.settings))
+      .then((data) => {
+        setSettings(data.settings);
+        originalSettingsRef.current = data.settings;
+      })
       .catch(() => {})
       .finally(() => setLoading(false));
   }, []);
@@ -1095,7 +1102,11 @@ function AdminSecuritySettings() {
     setSaving(true);
     setSaveMsg(null);
     try {
-      await apiPut("/v1/settings", writableSettings(settings));
+      await apiPut(
+        "/v1/settings",
+        writableSettings(changedSettings(originalSettingsRef.current, settings)),
+      );
+      originalSettingsRef.current = { ...settings };
       setSaveMsg(t.settings.security.securitySettingsSaved);
     } catch {
       setSaveMsg(t.settings.security.securitySettingsFailed);

apps/web/src/lib/analytics.ts

@@ -67,6 +67,14 @@ export async function initAnalytics(config: AnalyticsConfig): Promise<void> {
   }
 
   if (posthog) {
+    // Clear any persisted opt-out from a previous disabled period. opt_out_capturing()
+    // writes a localStorage flag that survives reloads, so without this a browser that
+    // once opted out would stay silent even after the instance re-enables analytics.
+    try {
+      posthog.opt_in_capturing();
+    } catch {
+      // ignore
+    }
     // app_version only; no instance_id, so plain events stay person-less.
     posthog.register({ app_version: (await import("@snapotter/shared")).APP_VERSION });
   }
@@ -160,3 +168,13 @@ export function optOut(): void {
     })
     .catch(() => {});
 }
+
+/** Reverse a prior optOut() in this tab: resume PostHog capture without a reload. */
+export function optIn(): void {
+  enabled = true;
+  try {
+    posthog?.opt_in_capturing();
+  } catch {
+    // ignore
+  }
+}

apps/web/src/lib/settings-payload.ts

@@ -0,0 +1,28 @@
+// PUT /v1/settings rejects server-managed read-only keys (instance_id, cookie_secret)
+// with 400 READONLY_SETTING, and GET returns redacted secrets as the literal "********".
+// Echoing either back breaks the save or overwrites a real secret with the mask, so strip
+// both before any bulk save.
+const READONLY_SETTING_KEYS = new Set(["instance_id", "cookie_secret"]);
+
+export function writableSettings(settings: Record<string, string>): Record<string, string> {
+  return Object.fromEntries(
+    Object.entries(settings).filter(
+      ([key, value]) => !READONLY_SETTING_KEYS.has(key) && value !== "********",
+    ),
+  );
+}
+
+// Only the keys this tab actually changed from the snapshot it loaded at mount.
+// Saving the whole settings blob lets a value captured at mount clobber another
+// admin's concurrent change, most dangerously flipping an instance-wide analytics
+// opt-out back on when saving an unrelated field.
+export function changedSettings(
+  original: Record<string, string>,
+  current: Record<string, string>,
+): Record<string, string> {
+  const out: Record<string, string> = {};
+  for (const [key, value] of Object.entries(current)) {
+    if (original[key] !== value) out[key] = value;
+  }
+  return out;
+}

apps/web/src/lib/with-timeout.ts

@@ -0,0 +1,10 @@
+// Reject if `promise` has not settled within `ms`. Guards UI that disables its
+// controls while a write is in flight: without a ceiling, a request that hangs
+// (connection black-holed, never errors) would leave the controls disabled forever.
+export function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
+  let timer: ReturnType<typeof setTimeout>;
+  const timeout = new Promise<never>((_, reject) => {
+    timer = setTimeout(() => reject(new Error(`timed out after ${ms}ms`)), ms);
+  });
+  return Promise.race([promise, timeout]).finally(() => clearTimeout(timer));
+}

tests/unit/web/analytics-no-leak.test.ts

@@ -234,4 +234,33 @@ describe("Analytics No-Leak Invariant (baked model)", () => {
       ).toBeNull();
     });
   });
+
+  describe("runtime opt-out / opt-in toggle", () => {
+    it("opts in to capturing when analytics initializes enabled (clears a stale persisted opt-out)", async () => {
+      await mod.initAnalytics(enabledConfig);
+      const instance = mockPosthogInit.mock.results.at(-1)?.value;
+      expect(instance.opt_in_capturing).toHaveBeenCalled();
+    });
+
+    it("optOut() stops track() and opts out of capturing", async () => {
+      await mod.initAnalytics(enabledConfig);
+      const instance = mockPosthogInit.mock.results.at(-1)?.value;
+      mockCapture.mockClear();
+      mod.optOut();
+      mod.track("tool_opened", { tool_id: "resize" });
+      expect(mockCapture).not.toHaveBeenCalled();
+      expect(instance.opt_out_capturing).toHaveBeenCalled();
+    });
+
+    it("optIn() resumes track() and opts back in after an optOut()", async () => {
+      await mod.initAnalytics(enabledConfig);
+      const instance = mockPosthogInit.mock.results.at(-1)?.value;
+      mod.optOut();
+      mockCapture.mockClear();
+      mod.optIn();
+      mod.track("tool_opened", { tool_id: "resize" });
+      expect(mockCapture).toHaveBeenCalledWith("tool_opened", { tool_id: "resize" });
+      expect(instance.opt_in_capturing).toHaveBeenCalled();
+    });
+  });
 });

tests/unit/web/settings-payload.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from "vitest";
+import { changedSettings, writableSettings } from "@/lib/settings-payload";
+
+describe("writableSettings", () => {
+  it("strips server-managed read-only keys that the PUT rejects", () => {
+    expect(
+      writableSettings({ instance_id: "abc", cookie_secret: "shh", defaultTheme: "dark" }),
+    ).toEqual({ defaultTheme: "dark" });
+  });
+
+  it("strips masked secret placeholders so a real secret is never overwritten by the mask", () => {
+    expect(writableSettings({ oidc_client_secret: "********", fileUploadLimitMb: "100" })).toEqual({
+      fileUploadLimitMb: "100",
+    });
+  });
+});
+
+describe("changedSettings", () => {
+  it("returns only keys whose value differs from the original snapshot", () => {
+    const original = { analyticsEnabled: "true", defaultTheme: "system" };
+    const current = { analyticsEnabled: "true", defaultTheme: "dark" };
+    expect(changedSettings(original, current)).toEqual({ defaultTheme: "dark" });
+  });
+
+  it("omits an unchanged analyticsEnabled so a stale save cannot revert an instance-wide opt-out", () => {
+    const original = { analyticsEnabled: "true", fileUploadLimitMb: "100" };
+    const current = { analyticsEnabled: "true", fileUploadLimitMb: "250" };
+    expect("analyticsEnabled" in changedSettings(original, current)).toBe(false);
+  });
+
+  it("includes a key the user actually toggled", () => {
+    expect(changedSettings({ analyticsEnabled: "true" }, { analyticsEnabled: "false" })).toEqual({
+      analyticsEnabled: "false",
+    });
+  });
+
+  it("includes keys added since the snapshot was taken", () => {
+    expect(changedSettings({}, { defaultLocale: "fr" })).toEqual({ defaultLocale: "fr" });
+  });
+});

tests/unit/web/tool-feedback-prompt.test.tsx

@@ -139,6 +139,17 @@ describe("ToolFeedbackPrompt", () => {
     expect(screen.getByText("How did this tool work?")).toBeDefined();
   });
 
+  it("stops nagging on the next result once shown, even without interaction", () => {
+    const { unmount } = render(<ToolFeedbackPrompt toolId="resize" />);
+    expect(screen.getByText("How did this tool work?")).toBeDefined();
+    unmount();
+
+    // A different tool finishes moments later. The user never touched the first
+    // prompt, but it must not reappear on the very next result.
+    render(<ToolFeedbackPrompt toolId="convert" />);
+    expect(screen.queryByText("How did this tool work?")).toBeNull();
+  });
+
   it("supports Don't ask again suppression", () => {
     const { unmount } = render(<ToolFeedbackPrompt toolId="resize" />);
 

tests/unit/web/with-timeout.test.ts

@@ -0,0 +1,26 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { withTimeout } from "@/lib/with-timeout";
+
+beforeEach(() => {
+  vi.useFakeTimers();
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+});
+
+describe("withTimeout", () => {
+  it("resolves with the promise value when it settles before the deadline", async () => {
+    await expect(withTimeout(Promise.resolve("ok"), 1000)).resolves.toBe("ok");
+  });
+
+  it("rejects when the promise is still pending past the deadline", async () => {
+    const neverSettles = new Promise<string>(() => {});
+    const settled = withTimeout(neverSettles, 1000).then(
+      () => "resolved",
+      (err: Error) => `rejected:${err.message}`,
+    );
+    await vi.advanceTimersByTimeAsync(1000);
+    expect(await settled).toMatch(/rejected:.*timed out/i);
+  });
+});