Fix activation analytics gaps: snapshot cron, event gating, rate limit - #142
Merged
Conversation
- Add daily /api/cron/refresh-open-stats (CRON_SECRET-guarded) so /open and the landing ticker snapshots actually refresh - Gate first_sync_confirmed on earliest daily_usage row <24h old with a per-user $insert_id, instead of firing on every status poll - Rate-limit /api/analytics/activation (20/min per user or client IP) - Capture activation events server-side only (drop browser double-capture) - Cleanups: shared getCookieValue, dead dedup loop, after() fallback logs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds rate limiting to the activation analytics endpoint, introduces a new cron route to refresh open-stats snapshots, gates the ChangesActivation analytics, cron snapshot, and usage-status changes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ActivationRoute as "/api/analytics/activation"
participant RateLimiter as "rateLimit()"
participant Analytics as "captureServerActivationEvent"
Client->>ActivationRoute: POST activation event
ActivationRoute->>ActivationRoute: derive rateLimitSubject (userId or IP)
ActivationRoute->>RateLimiter: rateLimit("activation-analytics", subject, limits)
RateLimiter-->>ActivationRoute: limited response or null
alt limited
ActivationRoute-->>Client: 429 response
else not limited
ActivationRoute->>Analytics: capture activation event
ActivationRoute-->>Client: success response
end
sequenceDiagram
participant Vercel as "Vercel Cron"
participant Route as "/api/cron/refresh-open-stats"
participant Snapshot as "refreshOpenStatsSnapshot()"
Vercel->>Route: GET with Authorization Bearer CRON_SECRET
Route->>Route: validate Bearer token
alt unauthorized
Route-->>Vercel: 401 Unauthorized
else authorized
Route->>Snapshot: refreshOpenStatsSnapshot()
alt success
Snapshot-->>Route: snapshotDate, totalSpend, trackedUsers
Route-->>Vercel: 200 ok:true
else failure
Snapshot-->>Route: throws error
Route-->>Vercel: 500 error message
end
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/api/cron/refresh-open-stats/route.ts`:
- Around line 14-22: The refresh-open-stats cron endpoint currently reports
success even when the snapshot persistence path fails. Update the route handler
around refreshOpenStatsSnapshot so it does not return a 200 response unless the
durable write actually succeeds; either propagate the write error or have
refreshOpenStatsSnapshot expose the write outcome and branch on it before
building the NextResponse.json success payload. Ensure the handler reflects
failures clearly instead of returning live stats from a failed snapshot write.
In `@apps/web/lib/analytics/activation.ts`:
- Around line 99-107: Guard the cookie parsing in getCookieValue so malformed
values do not throw; this helper is used by the activation and auth callback
routes with raw request cookies, and a bad value can currently surface as a 500.
Update getCookieValue to catch decodeURIComponent failures and return null
instead of propagating the exception, while keeping the existing name matching
and return behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab453d0f-d796-4fff-a962-40e4f8c25db2
📒 Files selected for processing (16)
apps/web/__tests__/api/activation-analytics.test.tsapps/web/__tests__/api/cron-refresh-open-stats.test.tsapps/web/__tests__/api/usage-status.test.tsapps/web/app/(auth)/callback/route.tsapps/web/app/api/analytics/activation/route.tsapps/web/app/api/app/right-sidebar/route.tsapps/web/app/api/cron/refresh-open-stats/route.tsapps/web/app/api/usage/status/route.tsapps/web/components/providers/PublicAnalytics.tsxapps/web/lib/analytics/activation.tsapps/web/lib/analytics/client.tsapps/web/lib/utils/after.tsdocs/CHANGELOG.mddocs/DECISIONS.mddocs/ROADMAP.mdvercel.json
Comment on lines
+14
to
+22
| try { | ||
| const stats = await refreshOpenStatsSnapshot(); | ||
|
|
||
| return NextResponse.json({ | ||
| ok: true, | ||
| snapshotDate: stats.snapshotDate, | ||
| totalSpend: stats.totalSpend, | ||
| trackedUsers: stats.trackedUsers, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route.ts =="
sed -n '1,120p' apps/web/app/api/cron/refresh-open-stats/route.ts
echo
echo "== open-stats.ts relevant section =="
sed -n '380,500p' apps/web/lib/open-stats.tsRepository: ohong/straude
Length of output: 3630
Persistence failures are reported as success. refreshOpenStatsSnapshot swallows snapshot-write errors and still returns live stats, so this endpoint responds 200 even when the durable write fails. That hides a broken persistence path from the cron job and any status-code-based alerting; surface the write outcome or rethrow on failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/api/cron/refresh-open-stats/route.ts` around lines 14 - 22, The
refresh-open-stats cron endpoint currently reports success even when the
snapshot persistence path fails. Update the route handler around
refreshOpenStatsSnapshot so it does not return a 200 response unless the durable
write actually succeeds; either propagate the write error or have
refreshOpenStatsSnapshot expose the write outcome and branch on it before
building the NextResponse.json success payload. Ensure the handler reflects
failures clearly instead of returning live stats from a failed snapshot write.
Comment on lines
+99
to
+107
| export function getCookieValue(cookieHeader: string | null, name: string): string | null { | ||
| if (!cookieHeader) return null; | ||
| const target = `${name}=`; | ||
| const entry = cookieHeader | ||
| .split(";") | ||
| .map((part) => part.trim()) | ||
| .find((part) => part.startsWith(target)); | ||
| return entry ? decodeURIComponent(entry.slice(target.length)) : null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and its usages.
git ls-files 'apps/web/lib/analytics/activation.ts' 'apps/web/**/*.{ts,tsx}' | sed 's#^`#FILE` #'
echo
echo '--- activation.ts (outline) ---'
ast-grep outline apps/web/lib/analytics/activation.ts --view expanded || true
echo
echo '--- getCookieValue definition ---'
rg -n -A8 -B4 'export function getCookieValue|decodeURIComponent\(' apps/web/lib/analytics/activation.ts
echo
echo '--- usages of getCookieValue ---'
rg -n -A4 -B4 'getCookieValue\(' apps/web
echo
echo '--- cookie header handling nearby callers ---'
rg -n -A8 -B6 'Cookie|cookie' apps/web/lib apps/web/app apps/web/routes apps/web/components || trueRepository: ohong/straude
Length of output: 41119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file in context and identify all call sites with line numbers.
wc -l apps/web/lib/analytics/activation.ts
cat -n apps/web/lib/analytics/activation.ts | sed -n '1,180p'
echo
echo '--- locate callback/activation routes ---'
rg -n -A6 -B6 'getCookieValue|activation_anon_id|callback' apps/webRepository: ohong/straude
Length of output: 23023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the auth callback route and the analytics route for error handling around getCookieValue.
cat -n apps/web/app/(auth)/callback/route.ts | sed -n '1,140p'
echo
cat -n apps/web/app/api/analytics/activation/route.ts | sed -n '1,150p'
echo
# JS semantics probe: malformed percent-encoding causes decodeURIComponent to throw.
node - <<'JS'
for (const value of ["%", "%E0%A4%A", "abc%zz", "straude_activation_id=%"]) {
try {
console.log(value, "=>", decodeURIComponent(value));
} catch (err) {
console.log(value, "=> THROW", err.name);
}
}
JSRepository: ohong/straude
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- callback route ---'
cat -n 'apps/web/app/(auth)/callback/route.ts' | sed -n '1,120p'
echo
echo '--- activation route ---'
cat -n 'apps/web/app/api/analytics/activation/route.ts' | sed -n '1,140p'
echo
echo '--- decodeURIComponent probe ---'
node - <<'JS'
for (const value of ["%", "%E0%A4%A", "abc%zz", "straude_activation_id=%"]) {
try {
console.log(value, "=>", decodeURIComponent(value));
} catch (err) {
console.log(value, "=> THROW", err.name);
}
}
JSRepository: ohong/straude
Length of output: 8173
Guard decodeURIComponent here
decodeURIComponent throws on malformed cookie values, and both the activation and auth callback routes pass raw request cookies into this helper. A crafted cookie like straude_activation_id=% can turn those requests into 500s. Return null on decode failure instead of throwing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/analytics/activation.ts` around lines 99 - 107, Guard the cookie
parsing in getCookieValue so malformed values do not throw; this helper is used
by the activation and auth callback routes with raw request cookies, and a bad
value can currently surface as a 500. Update getCookieValue to catch
decodeURIComponent failures and return null instead of propagating the
exception, while keeping the existing name matching and return behavior
unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the functional gaps found in the post-merge review of the activation workstreams (#133–#139):
/openand the landing ticker were switched to snapshot-only reads, but nothing calledrefreshOpenStatsSnapshot()— the snapshot would go stale forever. Adds a dailyCRON_SECRET-guarded/api/cron/refresh-open-statsroute +vercel.jsonentry.first_sync_confirmedover-firing (metric corruption): the event fired on every/api/usage/statuspoll for any user with usage, keyed to the latest usage row. Now fires only while the user's earliestdaily_usagerow is <24h old (a genuine first sync — a count gate was rejected since first pushes create up to 30 rows), with a per-user$insert_idso PostHog dedups repeat polls./api/analytics/activationhardening: rate-limited via the durable Supabase limiter (20/min per user, per client IP for anonymous), and the browser posthog-js double-capture removed — the server path is now the single canonical source for the 10 activation events (covers pre-consent users and owns anon→user identity stitching).getCookieValue, dead dedup loop in right-sidebar route,after()fallback now logs errors.Validation
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes