Dismiss cookie banner when the user accepts - #34

Review: automated, review-only (no code changes)

What this PR actually changes

One file, apps/web/src/components/cookie-banner.tsx, with two independent changes bundled together:

1. Hardening — the accept/decline handlers now set React state before calling posthog.opt_in_capturing() / opt_out_capturing(), and wrap the SDK call in try/catch with posthog.captureException(error), so a throwing SDK call can no longer prevent the banner from unmounting.
2. A new local persistence layer — a songup_cookie_consent localStorage key written on every choice, which on mount takes precedence over PostHog's own status: readStoredConsent() ?? posthog.get_explicit_consent_status().

Nothing other than banner visibility is derived from the new key. Rendering/hydration is untouched (initial state is still "" + rAF), so there's no banner flash and no SSR mismatch.

Verdict: needs changes

Change (1) is a reasonable, cheap fix for the reported symptom and is worth taking as-is. Change (2) is where the problems are: it makes localStorage the sole gate on the banner without ever reconciling that value back into the SDK, converting a self-healing failure into a permanent, silent one.

Claims below were verified against the actual posthog-js@1.399.1 bundle, not inferred from the PR description.

1. CONFIRMED — high — apps/web/src/components/cookie-banner.tsx:53 (with :62)

Stored consent overrides PostHog's state but is never applied to it, so a failed opt-in becomes permanent and unrecoverable.

readStoredConsent() short-circuits get_explicit_consent_status(), and nothing anywhere re-calls opt_in_capturing() for a stored "granted".

In posthog-js 1.399.1, opt_in_capturing() constructs a SessionIdManager (which throws "SessionIdManager requires a PostHogPersistence instance") before calling consent.optInOut(true). Failure scenario:

• Host clicks Accept → the SessionIdManager construction throws → consent is never written to __ph_opt_in_out_<token> → the banner hides anyway and songup_cookie_consent=granted is written.
• On every subsequent load, the stored value hides the banner while PostHog's status stays "pending".
• With cookieless_mode: "on_reject" (apps/web/src/instrumentation-client.ts:6), is_capturing() for pending consent is false → no events are ever captured in that browser again, and the user cannot retry because the banner never reappears.

Before this PR the same failure self-healed: the banner came back on the next load and a second click worked.

Fix: on mount, if the stored choice is granted/denied while get_explicit_consent_status() === "pending", re-apply opt_in_capturing() / opt_out_capturing() rather than only suppressing the UI.

(The mirror case — Decline throwing — fails safe under this config, since pending also means not capturing.)

2. CONFIRMED — medium — apps/web/src/components/cookie-banner.tsx:67 (and :77)

The captureException in the catch is dropped precisely in the failure mode it was added to observe.

PostHog.capture() is guarded by if (this.__loaded && this.persistence && this.sessionPersistence && ...) if (this.is_capturing()). In the finding-1 scenario the throw happens before consent is recorded, so consent is pending, is_capturing() is false, and the $exception is silently discarded — the stated goal ("so the next failure reaches error tracking") does not hold for accept-path failures.

Worse, the throwing case is by hypothesis one where the SDK is in a bad state (this.persistence missing), which is the same condition that makes capture() a no-op. To make this error visible, report it through something that doesn't depend on the SDK's consent/persistence state.

3. PLAUSIBLE — low — apps/web/src/components/cookie-banner.tsx:67

posthog.captureException() inside the catch is itself unguarded.

If the SDK is broken enough that opt_in_capturing() throws, captureException can throw too, and nothing catches it — an unhandled error escapes the click handler. The banner still closes (storeConsent/setConsentGiven already ran), so the practical impact is console/window.onerror noise rather than a stuck banner, but the "cannot throw" invariant isn't actually complete. A bare try {} catch {} around the reporting call would close it.

4. CONFIRMED — affects the justification more than the code

Both central premises of the PR description appear to be wrong.

• "cookieless_mode: "on_reject" … means the consent write is not reliable" — it doesn't. Consent lives in a dedicated store (opt_out_capturing_persistence_type defaults to "localStorage", key __ph_opt_in_out_<token>) written by consent.optInOut(); it is not routed through the cookieless persistence that cookieless_mode disables. There is no mechanism by which on_reject drops the consent write.
• The cited session-recording evidence ($autocapture → $dead_click → $opt_in) is inconsistent with a lost consent write: capture("$opt_in") runs only after consent.optInOut(true) has already succeeded, so in every session where $opt_in was seen, the choice was persisted and get_explicit_consent_status() would have returned "granted" on the next mount.

Those sessions are explained entirely by a throw late in opt_in_capturing() (after $opt_in, e.g. in the pageview/surveys tail) blocking the old pre-reorder setConsentGiven — which the try/catch alone fixes. The localStorage layer isn't needed for the reported bug, and per finding 1 it introduces a new one.

5. Privacy/compliance — pre-existing, but this PR interacts with it

apps/web/src/components/identification-provider.tsx:17 calls posthog.opt_in_capturing() for any signed-in non-anonymous user without consulting consent, so an explicit Decline is overridden on sign-in. That is not introduced here, but after this PR the two records can openly disagree (songup_cookie_consent=denied in localStorage while PostHog is opted in) with no reconciliation, which makes the existing gap harder to notice.

Separately, this banner is the only consent UI in the repo (no other opt_in_capturing / get_explicit_consent_status call sites besides these two), so there is no way to change a choice once made — and this PR adds a second key that must also be cleared to get the banner back. Worth a decision, not a blocker on its own.

6. Nits

• storeConsent(consent: CookieConsent) at :38 accepts "pending", which readStoredConsent will never honor. Narrowing the parameter to "granted" | "denied" makes a future storeConsent("pending") a type error rather than a silently ignored write.
• The storage key has no version suffix, so a future privacy-policy change can't force a re-prompt.

CI

All four checks green on 6d4c586: CodeQL, Analyze (javascript-typescript), Analyze (python), and the Vercel preview. No test or typecheck workflow runs on this PR beyond CodeQL, so the "check-types and lint pass" claim is unverified by CI. No tests were added; there are no existing tests for this component to regress.

Suggested resolution

Land the reorder + try/catch, and either drop the localStorage layer entirely or make it reconcile on mount (finding 1) — reading the stored choice and, when PostHog still reports "pending", re-issuing the corresponding opt-in/opt-out call. Also route the swallowed-error report somewhere that survives a pending or broken SDK (finding 2).

Generated by Claude Code