fix(cms): keep watching a hover entry until it is really visible (#8263)

The hover entry sampled its visibility once, one animation frame after the
state update that removes `hidden`. When that frame won the race with React's
commit the host was still `display: none` and reported no box, and because
nothing ever looked again the Test acceptance receipt was suppressed for the
whole session — the entry then displayed normally, so the campaign sat in the
CMS waiting on a receipt that could no longer arrive.

Visibility now resolves through one shared watch that re-checks on layout,
on page visibility and after re-mount, reports at most once, and keeps
watching a slow host instead of giving up on it. `mountTouchpoint` moves onto
the same watch so the modal and badge hosts stop carrying a second, weaker
copy of this rule.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

apps/web/src/components/HoverTouchpointOverlay.tsx

@@ -12,6 +12,7 @@ import {
 	webTouchpointContext,
 	verifyWebTouchpoint,
 } from "./touchpoint-component";
+import { watchTouchpointVisibility } from "./touchpoint-lifecycle";
 import styles from "./HoverTouchpointOverlay.module.css";
 
 const ENTRY_PLACEMENT = "opend.home.hover-entry";
@@ -280,11 +281,9 @@ export function HoverTouchpointOverlay({
 					},
 				);
 				if (abandon()) return;
+				// Visibility is decided by the effect below, once React has
+				// committed `hidden`. Sampling it here races that commit.
 				setReady(true);
-				requestAnimationFrame(() => {
-					if (!cancelled && isAuthorized() && !document.hidden && entryElement?.getClientRects().length)
-						onEntryVisible?.();
-				});
 			} catch (error) {
 				dispose();
 				if (!cancelled) {
@@ -301,19 +300,31 @@ export function HoverTouchpointOverlay({
 			setReady(false);
 			dispose();
 		};
-	}, [close, dispatchEntryAction, dispatchLayerAction, elementReady, entry, entryActionIds, isAuthorized, layer, layerActionIds, mode, onDiagnostic, onEntryVisible, onLayerVisible]);
+	}, [close, dispatchEntryAction, dispatchLayerAction, elementReady, entry, entryActionIds, isAuthorized, layer, layerActionIds, mode, onDiagnostic]);
 
 	useLayoutEffect(() => {
 		if (open && ready) refreshPosition();
 	}, [open, ready, refreshPosition]);
 	useEffect(() => {
-		if (!open || !ready) return;
-		const frame = requestAnimationFrame(() => {
-			if (!document.hidden && layerRef.current?.getClientRects().length)
-				onLayerVisible?.();
+		const element = entryRef.current;
+		if (!ready || !element || !onEntryVisible) return;
+		return watchTouchpointVisibility({
+			element,
+			isCurrent: isAuthorized,
+			onVisible: onEntryVisible,
+			onSlow: onDiagnostic,
 		});
-		return () => cancelAnimationFrame(frame);
-	}, [onLayerVisible, open, ready]);
+	}, [isAuthorized, onDiagnostic, onEntryVisible, ready]);
+	useEffect(() => {
+		const element = layerRef.current;
+		if (!open || !ready || !element || !onLayerVisible) return;
+		return watchTouchpointVisibility({
+			element,
+			isCurrent: isAuthorized,
+			onVisible: onLayerVisible,
+			onSlow: onDiagnostic,
+		});
+	}, [isAuthorized, onDiagnostic, onLayerVisible, open, ready]);
 	useEffect(() => {
 		if (!open || !ready) return;
 		const reposition = () => refreshPosition();

apps/web/src/components/touchpoint-lifecycle.ts

@@ -312,6 +312,87 @@ export function useTouchpointLifecycle<T>({ enabled, identity, load, onError }:
 }
 
 
+/**
+ * A host that is laid out but still reports no box has not been committed yet;
+ * one that reports a box while the page is hidden was never shown. Neither can
+ * be decided from a single sample, so the warning only reports, never resolves.
+ */
+export const VISIBILITY_WARNING_MS = 5_000;
+
+export type TouchpointVisibilityWatch = Readonly<{
+	element: HTMLElement;
+	isCurrent: () => boolean;
+	onVisible: () => void;
+	onSlow?: (code: string) => void;
+	slowAfterMs?: number;
+}>;
+
+/**
+ * Resolves the first moment a mounted host is really on screen, then stops.
+ *
+ * Sampling once cannot answer this. `hidden` is bound to React state, and a
+ * frame scheduled in the same continuation as that state update can run before
+ * React commits it — the host is then still `display: none` and reports no box.
+ * Because the old callers never looked again, that one lost sample permanently
+ * suppressed the receipt for the whole session. Three sources can change the
+ * answer, so all three re-check: layout (`ResizeObserver`, which also fires the
+ * initial observation), page visibility, and the caller's own re-mount.
+ */
+export function watchTouchpointVisibility({
+	element,
+	isCurrent,
+	onVisible,
+	onSlow,
+	slowAfterMs = VISIBILITY_WARNING_MS,
+}: TouchpointVisibilityWatch): () => void {
+	let recorded = false;
+	let stopped = false;
+	let frame: number | undefined;
+	let observer: ResizeObserver | undefined;
+	let slowTimer: ReturnType<typeof setTimeout> | undefined;
+	const stop = () => {
+		if (stopped) return;
+		stopped = true;
+		if (frame !== undefined) cancelAnimationFrame(frame);
+		frame = undefined;
+		observer?.disconnect();
+		if (slowTimer !== undefined) clearTimeout(slowTimer);
+		document.removeEventListener("visibilitychange", check);
+	};
+	function check() {
+		if (stopped || recorded || frame !== undefined) return;
+		frame = requestAnimationFrame(() => {
+			frame = undefined;
+			if (stopped || recorded) return;
+			if (
+				!isCurrent() ||
+				document.hidden ||
+				!element.isConnected ||
+				element.hidden ||
+				element.getClientRects().length === 0
+			)
+				return;
+			recorded = true;
+			stop();
+			onVisible();
+		});
+	}
+	// Layout is the strongest signal but the only optional one: a host without
+	// `ResizeObserver` must still mount and still report, so its absence costs
+	// this watch a wake-up source and never the display itself.
+	observer =
+		typeof ResizeObserver === "function" ? new ResizeObserver(check) : undefined;
+	observer?.observe(element);
+	document.addEventListener("visibilitychange", check);
+	// A slow host is reported but keeps its watch: a late box still earns its
+	// receipt, and dropping the watch here would recreate the lost-sample bug.
+	slowTimer = setTimeout(() => {
+		if (!recorded && !stopped) onSlow?.("touchpoint_visibility_slow");
+	}, slowAfterMs);
+	check();
+	return stop;
+}
+
 type MountAdapter = Readonly<{
 	content: WebTouchpointContent;
 	placementKey: string;
@@ -342,9 +423,7 @@ export function mountTouchpoint(
 		elementDisposed = false,
 		verifiedDisposed = false;
 	let verified: Awaited<ReturnType<typeof verifyWebTouchpoint>> | undefined;
-	let frame: number | undefined;
-	let mounted = false,
-		recorded = false;
+	let stopVisibilityWatch: (() => void) | undefined;
 	let observer: MutationObserver | undefined;
 	const current = () => !cancelled && adapter.isCurrent();
 	const dispose = () => {
@@ -357,23 +436,6 @@ export function mountTouchpoint(
 			verified.dispose();
 		}
 	};
-	const recordWhenVisible = () => {
-		if (!mounted || recorded || frame !== undefined || !adapter.onVisible)
-			return;
-		frame = requestAnimationFrame(() => {
-			frame = undefined;
-			if (
-				!current() ||
-				document.hidden ||
-				!element.isConnected ||
-				element.hidden ||
-				element.getClientRects().length === 0
-			)
-				return;
-			recorded = true;
-			adapter.onVisible?.();
-		});
-	};
 	const fail = (code: string) => {
 		emitWebTouchpointDiagnostic({ code });
 		adapter.onCloseControlChange?.(false);
@@ -381,7 +443,6 @@ export function mountTouchpoint(
 	};
 	adapter.onCloseControlChange?.(null);
 	container.replaceChildren(element);
-	document.addEventListener("visibilitychange", recordWhenVisible);
 	void (async () => {
 		try {
 			verified = await verifyWebTouchpoint(adapter.content);
@@ -440,7 +501,14 @@ export function mountTouchpoint(
 				dispose();
 				return;
 			}
-			mounted = true;
+			const onVisible = adapter.onVisible;
+			if (onVisible)
+				stopVisibilityWatch = watchTouchpointVisibility({
+					element,
+					isCurrent: current,
+					onVisible,
+					onSlow: (code) => emitWebTouchpointDiagnostic({ code }),
+				});
 			if (adapter.onCloseControlChange) {
 				const update = () => {
 					if (current())
@@ -471,7 +539,6 @@ export function mountTouchpoint(
 				if (dialog) observer.observe(dialog, options);
 			}
 			adapter.onReady?.();
-			recordWhenVisible();
 		} catch (error) {
 			if (current())
 				fail(error instanceof Error ? error.message : "touchpoint_load_failed");
@@ -481,8 +548,7 @@ export function mountTouchpoint(
 	return () => {
 		cancelled = true;
 		observer?.disconnect();
-		document.removeEventListener("visibilitychange", recordWhenVisible);
-		if (frame !== undefined) cancelAnimationFrame(frame);
+		stopVisibilityWatch?.();
 		adapter.onCloseControlChange?.(null);
 		dispose();
 		if (element.parentNode === container) container.replaceChildren();

apps/web/tests/components/TestCampaignHosts.test.tsx

@@ -106,15 +106,38 @@ function decision(placementKey: (typeof placements)[number]): TestDecision {
 }
 
 describe("Test decisions at the existing host touchpoints", () => {
+	/**
+	 * A host reports no box until React commits `hidden` and the package lays
+	 * out. Pinning this to 1 for every element would assert away the very frame
+	 * the receipt is lost in, so tests drive it and wake the observer by hand.
+	 */
+	let clientRectCount = 1;
+	const resizeObserverCallbacks: Array<() => void> = [];
+	const notifyResizeObservers = () => {
+		for (const trigger of [...resizeObserverCallbacks]) trigger();
+	};
 	beforeEach(() => {
 		localStorage.clear();
+		clientRectCount = 1;
+		resizeObserverCallbacks.length = 0;
 		vi.stubEnv("NEXT_PUBLIC_CMS_HOST_RELEASE", `sha256:${"a".repeat(64)}`);
 		document.documentElement.lang = "zh-CN";
 		vi.stubGlobal(
 			"ResizeObserver",
 			class {
-				observe() {}
-				disconnect() {}
+				private trigger?: () => void;
+				constructor(private readonly callback: ResizeObserverCallback) {}
+				observe() {
+					this.trigger = () =>
+						this.callback([], this as unknown as ResizeObserver);
+					resizeObserverCallbacks.push(this.trigger);
+				}
+				disconnect() {
+					const index = this.trigger
+						? resizeObserverCallbacks.indexOf(this.trigger)
+						: -1;
+					if (index >= 0) resizeObserverCallbacks.splice(index, 1);
+				}
 			},
 		);
 		(globalThis as HostGlobal).__cmsTestHost = {
@@ -133,10 +156,13 @@ describe("Test decisions at the existing host touchpoints", () => {
 					document.createTextNode("Test host content"),
 				);
 			});
-		vi.spyOn(HTMLElement.prototype, "getClientRects").mockReturnValue({
-			length: 1,
-			item: () => null,
-		} as unknown as DOMRectList);
+		vi.spyOn(HTMLElement.prototype, "getClientRects").mockImplementation(
+			() =>
+				({
+					length: clientRectCount,
+					item: () => null,
+				}) as unknown as DOMRectList,
+		);
 	});
 	afterEach(() => {
 		clearTestRuntimeSession();
@@ -732,4 +758,56 @@ describe("Test decisions at the existing host touchpoints", () => {
 				.querySelector("opend-touchpoint"),
 		).not.toBeNull();
 	});
+
+	// Delayed visibility itself is covered directly in touchpointVisibility.test.ts:
+	// at this level a re-mount can supply a second sample, which would hide a
+	// regression back to sampling once. What this level can still pin down is
+	// that a host nobody could see never earns a receipt.
+	it("never accepts a hover entry that never gains a box", async () => {
+		clientRectCount = 0;
+		const decisions = new Map(
+			placements.map((placementKey) => [placementKey, decision(placementKey)]),
+		);
+		setTestRuntimeSession({
+			selectionKey: "deployment-four:sha256:four-snapshot:active",
+			deployment: {
+				id: context.deploymentId,
+				activityId: "activity-four",
+				snapshotHash: "sha256:four-snapshot",
+				snapshot: {
+					contentVersionId: "version-four-placement",
+					manifestHash: "sha256:four-manifest",
+					artifactHash: "sha256:four-artifact",
+					placementKeys: [...placements],
+				},
+			},
+			context,
+			decisions,
+			isAuthorized: () => true,
+		});
+		const fetchMock = vi.fn(async (url: string, _init?: RequestInit) =>
+			url.includes("acceptances")
+				? new Response(JSON.stringify({ id: "acceptance" }), { status: 201 })
+				: new Response(JSON.stringify({ error: "production_read_forbidden" }), {
+						status: 404,
+					}),
+		);
+		vi.stubGlobal("fetch", fetchMock);
+		render(<ProductionCampaignHover authenticated sessionSubject="account-a" />);
+		const entry = (await screen.findByTestId(
+			"cms-hover-overlay-root",
+		)).querySelector("opend-touchpoint");
+		expect(entry).not.toBeNull();
+		await waitFor(() => expect(entry).not.toHaveAttribute("hidden"));
+		const entryAcceptances = () =>
+			fetchMock.mock.calls
+				.filter(([url]) => url.includes("acceptances"))
+				.map(([, init]) => JSON.parse(String(init?.body)))
+				.filter((report) => report.placementKey === "opend.home.hover-entry");
+		notifyResizeObservers();
+		await act(async () => {
+			await new Promise((resolve) => setTimeout(resolve, 50));
+		});
+		expect(entryAcceptances()).toHaveLength(0);
+	});
 });

apps/web/tests/components/touchpointVisibility.test.ts

@@ -0,0 +1,172 @@
+// @vitest-environment jsdom
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { watchTouchpointVisibility } from "../../src/components/touchpoint-lifecycle";
+
+/**
+ * These cover the frame the acceptance receipt used to be lost in. The old
+ * callers sampled visibility once, right after the state update that removes
+ * `hidden`; when that sample lost the race with React's commit the host was
+ * still `display: none`, reported no box, and nothing ever looked again.
+ */
+describe("watchTouchpointVisibility", () => {
+	let rectCount = 1;
+	let hidden = false;
+	const observerCallbacks: Array<() => void> = [];
+	const notifyResize = () => {
+		for (const trigger of [...observerCallbacks]) trigger();
+	};
+	const settleFrames = async () => {
+		for (let i = 0; i < 3; i++)
+			await new Promise((resolve) => requestAnimationFrame(() => resolve(null)));
+	};
+	const host = () => {
+		const element = document.createElement("div");
+		document.body.append(element);
+		return element;
+	};
+
+	beforeEach(() => {
+		rectCount = 1;
+		hidden = false;
+		observerCallbacks.length = 0;
+		// Registering on `observe`, not on construction, is what makes these
+		// tests able to tell an observed host from a merely constructed observer.
+		vi.stubGlobal(
+			"ResizeObserver",
+			class {
+				private trigger?: () => void;
+				constructor(private readonly callback: ResizeObserverCallback) {}
+				observe() {
+					this.trigger = () =>
+						this.callback([], this as unknown as ResizeObserver);
+					observerCallbacks.push(this.trigger);
+				}
+				disconnect() {
+					const index = this.trigger
+						? observerCallbacks.indexOf(this.trigger)
+						: -1;
+					if (index >= 0) observerCallbacks.splice(index, 1);
+				}
+			},
+		);
+		vi.spyOn(document, "hidden", "get").mockImplementation(() => hidden);
+		vi.spyOn(HTMLElement.prototype, "getClientRects").mockImplementation(
+			() => ({ length: rectCount, item: () => null }) as unknown as DOMRectList,
+		);
+	});
+	afterEach(() => {
+		document.body.replaceChildren();
+		vi.unstubAllGlobals();
+		vi.restoreAllMocks();
+	});
+
+	it("waits for a host that only gains its box after the first frame", async () => {
+		rectCount = 0;
+		const onVisible = vi.fn();
+		const stop = watchTouchpointVisibility({
+			element: host(),
+			isCurrent: () => true,
+			onVisible,
+		});
+		await settleFrames();
+		expect(onVisible).not.toHaveBeenCalled();
+		rectCount = 1;
+		notifyResize();
+		await settleFrames();
+		expect(onVisible).toHaveBeenCalledTimes(1);
+		stop();
+	});
+
+	it("reports a host at most once however often its layout changes", async () => {
+		const onVisible = vi.fn();
+		const stop = watchTouchpointVisibility({
+			element: host(),
+			isCurrent: () => true,
+			onVisible,
+		});
+		await settleFrames();
+		notifyResize();
+		notifyResize();
+		await settleFrames();
+		expect(onVisible).toHaveBeenCalledTimes(1);
+		stop();
+	});
+
+	it("does not report a laid-out host while the page is hidden, and recovers when it returns", async () => {
+		hidden = true;
+		const onVisible = vi.fn();
+		const stop = watchTouchpointVisibility({
+			element: host(),
+			isCurrent: () => true,
+			onVisible,
+		});
+		await settleFrames();
+		expect(onVisible).not.toHaveBeenCalled();
+		hidden = false;
+		document.dispatchEvent(new Event("visibilitychange"));
+		await settleFrames();
+		expect(onVisible).toHaveBeenCalledTimes(1);
+		stop();
+	});
+
+	it("never reports a host whose authority lapsed while it was waiting", async () => {
+		rectCount = 0;
+		let authorized = true;
+		const onVisible = vi.fn();
+		const stop = watchTouchpointVisibility({
+			element: host(),
+			isCurrent: () => authorized,
+			onVisible,
+		});
+		await settleFrames();
+		authorized = false;
+		rectCount = 1;
+		notifyResize();
+		await settleFrames();
+		expect(onVisible).not.toHaveBeenCalled();
+		stop();
+	});
+
+	it("stops watching once released, so a later layout cannot report a released host", async () => {
+		rectCount = 0;
+		const onVisible = vi.fn();
+		const stop = watchTouchpointVisibility({
+			element: host(),
+			isCurrent: () => true,
+			onVisible,
+		});
+		stop();
+		rectCount = 1;
+		notifyResize();
+		document.dispatchEvent(new Event("visibilitychange"));
+		await settleFrames();
+		expect(onVisible).not.toHaveBeenCalled();
+	});
+
+	it("reports a slow host without giving up on it", async () => {
+		vi.useFakeTimers();
+		try {
+			rectCount = 0;
+			const onVisible = vi.fn();
+			const onSlow = vi.fn();
+			const stop = watchTouchpointVisibility({
+				element: host(),
+				isCurrent: () => true,
+				onVisible,
+				onSlow,
+				slowAfterMs: 1_000,
+			});
+			vi.advanceTimersByTime(1_500);
+			expect(onSlow).toHaveBeenCalledWith("touchpoint_visibility_slow");
+			expect(onVisible).not.toHaveBeenCalled();
+			rectCount = 1;
+			notifyResize();
+			await vi.advanceTimersByTimeAsync(100);
+			expect(onVisible).toHaveBeenCalledTimes(1);
+			stop();
+		} finally {
+			vi.useRealTimers();
+		}
+	});
+});