feat(consent): manage cookies from Settings, not from a card in the workspace - #6835
Conversation
… workspace The banner no longer mounts inside the workspace at all. The gate sits above the dynamic() boundary rather than inside the lazily-loaded module, so the product pays neither the consent chunk nor its init request on the surface with the most hard loads. A signed-in user manages the same choice from Settings -> Privacy, which shares one store with the banner: the options live in ConsentStoreProvider and are not exported, so two call sites cannot drift into two stores. The banner also stops pinning the light token layer and simply inherits. The cause it was working around is that LandingShell pins light on a wrapper inside the page while <html> keeps the visitor's theme, so landing routes missing from ThemeProvider's hand-written list rendered light pages under dark root chrome. LANDING_ROUTES becomes one source of truth in lib/landing/routes, read by both next.config (COEP) and ThemeProvider (forced light) -- the same drift that let /cookie-policy ship without its COEP exemption. Diffed old against new across every real route: 16 landing routes gain the correct theme and nothing regresses. /cli/auth and /credential-groups/complete are added too; both render AuthShell and were never covered. Verified the shared-store assumption directly rather than trusting the docs: getOrCreateConsentRuntime returns the same store and manager for equal options.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Workspace: Shared consent layer: Theme / routing: Reviewed by Cursor Bugbot for commit 15dd69a. Configure here. |
Greptile SummaryThe PR moves hosted cookie-consent controls into unified workspace Settings and prevents the floating consent runtime from loading on workspace routes. It also centralizes landing-route classification for theme and COEP behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported Privacy account-settings fallthrough is eliminated by keeping Privacy exclusively on the hosted unified settings surface.
|
| Filename | Overview |
|---|---|
| apps/sim/components/settings/navigation.ts | Registers Privacy only in hosted unified settings, removing the account-plane projection that caused the previously reported renderer fallthrough. |
| apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx | Renders Privacy for hosted workspace settings and safely redirects self-hosted requests to General. |
| apps/sim/app/workspace/[workspaceId]/settings/components/privacy/privacy.tsx | Adds cookie-preference editing with dirty-state tracking, save/discard actions, and consent-store integration. |
| apps/sim/app/_shell/consent/consent-provider.tsx | Prevents the consent runtime and its dynamic chunk from loading on workspace paths. |
| apps/sim/app/_shell/providers/theme-provider.tsx | Replaces prefix-based forced-light classification with exact first-segment membership from centralized route definitions. |
| apps/sim/lib/landing/routes.ts | Defines the shared landing-route segments consumed by theme and COEP configuration. |
| apps/sim/next.config.ts | Reuses the centralized landing-route list without changing the effective COEP exemption set. |
Reviews (2): Last reviewed commit: "fix(consent): keep Privacy on one settin..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Projecting the section into the account plane put it in a catalog that buildPlaneSettingsItems does not gate on requiresHosted, so a self-hosted deployment would list a Privacy entry, and AccountSettingsRenderer's catch-all rendered Mothership for it. The unified settings already gate the section and redirect self-hosted deployments to General, so the section lives there only.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 15dd69a. Configure here.
1 participant
Summary
Follow-up to #6832. The consent banner mounted in the root layout, so it appeared on every page — including inside the product. This moves in-app consent to Settings and fixes the theme mismatch the banner was working around rather than papering over it.
The banner never appears in the workspace
ConsentProviderreturns before thedynamic()boundary on any/workspacepath, so the product pays neither the consent chunk nor its init request — the surface with the most hard page loads. Gating inside the lazily-loaded module would still have downloaded it.Consent inside the product is managed from Settings → Privacy instead, which is the documented pattern for authenticated surfaces: a floating card is the wrong shape once someone is signed in.
One store, structurally
The banner sits behind an
ssr: falseboundary that cannot wrap the app, so nothing reaches its store through React context. Both surfaces therefore mount their own provider — butgetOrCreateConsentRuntimecaches manager and store by the option values, so they share one of each.CONSENT_OPTIONSnow lives insideConsentStoreProviderand is not exported, which makes that invariant unbreakable rather than conventional.Verified directly rather than trusting the docs:
The theme fix, at the cause
The banner previously pinned the
lighttoken layer with a comment explaining why. The actual problem is thatLandingShellpinslighton a wrapper inside the page while<html>keeps the visitor's theme, so any landing route missing fromThemeProvider's hand-written list rendered a light page under dark root chrome — scrollbars,color-scheme, and anything portalled to<body>.LANDING_ROUTESbecomes one source of truth inlib/landing/routes.ts, read by bothnext.config.ts(COEP exemptions) andThemeProvider(forced light). That is the same drift that let/cookie-policyship without its COEP exemption in the last PR. The banner now simply inherits the theme.Two pre-existing gaps closed along the way:
/cli/authand/credential-groups/completeboth renderAuthShell(which pins light) and were in neither list.Type of Change
Testing
type-check,lint:check, all 29check:audits, and 155 tests pass.<html>theme, nothing regresses in the other direction./pricingunder a dark system theme renders<html class="light">with a light card;/playgroundrenders<html class="dark">with a#1b1b1bcard. The banner no longer decides for itself./workspaceneeds auth, so the gate is covered by tests asserting the dynamic loader is never invoked on/workspace,/workspace/abc,/workspace/abc/logs— verified those fail when the gate is removed.ThemeProviderchange: it moves fromstartsWithprefix matching to first-segment set membership, and that route-classification diff is the riskiest part of this PR.Known gap (unchanged from #6832)
Prior blocking is still not implemented — GTM, GA, and PostHog load before consent. This PR makes that more load-bearing, not less: a visitor who reaches the workspace with no consent record is now never prompted. It closes properly when the analytics scripts move behind consent, since nothing non-essential loads without a record at all.
Checklist