Hacktoberfest 2026：メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

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

オープン
  • 何か問題がありますか？
#26 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
48/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
活発
技術スタック
typescript
領域
api, backend, distributed-systems

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.
主要言語
TypeScript
スター
189
フォーク
40
平均マージ
7時間 38分
マージ済み PR（30日）
30

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

nextlevelbuilder/skillx のほかの issue

nextlevelbuilder/skillx の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。