From 56161065b07f7db2d4c24db228b10bb2fbdafdb9 Mon Sep 17 00:00:00 2001 From: MP2EZ <182439403+MP2EZ@users.noreply.github.com> Date: Sun, 31 May 2026 11:37:11 -0700 Subject: [PATCH] chore: remove dead scaffolding + guard GPC cookie clear (audit cleanup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cuts the bloat findings from /m:audit --bloat — all scaffolding for paths the architecture deliberately doesn't use. Zero behavior change. - Remove trackConversion() + ConversionEvent (lib/ab-testing.ts): the function only console.log'd in dev; real conversion tracking is the variant written to Notion in the waitlist route (its own comment admitted this). Dropped the call site, import, and 4 tests. - Remove getGpcFromCookie() (lib/gpc.ts): exported "for server components" but zero production callers — GPC is read client-side in AnalyticsGate/GpcNotice and request-side in middleware. Dropping it also removes the only next/headers cookies() import. Dropped 4 tests. - Remove POSTHOG_ASSETS_HOST (lib/posthog/config.ts): exported, never imported. The CSP uses literal strings; init only sets api_host. - Trim phase-2-in-comments (lib/posthog/events.ts) and restating JSDoc (lib/gpc.ts). Kept the @see INFRA refs and load-bearing compliance comments. Perf finding (PERF-01): middleware now only clears being_gpc when the cookie is actually present (request.cookies.has), instead of emitting a needless clearing Set-Cookie on every no-GPC request — that Set-Cookie on ~all traffic defeats shared/CDN caching of HTML responses. Updated the middleware test accordingly (clears-when-present + does-not-emit- when-absent). Also: add /.open-next/ to .gitignore. The OpenNext Cloudflare build output (27MB) was untracked-but-not-ignored, so a local `opennextjs-cloudflare build` pollutes git status and makes `eslint` glob 12k+ generated files. Pre-existing gap, fixed here since it bit this very cleanup. Net: ~70 LOC removed, 80 → 75 tests (removed 6 dead-path tests, added 1 middleware test). lint + typecheck clean, build + worker build OK. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 3 +++ app/api/waitlist/route.test.ts | 6 +----- app/api/waitlist/route.ts | 7 +------ lib/ab-testing.test.ts | 26 ------------------------ lib/ab-testing.ts | 36 --------------------------------- lib/gpc.test.ts | 37 ---------------------------------- lib/gpc.ts | 27 +++---------------------- lib/posthog/config.ts | 1 - lib/posthog/events.ts | 17 +++++----------- middleware.test.ts | 18 +++++++++++++---- middleware.ts | 5 ++++- 11 files changed, 31 insertions(+), 152 deletions(-) diff --git a/.gitignore b/.gitignore index 2019ca9..3055ac7 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,9 @@ /.next/ /out/ +# opennext / cloudflare workers build output +/.open-next/ + # production /build diff --git a/app/api/waitlist/route.test.ts b/app/api/waitlist/route.test.ts index 635fcb5..eb6be4a 100644 --- a/app/api/waitlist/route.test.ts +++ b/app/api/waitlist/route.test.ts @@ -3,7 +3,6 @@ import { NextRequest } from 'next/server'; vi.mock('@/lib/ab-testing', () => ({ getVariant: vi.fn(), - trackConversion: vi.fn(), })); function buildRequest(body: unknown): NextRequest { @@ -94,10 +93,9 @@ describe('POST /api/waitlist', () => { expect(sent.properties['Signed Up'].date.start).toMatch( /^\d{4}-\d{2}-\d{2}T/, ); - expect(ab.trackConversion).not.toHaveBeenCalled(); }); - it('posts to Notion with Variant and tracks conversion when variant assigned', async () => { + it('posts to Notion with Variant when a variant is assigned', async () => { const { route, ab } = await importRouteFresh(); (ab.getVariant as ReturnType).mockResolvedValue('A'); const fetchSpy = vi @@ -111,7 +109,6 @@ describe('POST /api/waitlist', () => { (fetchSpy.mock.calls[0]![1] as RequestInit).body as string, ); expect(sent.properties.Variant).toEqual({ select: { name: 'A' } }); - expect(ab.trackConversion).toHaveBeenCalledWith('A', 'waitlist_signup'); }); it('returns 500 when Notion API responds non-ok', async () => { @@ -125,7 +122,6 @@ describe('POST /api/waitlist', () => { expect(res.status).toBe(500); expect(await res.json()).toEqual({ error: 'Failed to join waitlist' }); - expect(ab.trackConversion).not.toHaveBeenCalled(); }); it("returns 400 'Invalid JSON' when request body is malformed", async () => { diff --git a/app/api/waitlist/route.ts b/app/api/waitlist/route.ts index 9993e12..eaa3f86 100644 --- a/app/api/waitlist/route.ts +++ b/app/api/waitlist/route.ts @@ -8,7 +8,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { z } from 'zod'; -import { getVariant, trackConversion } from '@/lib/ab-testing'; +import { getVariant } from '@/lib/ab-testing'; const NOTION_TOKEN = process.env.NOTION_TOKEN; const NOTION_DATABASE_ID = process.env.NOTION_WAITLIST_DB_ID; @@ -96,11 +96,6 @@ export async function POST(request: NextRequest) { throw new Error('Failed to add to Notion'); } - // Track conversion for A/B analysis - if (variant) { - trackConversion(variant, 'waitlist_signup'); - } - return NextResponse.json({ success: true }); } catch (error) { console.error('Waitlist error:', error); diff --git a/lib/ab-testing.test.ts b/lib/ab-testing.test.ts index fa64079..41f2cfd 100644 --- a/lib/ab-testing.test.ts +++ b/lib/ab-testing.test.ts @@ -183,30 +183,4 @@ describe('lib/ab-testing', () => { expect(getVariantFromRequest(buildRequest('being_ab_variant=C'))).toBeNull(); }); }); - - describe('trackConversion', () => { - it('logs in development mode', async () => { - vi.stubEnv('NODE_ENV', 'development'); - const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - const { trackConversion } = await import('./ab-testing'); - - trackConversion('A', 'waitlist_signup', { source: 'homepage' }); - - expect(logSpy).toHaveBeenCalledWith('[A/B] Conversion tracked:', { - variant: 'A', - event: 'waitlist_signup', - metadata: { source: 'homepage' }, - }); - }); - - it('does not log in production mode', async () => { - vi.stubEnv('NODE_ENV', 'production'); - const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - const { trackConversion } = await import('./ab-testing'); - - trackConversion('B', 'download_click'); - - expect(logSpy).not.toHaveBeenCalled(); - }); - }); }); diff --git a/lib/ab-testing.ts b/lib/ab-testing.ts index b9e81c2..5afdfed 100644 --- a/lib/ab-testing.ts +++ b/lib/ab-testing.ts @@ -85,39 +85,3 @@ export function getVariantFromRequest(request: NextRequest): Variant | null { return null; } - -/** - * Conversion event types we track - */ -export type ConversionEvent = - | 'waitlist_signup' - | 'download_click' - | 'cta_click'; - -/** - * Tracks a conversion event - * Conversions are stored in Notion alongside the variant for analysis - * - * @param variant - The assigned variant - * @param event - The conversion event type - * @param metadata - Optional additional data - */ -export function trackConversion( - variant: Variant, - event: ConversionEvent, - metadata?: Record -): void { - // For now, conversions are tracked by including the variant - // in the Notion database entry (see waitlist API) - // - // Future enhancement: Send to Cloudflare Analytics Engine - // await env.ANALYTICS.writeDataPoint({ - // blobs: [variant, event], - // doubles: [1], // count - // indexes: [event], - // }); - - if (process.env.NODE_ENV === 'development') { - console.log('[A/B] Conversion tracked:', { variant, event, metadata }); - } -} diff --git a/lib/gpc.test.ts b/lib/gpc.test.ts index 0bb170b..1258699 100644 --- a/lib/gpc.test.ts +++ b/lib/gpc.test.ts @@ -1,10 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { NextRequest } from 'next/server'; -vi.mock('next/headers', () => ({ - cookies: vi.fn(), -})); - describe('lib/gpc', () => { beforeEach(() => { vi.clearAllMocks(); @@ -64,37 +60,4 @@ describe('lib/gpc', () => { expect(getGpcFromRequest(buildRequest('true'))).toBe(false); }); }); - - describe('getGpcFromCookie', () => { - async function setCookieValue(value: string | undefined) { - const { cookies } = await import('next/headers'); - (cookies as ReturnType).mockResolvedValue({ - get: vi.fn().mockReturnValue(value === undefined ? undefined : { value }), - }); - } - - it("returns true when cookie value is '1'", async () => { - await setCookieValue('1'); - const { getGpcFromCookie } = await import('./gpc'); - expect(await getGpcFromCookie()).toBe(true); - }); - - it('returns false when the cookie is absent', async () => { - await setCookieValue(undefined); - const { getGpcFromCookie } = await import('./gpc'); - expect(await getGpcFromCookie()).toBe(false); - }); - - it("returns false when the cookie value is '0'", async () => { - await setCookieValue('0'); - const { getGpcFromCookie } = await import('./gpc'); - expect(await getGpcFromCookie()).toBe(false); - }); - - it('returns false for an unexpected cookie value', async () => { - await setCookieValue('true'); - const { getGpcFromCookie } = await import('./gpc'); - expect(await getGpcFromCookie()).toBe(false); - }); - }); }); diff --git a/lib/gpc.ts b/lib/gpc.ts index b0bda3a..e5a9f73 100644 --- a/lib/gpc.ts +++ b/lib/gpc.ts @@ -1,17 +1,12 @@ /** * Global Privacy Control (GPC) detection. * - * Reads the `Sec-GPC` request header per the GPC spec - * (https://globalprivacycontrol.org/) and exposes helpers for middleware - * (request-time) and server components (cookie-time). - * - * Only the literal string `'1'` is the affirmative signal; everything else - * — `'0'`, `''`, malformed, absent — is treated as no signal. + * Only the literal string `'1'` is the affirmative signal per the GPC spec + * (https://globalprivacycontrol.org/); everything else is no signal. * * @see INFRA-151 */ -import { cookies } from 'next/headers'; import { NextRequest } from 'next/server'; export const GPC_COOKIE_NAME = 'being_gpc'; @@ -19,27 +14,11 @@ export const GPC_COOKIE_MAX_AGE = 24 * 60 * 60; // 24 hours in seconds export const GPC_REQUEST_HEADER = 'sec-gpc'; export const GPC_RESPONSE_HEADER = 'X-GPC-Honored'; -/** - * Returns true when the header value is the affirmative GPC signal. - * Per spec, only the literal string `'1'` qualifies. - */ export function isGpcSignal(headerValue: string | null | undefined): boolean { return headerValue === '1'; } -/** - * Reads the GPC signal from a NextRequest (for use in middleware). - */ +/** Reads the GPC signal from a NextRequest (for middleware). */ export function getGpcFromRequest(request: NextRequest): boolean { return isGpcSignal(request.headers.get(GPC_REQUEST_HEADER)); } - -/** - * Reads the cached GPC signal from cookies (for use in Server Components). - * The cookie is set/cleared by middleware on every request, so this reflects - * the current request's signal as of the most recent middleware pass. - */ -export async function getGpcFromCookie(): Promise { - const cookieStore = await cookies(); - return cookieStore.get(GPC_COOKIE_NAME)?.value === '1'; -} diff --git a/lib/posthog/config.ts b/lib/posthog/config.ts index 5af639a..709a167 100644 --- a/lib/posthog/config.ts +++ b/lib/posthog/config.ts @@ -11,7 +11,6 @@ */ export const POSTHOG_HOST = 'https://eu.i.posthog.com'; -export const POSTHOG_ASSETS_HOST = 'https://eu-assets.i.posthog.com'; export const POSTHOG_KEY: string | undefined = process.env.NEXT_PUBLIC_POSTHOG_KEY; diff --git a/lib/posthog/events.ts b/lib/posthog/events.ts index b0d1221..7cb3337 100644 --- a/lib/posthog/events.ts +++ b/lib/posthog/events.ts @@ -1,17 +1,10 @@ /** - * PostHog event helpers — type-safe wrappers for the PR #1 event set. + * PostHog event helpers. * - * Design notes: - * - No `posthog-js` import at module scope — these helpers reach the - * PostHog instance via `window.__posthog`, which is only set after - * PosthogProvider has dynamic-imported the library AND init'd it. - * - If PostHog never loaded (GPC kill, missing key, SSR), every helper - * is a silent no-op. No throws, no console noise. - * - PR #1 event set: $pageview (PostHog default — fires automatically - * on init via capture_pageview: true), trackWaitlistSubmitted, - * trackWaitlistFailed. - * - Future helpers (cta_clicked, crisis_resource_clicked, ab_variant_assigned, - * identifyByEmailHash) ship in PR #2+. + * No `posthog-js` import at module scope — helpers reach the instance via + * `window.__posthog`, only set after PosthogProvider dynamic-imports + inits. + * If PostHog never loaded (GPC kill, missing key, SSR), every helper is a + * silent no-op. */ declare global { diff --git a/middleware.test.ts b/middleware.test.ts index 2557e45..fdd9a9b 100644 --- a/middleware.test.ts +++ b/middleware.test.ts @@ -131,19 +131,29 @@ describe('middleware — GPC detection', () => { expect(response.headers.get('X-GPC-Honored')).toBeNull(); }); - it('clears the being_gpc cookie when the header is absent', async () => { + it('clears the being_gpc cookie when the header is absent but the cookie is present', async () => { const { middleware, ab } = await importFresh(); (ab.getVariantFromRequest as ReturnType).mockReturnValue('A'); - const response = middleware(buildRequest()); + // Request carries a stale being_gpc cookie but no Sec-GPC header. + const response = middleware(buildRequest({ cookie: 'being_gpc=1' })); // NextResponse.cookies.delete() emits a Set-Cookie with empty value and - // an expired date (the exact attrs are framework-internal — we only - // assert the deletion signal: an empty-value cookie was emitted). + // an expired date — we assert only the deletion signal (empty value). const cookie = response.cookies.get('being_gpc'); expect(cookie?.value).toBe(''); }); + it('does NOT emit a being_gpc Set-Cookie when neither header nor cookie is present', async () => { + const { middleware, ab } = await importFresh(); + (ab.getVariantFromRequest as ReturnType).mockReturnValue('A'); + + // Common path: no Sec-GPC header, no existing cookie → no Set-Cookie at all. + const response = middleware(buildRequest()); + + expect(response.cookies.get('being_gpc')).toBeUndefined(); + }); + it('coexists with A/B variant assignment (both signals set on same response)', async () => { const { middleware, ab } = await importFresh(); (ab.getVariantFromRequest as ReturnType).mockReturnValue(null); diff --git a/middleware.ts b/middleware.ts index be0443e..77fd410 100644 --- a/middleware.ts +++ b/middleware.ts @@ -60,7 +60,10 @@ export function middleware(request: NextRequest) { httpOnly: false, // Client reads it to render the GpcNotice }); response.headers.set(GPC_RESPONSE_HEADER, '1'); - } else { + } else if (request.cookies.has(GPC_COOKIE_NAME)) { + // Only emit a clearing Set-Cookie when the cookie actually exists — + // avoids a needless Set-Cookie (and the cache-busting it causes) on the + // common no-GPC path where there's nothing to clear. response.cookies.delete(GPC_COOKIE_NAME); }