Merge pull request 'analytics: make each weekly funnel stage separately measurable (FRE-366)' (#2) from agent-code-review/freno-dev:freno-dev/FRE-366-funnel-stages into master

Reviewed-on: Mike/freno-dev#2

src/routes/api/analytics-retention.ts

@@ -0,0 +1,35 @@
+import type { APIEvent } from "@solidjs/start/server";
+import { env } from "~/env/server";
+import { ConnectionFactory } from "~/server/db-connections";
+import { runRetentionCron } from "~/server/analytics-retention";
+
+/**
+ * GET /api/analytics-retention — daily Vercel cron.
+ *
+ * Deletes `VisitorAnalytics` rows older than the retention window, so visitor
+ * data has a bound instead of accumulating forever (FRE-369, finding Q4 — the
+ * table's only cleaner used to be a manual admin call, and since FRE-366 the
+ * rows carry a truncated but still real client address).
+ *
+ * Window: `ANALYTICS_RETENTION_DAYS`, defaulting to 365 when unset or unusable
+ * (see `~/server/analytics-retention` for why the default is conservative
+ * rather than the reviewer's 90).
+ *
+ * Auth: `Authorization: Bearer <secret>` where the secret is
+ * `ANALYTICS_RETENTION_SECRET` or Vercel's `CRON_SECRET`. An unconfigured secret
+ * denies every request — an unauthenticated trigger of a bulk delete is the one
+ * outcome this endpoint must never have.
+ */
+export async function GET(event: APIEvent) {
+  const result = await runRetentionCron({
+    authorization: event.request.headers.get("authorization"),
+    secret: env.ANALYTICS_RETENTION_SECRET ?? env.CRON_SECRET,
+    configuredDays: env.ANALYTICS_RETENTION_DAYS,
+    conn: ConnectionFactory()
+  });
+
+  return new Response(JSON.stringify(result.body), {
+    status: result.status,
+    headers: { "Content-Type": "application/json" }
+  });
+}

src/routes/api/downloads/[filename].ts

@@ -95,7 +95,7 @@ export async function GET(event: APIEvent) {
 
     console.log(`✓ Serving ${filename} (${body.length} bytes)`);
 
-    await trackDownload(event, assetKeys[asset]!);
+    await trackDownload(event, assetKeys[asset]!, "served");
 
     // SAFETY: `transformToByteArray` yields `Uint8Array<ArrayBufferLike>`, which the
     // DOM lib's `BufferSource` (ArrayBuffer-backed) does not accept, though undici

src/routes/api/the-nook/checkout.ts

@@ -3,6 +3,7 @@ import { env } from "~/env/server";
 import { TURNSTILE_CONFIG } from "~/config";
 import { verifyTurnstileToken } from "~/server/fetch-utils";
 import { json, error } from "./_lib";
+import { trackRequest } from "~/server/track-download";
 
 /**
  * POST /api/the-nook/checkout
@@ -12,6 +13,14 @@ import { json, error } from "./_lib";
  * session for the one-time $10 The Nook license. Returns the hosted
  * checkout URL for the client to redirect to.
  *
+ * Analytics: a created session is the funnel's "checkout started" stage
+ * (FRE-366). Until then the only trace was the client-side /checkout page
+ * view — a page, not a started checkout. Recorded through the shared
+ * `trackRequest` writer (referrer, campaign tags, country, real client
+ * address, host) under this route's own path, best-effort: a failed
+ * analytics write never blocks a checkout, and an attempt Turnstile or
+ * Stripe rejects is correctly not a started checkout.
+ *
  * Uses raw `fetch` (Node 24 / Vercel Node functions have global fetch) — no
  * `stripe` npm dependency added.
  */
@@ -67,5 +76,14 @@ export async function POST(event: APIEvent) {
   if (!data.url) {
     return error("Stripe returned no checkout URL", 502);
   }
+
+  // The session exists: that is "checkout started". Awaited so the row is
+  // flushed before the response — a buffered write is not guaranteed to
+  // survive the response on serverless.
+  await trackRequest(event, {
+    path: "/api/the-nook/checkout",
+    method: "POST"
+  });
+
   return json({ checkoutUrl: data.url });
 }

src/routes/api/the-nook/download.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it, mock, beforeEach } from "bun:test";
+import { truncateIpAddress } from "~/server/request-client";
 
 // The route's contracts:
 //   1. Every click is recorded through the shared analytics buffer with the
@@ -55,9 +56,13 @@ mock.module("~/server/analytics", () => ({
   flushAnalytics: async () => {
     flushCalls += 1;
   },
-  // Mirrors the real enrichment contract: deviceType is derived from the UA.
+  // Mirrors the real enrichment contract: deviceType is derived from the UA,
+  // and the address is cut to its prefix by the FRE-369 privacy floor. The
+  // real `enrichAnalyticsEntry` does both, so the mirror has to apply the same
+  // truncation — otherwise this file would prove the door stores a full address.
   enrichAnalyticsEntry: (entry: Entry) => ({
     ...entry,
+    ipAddress: truncateIpAddress(entry.ipAddress as string | null),
     deviceType: String(entry.userAgent ?? "").includes("Macintosh")
       ? "desktop"
       : null
@@ -113,6 +118,10 @@ describe("GET /api/the-nook/download", () => {
     expect(logged[0]).toMatchObject({
       path: "/api/downloads/thenook",
       method: "GET",
+      // The door is the click half of the split; the file route writes
+      // "served". And the row names the site it came from (FRE-366).
+      stage: "click",
+      host: "nook.freno.me",
       referrer: "https://nook.freno.me/"
     });
     // Flushed within the request, not left to the batch timer.
@@ -137,9 +146,16 @@ describe("GET /api/the-nook/download", () => {
     expect(logged[2]!.country).toBe("JP");
   });
 
-  it("falls back to the forwarded address when no request IP is available", async () => {
-    await GET(request({ "x-forwarded-for": "198.51.100.9, 10.0.0.1" }));
-    expect(logged[0]!.ipAddress).toBe("198.51.100.9");
+  it("records the forwarded visitor below the trusted edge, truncated", async () => {
+    // Client → Cloudflare → origin proxy: the sender's spoof is first, the real
+    // client second, the edge last. And the stored value is the /24, never the
+    // address itself (FRE-369).
+    await GET(
+      request({
+        "x-forwarded-for": "198.51.100.9, 203.0.113.7, 198.51.100.1"
+      })
+    );
+    expect(logged[0]!.ipAddress).toBe("203.0.113.0");
   });
 
   it("still logs the click when the appcast cannot be read", async () => {

src/routes/api/the-nook/download.ts

@@ -13,8 +13,10 @@ import { resolveLatestDownload } from "./_download";
  * Why a redirect and not a direct S3 link: the appcast route and the DMG
  * route never logged anything, so step 2 of the funnel (download click →
  * DMG served) was invisible. Every click now lands one row in
- * VisitorAnalytics under `/api/downloads/thenook`, alongside the clicks and
- * served files of every other product — see `~/server/track-download`.
+ * VisitorAnalytics under `/api/downloads/thenook` with `stage = "click"`,
+ * distinct from the `stage = "served"` row the file route writes when the
+ * DMG is actually streamed — the split FRE-366 added, alongside the clicks
+ * and served files of every other product — see `~/server/track-download`.
  *
  * Referrer, campaign tags, edge country (with `?c=` override) and user agent
  * are read by `trackDownload`, which is shared with the other download doors.
@@ -28,7 +30,7 @@ import { resolveLatestDownload } from "./_download";
 export async function GET(event: APIEvent): Promise<Response> {
   // Recorded before resolving: a click that dies at resolution is exactly the
   // failure the funnel needs to show.
-  await trackDownload(event, "thenook");
+  await trackDownload(event, "thenook", "click");
 
   let target: string | null = null;
   try {