fix: dedupe download metrics hourly by user-or-ip identity

convex/downloads.test.ts

@@ -1,12 +1,43 @@
-import { describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 import { __test } from './downloads'
 
 describe('downloads helpers', () => {
-  it('calculates day start boundaries', () => {
-    const day = 86_400_000
-    expect(__test.getDayStart(0)).toBe(0)
-    expect(__test.getDayStart(day - 1)).toBe(0)
-    expect(__test.getDayStart(day)).toBe(day)
-    expect(__test.getDayStart(day + 1)).toBe(day)
+  afterEach(() => {
+    vi.unstubAllEnvs()
+  })
+
+  it('calculates hour start boundaries', () => {
+    const hour = 3_600_000
+    expect(__test.getHourStart(0)).toBe(0)
+    expect(__test.getHourStart(hour - 1)).toBe(0)
+    expect(__test.getHourStart(hour)).toBe(hour)
+    expect(__test.getHourStart(hour + 1)).toBe(hour)
+  })
+
+  it('prefers user identity when token user exists', () => {
+    const request = new Request('https://example.com', {
+      headers: { 'cf-connecting-ip': '1.2.3.4' },
+    })
+    expect(__test.getDownloadIdentityValue(request, 'users_123')).toBe('user:users_123')
+  })
+
+  it('uses cf-connecting-ip for anonymous identity', () => {
+    const request = new Request('https://example.com', {
+      headers: { 'cf-connecting-ip': '1.2.3.4' },
+    })
+    expect(__test.getDownloadIdentityValue(request, null)).toBe('ip:1.2.3.4')
+  })
+
+  it('falls back to forwarded ip when explicitly enabled', () => {
+    vi.stubEnv('TRUST_FORWARDED_IPS', 'true')
+    const request = new Request('https://example.com', {
+      headers: { 'x-forwarded-for': '10.0.0.1, 10.0.0.2' },
+    })
+    expect(__test.getDownloadIdentityValue(request, null)).toBe('ip:10.0.0.1')
+  })
+
+  it('returns null when user and ip are missing', () => {
+    const request = new Request('https://example.com')
+    expect(__test.getDownloadIdentityValue(request, null)).toBeNull()
   })
 })

convex/downloads.ts

@@ -1,13 +1,14 @@
 import { v } from 'convex/values'
 import { api, internal } from './_generated/api'
 import { httpAction, internalMutation, mutation } from './_generated/server'
+import { getOptionalApiTokenUserId } from './lib/apiTokenAuth'
 import { applyRateLimit, getClientIp } from './lib/httpRateLimit'
 import { buildDeterministicZip } from './lib/skillZip'
 import { hashToken } from './lib/tokens'
 import { insertStatEvent } from './skillStatEvents'
 
-const DAY_MS = 86_400_000
-const DEDUPE_RETENTION_DAYS = 14
+const HOUR_MS = 3_600_000
+const DEDUPE_RETENTION_MS = 7 * 24 * HOUR_MS
 const PRUNE_BATCH_SIZE = 200
 const PRUNE_MAX_BATCHES = 50
 
@@ -87,15 +88,16 @@ export const downloadZip = httpAction(async (ctx, request) => {
   })
   const zipBlob = new Blob([zipArray], { type: 'application/zip' })
 
-  const ip = getClientIp(request) ?? 'unknown'
-  const ipHash = await hashToken(ip)
-  const dayStart = getDayStart(Date.now())
   try {
-    await ctx.runMutation(internal.downloads.recordDownloadInternal, {
-      skillId: skill._id,
-      ipHash,
-      dayStart,
-    })
+    const userId = await getOptionalApiTokenUserId(ctx, request)
+    const identity = getDownloadIdentityValue(request, userId ? String(userId) : null)
+    if (identity) {
+      await ctx.runMutation(internal.downloads.recordDownloadInternal, {
+        skillId: skill._id,
+        identityHash: await hashToken(identity),
+        hourStart: getHourStart(Date.now()),
+      })
+    }
   } catch {
     // Best-effort metric path; do not fail downloads.
   }
@@ -126,22 +128,25 @@ export const increment = mutation({
 export const recordDownloadInternal = internalMutation({
   args: {
     skillId: v.id('skills'),
-    ipHash: v.string(),
-    dayStart: v.number(),
+    identityHash: v.string(),
+    hourStart: v.number(),
   },
   handler: async (ctx, args) => {
     const existing = await ctx.db
       .query('downloadDedupes')
-      .withIndex('by_skill_ip_day', (q) =>
-        q.eq('skillId', args.skillId).eq('ipHash', args.ipHash).eq('dayStart', args.dayStart),
+      .withIndex('by_skill_identity_hour', (q) =>
+        q
+          .eq('skillId', args.skillId)
+          .eq('identityHash', args.identityHash)
+          .eq('hourStart', args.hourStart),
       )
       .unique()
     if (existing) return
 
     await ctx.db.insert('downloadDedupes', {
       skillId: args.skillId,
-      ipHash: args.ipHash,
-      dayStart: args.dayStart,
+      identityHash: args.identityHash,
+      hourStart: args.hourStart,
       createdAt: Date.now(),
     })
 
@@ -155,12 +160,12 @@ export const recordDownloadInternal = internalMutation({
 export const pruneDownloadDedupesInternal = internalMutation({
   args: {},
   handler: async (ctx) => {
-    const cutoff = Date.now() - DEDUPE_RETENTION_DAYS * DAY_MS
+    const cutoff = Date.now() - DEDUPE_RETENTION_MS
 
     for (let batches = 0; batches < PRUNE_MAX_BATCHES; batches += 1) {
       const stale = await ctx.db
         .query('downloadDedupes')
-        .withIndex('by_day', (q) => q.lt('dayStart', cutoff))
+        .withIndex('by_hour', (q) => q.lt('hourStart', cutoff))
         .take(PRUNE_BATCH_SIZE)
 
       if (stale.length === 0) break
@@ -174,12 +179,20 @@ export const pruneDownloadDedupesInternal = internalMutation({
   },
 })
 
-export function getDayStart(timestamp: number) {
-  return Math.floor(timestamp / DAY_MS) * DAY_MS
+export function getHourStart(timestamp: number) {
+  return Math.floor(timestamp / HOUR_MS) * HOUR_MS
+}
+
+export function getDownloadIdentityValue(request: Request, userId: string | null) {
+  if (userId) return `user:${userId}`
+  const ip = getClientIp(request)
+  if (!ip) return null
+  return `ip:${ip}`
 }
 
 export const __test = {
-  getDayStart,
+  getHourStart,
+  getDownloadIdentityValue,
 }
 
 function mergeHeaders(base: HeadersInit, extra: HeadersInit) {

convex/lib/apiTokenAuth.test.ts

@@ -0,0 +1,85 @@
+import { describe, expect, it, vi } from 'vitest'
+import { getOptionalApiTokenUserId } from './apiTokenAuth'
+import { hashToken } from './tokens'
+
+describe('getOptionalApiTokenUserId', () => {
+  it('returns null when auth header is missing', async () => {
+    const ctx = {
+      runQuery: vi.fn(),
+    }
+    const request = new Request('https://example.com')
+
+    const userId = await getOptionalApiTokenUserId(ctx as never, request)
+
+    expect(userId).toBeNull()
+    expect(ctx.runQuery).not.toHaveBeenCalled()
+  })
+
+  it('returns null for unknown token', async () => {
+    const ctx = {
+      runQuery: vi.fn().mockResolvedValue(null),
+    }
+    const request = new Request('https://example.com', {
+      headers: { authorization: 'Bearer token-1' },
+    })
+
+    const userId = await getOptionalApiTokenUserId(ctx as never, request)
+
+    expect(userId).toBeNull()
+    expect(ctx.runQuery).toHaveBeenCalledTimes(1)
+    expect(ctx.runQuery.mock.calls[0]?.[1]).toEqual({
+      tokenHash: await hashToken('token-1'),
+    })
+  })
+
+  it('returns user id when token and user are valid', async () => {
+    const tokenId = 'apiTokens_1'
+    const expectedUserId = 'users_1'
+    const ctx = {
+      runQuery: vi
+        .fn()
+        .mockImplementation(async (_fn, args: { tokenHash?: string; tokenId?: string }) => {
+          if (args.tokenHash) {
+            return { _id: tokenId, revokedAt: undefined }
+          }
+          if (args.tokenId) {
+            return { _id: expectedUserId, deletedAt: undefined }
+          }
+          return null
+        }),
+    }
+    const request = new Request('https://example.com', {
+      headers: { authorization: 'Bearer token-2' },
+    })
+
+    const userId = await getOptionalApiTokenUserId(ctx as never, request)
+
+    expect(userId).toBe(expectedUserId)
+    expect(ctx.runQuery).toHaveBeenCalledTimes(2)
+  })
+
+  it('returns null when user is deleted', async () => {
+    const tokenId = 'apiTokens_2'
+    const ctx = {
+      runQuery: vi
+        .fn()
+        .mockImplementation(async (_fn, args: { tokenHash?: string; tokenId?: string }) => {
+          if (args.tokenHash) {
+            return { _id: tokenId, revokedAt: undefined }
+          }
+          if (args.tokenId) {
+            return { _id: 'users_deleted', deletedAt: Date.now() }
+          }
+          return null
+        }),
+    }
+    const request = new Request('https://example.com', {
+      headers: { authorization: 'Bearer token-3' },
+    })
+
+    const userId = await getOptionalApiTokenUserId(ctx as never, request)
+
+    expect(userId).toBeNull()
+    expect(ctx.runQuery).toHaveBeenCalledTimes(2)
+  })
+})

convex/lib/apiTokenAuth.ts

@@ -27,6 +27,26 @@ export async function requireApiTokenUser(
   return { user, userId: user._id }
 }
 
+export async function getOptionalApiTokenUserId(
+  ctx: ActionCtx,
+  request: Request,
+): Promise<Doc<'users'>['_id'] | null> {
+  const header = request.headers.get('authorization') ?? request.headers.get('Authorization')
+  const token = parseBearerToken(header)
+  if (!token) return null
+
+  const tokenHash = await hashToken(token)
+  const apiToken = await ctx.runQuery(internal.tokens.getByHashInternal, { tokenHash })
+  if (!apiToken || apiToken.revokedAt) return null
+
+  const user = await ctx.runQuery(internal.tokens.getUserForTokenInternal, {
+    tokenId: apiToken._id,
+  })
+  if (!user || user.deletedAt) return null
+
+  return user._id
+}
+
 function parseBearerToken(header: string | null) {
   if (!header) return null
   const trimmed = header.trim()

convex/schema.ts

@@ -492,12 +492,12 @@ const rateLimits = defineTable({
 
 const downloadDedupes = defineTable({
   skillId: v.id('skills'),
-  ipHash: v.string(),
-  dayStart: v.number(),
+  identityHash: v.string(),
+  hourStart: v.number(),
   createdAt: v.number(),
 })
-  .index('by_skill_ip_day', ['skillId', 'ipHash', 'dayStart'])
-  .index('by_day', ['dayStart'])
+  .index('by_skill_identity_hour', ['skillId', 'identityHash', 'hourStart'])
+  .index('by_hour', ['hourStart'])
 
 const githubBackupSyncState = defineTable({
   key: v.string(),

docs/http-api.md

@@ -131,6 +131,7 @@ Notes:
 
 - If neither `version` nor `tag` is provided, the latest version is used.
 - Soft-deleted versions return `410`.
+- Download stats are counted as unique identities per hour (`userId` when API token is valid, otherwise IP).
 
 ## Auth endpoints (Bearer token)