analytics: make each weekly funnel stage separately measurable (FRE-366) #2

agent-code-review · 2026-10-30T20:48:47-04:00

agent-code-review commented

2026-10-30 20:48:47 -04:00

What this is

FRE-366: make each weekly funnel stage separately measurable. Splits the one
shared download label into stage = 'click' (the doors) vs stage = 'served'
(the DMG file route), records a created Stripe session as the checkout stage
(POST /api/the-nook/checkout), and records the real client address plus the
host each request addressed (stage/host columns through the existing
VISITOR_ANALYTICS_ADDED_COLUMNS bootstrap path). Every analytics entry is
truncated to an address prefix (/24 v4, /56 v6) before storage — the
FRE-369 privacy floor — and a daily retention cron (/api/analytics-retention,
12:00 UTC, Bearer ANALYTICS_RETENTION_SECRET ?? CRON_SECRET, fail-closed)
bounds row lifetime. The weekly job now renders seven separately measured
stages with computed caveats.

Base relationship (read before merging)

This branch is based on the FRE-202 tip 89f56fc, which is not yet on
master. Landing order: FRE-202 first, then this branch; once FRE-202 is
on master this PR narrows to the FRE-366 delta automatically. Merging stays
with the repo owner/CTO — this PR is the landing act, not a merge.

Security gate (Security Engineer, FRE-369 / FRE-370)

src/server/request-client.ts is the client-IP trust boundary. Two findings
from the FRE-369 review (leftmost x-forwarded-for hop is attacker-chosen
text; full IPs with no retention) are fixed in this head. FRE-370 verified
the deploy-side precondition and recorded: this read must not deploy ahead
of the FRE-371 topology-correct fix (rate-limit / abuse paths). The value
stays truncated and out of every security path; see the module doc for the
stated residual.

## What this is FRE-366: make each weekly funnel stage separately measurable. Splits the one shared download label into `stage = 'click'` (the doors) vs `stage = 'served'` (the DMG file route), records a created Stripe session as the checkout stage (`POST /api/the-nook/checkout`), and records the real client address plus the host each request addressed (`stage`/`host` columns through the existing `VISITOR_ANALYTICS_ADDED_COLUMNS` bootstrap path). Every analytics entry is truncated to an address prefix (/24 v4, /56 v6) before storage — the FRE-369 privacy floor — and a daily retention cron (`/api/analytics-retention`, 12:00 UTC, Bearer `ANALYTICS_RETENTION_SECRET ?? CRON_SECRET`, fail-closed) bounds row lifetime. The weekly job now renders seven separately measured stages with computed caveats. ## Base relationship (read before merging) This branch is based on the FRE-202 tip `89f56fc`, which is **not yet on `master`**. Landing order: FRE-202 first, then this branch; once FRE-202 is on master this PR narrows to the FRE-366 delta automatically. Merging stays with the repo owner/CTO — this PR is the landing act, not a merge. ## Security gate (Security Engineer, FRE-369 / FRE-370) `src/server/request-client.ts` is the client-IP trust boundary. Two findings from the FRE-369 review (leftmost `x-forwarded-for` hop is attacker-chosen text; full IPs with no retention) are fixed in this head. FRE-370 verified the deploy-side precondition and recorded: **this read must not deploy ahead of the FRE-371 topology-correct fix** (rate-limit / abuse paths). The value stays truncated and out of every security path; see the module doc for the stated residual.

agent-code-review added 9 commits 2026-10-30 20:48:47 -04:00

site: remove the unproven remote claim from the Nook landing page c1c498e37f

FRE-323. The one-click remote install is app-UI-only and needs hand-edited ~/.ssh/config plus a one-time ssh-copy-id (FRE-320); FRE-315 and FRE-322 hold the claim unverified and demo-gated.

- drop the Remote tile and its mock from the feature collage
- drop the "Any SSH box, one click away" breakdown, its demo and bullets
- drop "Works across every hooked agent, local or remote"
- drop the "One-click SSH setup" chip
- move the copy to marketing/drafts/remote-ssh/ and mark remote held in marketing/positioning.md

No replacement claim was added. Deploy waits on a push credential for the GitHub repo Vercel builds.

marketing: reconcile positioning.md with the CTO verdicts and the price decision 210a7e6e75

The source-of-truth file still sold claims the board has since ruled on:

- 'thirteen agents' as a public coverage count — FRE-322 §1 counts 18 specs,
  9 of which can raise an approval card; no count is publishable (FRE-203 plan-b).
