Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

fix(leaderboard): bulk recompute has no concurrency guard, overlapping all=true runs race on the KV checkpoint

Aperta
  • Qualcosa non va?
#26 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
48/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
api, backend, distributed-systems

Direzione di ricerca

Read apps/web/app/routes/api.admin.recompute.ts and apps/web/app/lib/leaderboard/recompute-all-skills.ts, starting at the cited action, state load, and state write paths. Use the overlapping curl reproduction to verify that a second active run is rejected or coordinated atomically, and that the KV checkpoint remains consistent.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Summary

POST /api/admin/recompute?all=true has no concurrency guard, so two simultaneous runs share one KV checkpoint. Both requests read and write the same state, offset can be advanced or reset by whichever write lands last, and every skill in the overlap window is scored twice or skipped. Both callers still get HTTP 200.

Current behavior

  • apps/web/app/routes/api.admin.recompute.ts:51 — action() verifies the admin secret and starts batching immediately. There is no in-flight check and no 409 response path anywhere in the file.
  • apps/web/app/lib/leaderboard/recompute-all-skills.ts:45-46 — KV state is read only when resume is true, and only to choose a starting offset: 
    savedState = await loadState(kv);
offset = savedState?.status === "running" ? savedState.offset : 0;
  • apps/web/app/lib/leaderboard/recompute-all-skills.ts:191-194 — every batch then overwrites or clears that same state, with no compare-and-set: 
    if (isComplete) { await clearState(kv); } else { await saveState(kv, state); }
  • apps/web/app/routes/api.admin.recompute.ts:21 caps one request at MAX_BATCHES_PER_REQUEST = 50, so a single call can run for many seconds — long enough for a second call to overlap. batchCount is per-request only, so it never coordinates across requests.

Expected behavior

A second run is rejected while a run is active — for example 409 Conflict when status === "running" and lastBatchAt is recent — or the runs coordinate through an atomic lock so the checkpoint advances monotonically.

Reproduction

  1. Send two overlapping requests with the admin secret in the header: 
    ADMIN_SECRET=... curl -sS -X POST "https://skillx.sh/api/admin/recompute?all=true&batch=100" &
ADMIN_SECRET=... curl -sS -X POST "https://skillx.sh/api/admin/recompute?all=true&batch=100" &
wait
  2. Watch the returned nextOffset and state from each response, then GET /api/admin/recompute to read the surviving checkpoint.
  3. Interleaved responses show both runs advancing the same checkpoint.

Conditions: requires ADMIN_SECRET plus the D1 and KV bindings; admin-only endpoint. The code paths above are read directly from main at 9d0f9e9 — no live concurrent run against production was performed for this report.

Flow

flowchart LR
  A[POST all=true] --> B[verifyAdmin]
  B --> C[recomputeBatch: load KV offset]
  D[POST all=true] --> E[verifyAdmin]
  E --> F[recomputeBatch: load KV offset]
  C --> G{{both write same KV checkpoint}}
  F --> G
  G --> H[offset / totalProcessed corrupted]

Evidence

  • Affected code: apps/web/app/routes/api.admin.recompute.ts:51, apps/web/app/routes/api.admin.recompute.ts:21, apps/web/app/lib/leaderboard/recompute-all-skills.ts:45, apps/web/app/lib/leaderboard/recompute-all-skills.ts:191
  • Introduced by: 88a5bff ("feat: add bulk recompute admin endpoint for leaderboard scores"), shipped on main via 9d0f9e9 (PR #9)
  • Related: plans/reports/code-reviewer-260306-1531-admin-recompute-endpoint.md, section "4. No concurrency guard"

Open questions

  • Preferred mechanism: a KV read-check-write optimistic lock, a Durable Object, or simply rejecting when lastBatchAt is recent (under 5 minutes)?
  • Is this reachable in practice? scripts/run-recompute.sh sleeps DELAY seconds between batches but holds no lock, and it is currently untracked — see the runbook tooling issue.

Notes

  • The other findings from the same review are already fixed on main: the date/millisecond bug, the N+1 aggregation, the all=true batch guard, the secret-in-query-param auth, and the redundant ternary.
  • A KV read-check-write is not atomic on its own, so if that route is chosen the check must tolerate the small race window (accepting a rare double-reject) rather than assume a real lock.
Lingua principale
TypeScript
Stelle
189
Fork
40
Merge medio
6h 52m
PR unite (30g)
42

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di nextlevelbuilder/skillx

Tutte le issue di nextlevelbuilder/skillx

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.