- 'Price rises to 5 at 1.0' — withdrawn by the Chief of Staff on 2026-10-29
  04:01; price stays $10 and is not a launch asset.
- 'macOS 14+' — the app cannot start below 14.2 (881190d, site 134f41e).
- Approved-claims table carried no launch sentence at all, and no row for the
  remote-SSH hold or the banned absolute privacy sentences.
- Pointed at the-nook/docs/marketing-strategy.md, which no longer exists.

Now: the Plan B claim set, a per-claim status table with verdict pointers,
FRE-354's corrections to the hook/bridge sentence and the Sentry disclosure,
the iPhone TestFlight state, and an explicit excluded/banned list.

/agents: take the remote-session sentence off the page c718370674

FRE-323 removed the remote claim from the landing page and missed the second
page that carries it. /agents said 'Agents can run on this Mac or on remote
machines over SSH — remote sessions report into the same grid', which is the
same unverified claim (FRE-315 §1, FRE-320), in front of the same launch
traffic. Removed; the paragraph now ends at 'the agent keeps its own prompting'.

Not deployed: same read-only credential as c1c498e.

agents: drop the count and the coverage claim (FRE-359 verdict B) d2c2e6a4ee

FRE-359's verdict is B: the page's thirteen is the registry's first thirteen
of eighteen rows, so the count, "every coding agent The Nook supports" and
"Supported agents" all overstate or understate the app. The permitted
sentences are the CTO's, verbatim.

- agents.tsx: head description, ogTitle and H1 paragraph replaced with the
  verdict's sentences; title/H1 "Supported agents" -> "Agent coverage".
- agents.test.ts: the stale NOOK-336 guard asserted "all thirteen" and
  the 13-name roster, so it passed in lockstep with the page. It now asserts
  no count of any form, no completeness phrase, and that the rows are a
  subset of the 18-name registry list (copied 2026-10-29, @ f4258034) in
  registry order. The table still lists 13 rows; regenerating it from the
  registry is its own ticket, and the guard no longer pretends otherwise.
- FeatureBreakdowns.tsx / IslandExtras.tsx: the landing "13 agents:" bullet
  and the "13 agents" marquee chip lose the count; the chip becomes
  "Agents wired in".

bun test src/routes/nook/agents.test.ts: 4 pass. Server security and appcast
suites fail in this checkout on missing env (Environment validation failed /
mainDBConnection), unrelated to these files.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

marketing: the remote correction is committed, not deployed 578ee58f9c

copy.md said the sentences were "removed from the live site" and
rationale.md said "the landing page now says only what the product can back
today". Neither is true: nook.freno.me still serves all of them (HTTP 200,
re-fetched 2026-10-29T14:20Z) because the push is blocked on a write
credential (card 82d1e1b7). Both files now say committed-not-deployed, and
positioning.md's excluded-count row records FRE-359 verdict B and where the
count was removed.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

site: guard the removed remote claim and agent count against return (FRE-323, FRE-359) d44133fcb3

metrics: stand up the weekly funnel job (FRE-202) 89f56fc78a

`marketing/metrics/weekly.md` has been a placeholder since the marketing tree
landed — it promised a funnel and the one thing that moved, and nothing
produced it. This adds the producer: one command reads the live databases and
rewrites the file.

`scripts/weekly-metrics.ts` writes `marketing/metrics/weekly.md` from
`VisitorAnalytics` (visits, downloads, campaigns) and the-nook's `trials`,
`licenses`, `activations` (the paid funnel). It needs four env vars —
TURSO_DB_URL/TOKEN and NOOK_DB_URL/TOKEN — and imports no server env, so the
job runs from a checkout that has no Stripe or Apple secrets.
`src/server/weekly-metrics.ts` holds the queries and the renderer, taking both
connections as arguments; the tests run them against an in-memory SQLite shaped
like production, the same way the backfill suite does.

The campaign breakdown groups exactly as `getAnalyticsSummary.topCampaigns`
does (top 10, untagged excluded), so the weekly read and /analytics cannot
drift into different label sets.

The first real report says more about the instrumentation than about demand,
and the renderer computes that rather than asserting it:

- every addressed row the table has ever held (12,035 of 12,048) is loopback,
  so "unique visitors" is one host and per-channel reach cannot be read;
- the landing door and the file route both write `/api/downloads/thenook`, so
  "download click" and "DMG served" are one label — one served click writes two
  rows, a failed click one, a Sparkle update one;
- `POST /api/the-nook/checkout` records nothing, so checkout start is only a
  page-view proxy;
- `VisitorAnalytics` has no host dimension, so the Nook's `/` is freno.me's `/`.

Those three gaps are reported beside the numbers, not papered over. Week 0
(2026-10-22 → 2026-10-29): visit 61, download 156, trial 2, checkout 0,
purchase 0, activation 0 — and the one thing that moved is the download stage,
0 → 156, because the label was introduced that week.

Tests: `bun test src/server/weekly-metrics.test.ts` — 12 pass, 0 fail. The full
suite's 15 failures are pre-existing: this checkout's .env lacks
NOOK_LICENSE_PRIVATE_KEY, NOOK_STRIPE_SK/WEBHOOK/PRICE and LINEAGE_JWT_SECRET,
and every failure is that env validation.

Scripts/ is gitignored, so `scripts/weekly-metrics.ts` was added with `-f`, the
way the existing tracked scripts are.

FRE-202

analytics: make each weekly funnel stage separately measurable (FRE-366) 39d874a7e2

Gap 1 — click and served share one label: trackDownload now takes a
stage ("click" | "served") carried on a new VisitorAnalytics.stage
column; the Nook door and the tRPC proxy record clicks, the DMG file
route records served. summariseDownloads() and the /analytics downloads
block are untouched — the stage is a separate column, so the path
grouping cannot change.

Gap 2 — checkout start records nothing: the checkout route records a
created Stripe session through the shared trackRequest writer (same
referrer, campaign, country dimensions as every door) under its own
path, and STAGES.checkout reads it instead of the /checkout page proxy.

Gap 3 — loopback addresses, no host dimension: every writer now
resolves the client address through resolveClientIp, which trusts
x-forwarded-for only when the direct peer is the loopback production
proxy (trust boundary stated in request-client.ts), and records the
host on each row through the same site-context extractor routing uses.
stage and host reach the table through VISITOR_ANALYTICS_ADDED_COLUMNS;
the weekly job applies the same bootstrap before reading so it renders
seven stages against a database no deploy has migrated yet.

The weekly read splits downloadClick/dmgServed, scopes visits to
nook.freno.me, and computes its caveats from the window: pre-instrumentation
rows are stated and excluded, and the loopback warning now fires only
when rows recorded since the instrumentation still carry it.

analytics: fix the two FRE-369 findings in the funnel instrumentation 1d9287a199

The security review of 39d874a (FRE-369) found the address fix recorded
the wrong thing twice. Both are fixed here; the branch is the same one
under review, this commit does not rewrite it.

Q1 — the first x-forwarded-for hop is attacker-chosen text. In
client → Cloudflare → same-host proxy → app, both proxies *append*, so
the header arrives as `<client spoof…>, <true client>, <CF edge>` and
`split(",")[0]` took the sender's own invention — the loopback-peer gate
was sound, the hop selection defeated it. Preference is now
`cf-connecting-ip` (Cloudflare overwrites it, so a client value never
survives the edge), then x-forwarded-for counted from the right with one
trusted hop discounted, then the direct peer. Every candidate must parse
as an address. h3's `getRequestIP(…, { xForwardedFor: true })` has the
same leftmost flaw and is documented as not a fix.

Q4 — the table held full client addresses indefinitely. Every analytics
entry now passes through `truncateIpAddress` in `enrichAnalyticsEntry`
(IPv4 /24, IPv6 /56, unparseable dropped), so no writer can persist a
full address by accident. Retention got the schedule it never had:
`/api/analytics/retention` on a daily Vercel cron, Bearer-authenticated
and fail-closed, with the window parameterised (the old
`cleanupOldAnalytics` spliced the number into the SQL). Default 365 days
rather than the review's 90 so enabling the cron is not itself a
destructive event — the tighter window is an operational decision, named
on the task.

Also from the review: the module doc now forbids feeding this helper into
any security decision; `isLoopbackAddress` parses instead of
string-matching (`::ffff:127.9.9.9`, `0:0:0:0:0:0:0:1`); and `host` is
normalised (lowercased, port stripped) so `nook.freno.me:443` cannot
silently miss the `host = ?` scope. The weekly loopback caveat matches
the truncated shapes writers produce now, and says plainly that "unique
visitors" counts prefixes.

Mike merged commit 6fd0f84806 into master

2026-11-04 10:58:19 -05:00

Mike referenced this issue from a commit

2026-11-04 10:58:19 -05:00

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

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Mike/freno-dev#2

Allow edits from maintainers