Hacktoberfest 2026：维护者为十月标记出来的 issue，仍然开放、适合新手。 浏览 Hacktoberfest issue

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

未关闭
  • 哪里不对？
#26 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
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 分钟
30 天内合并 PR
30

环境准备

这个项目没有提供开发容器、Dockerfile 或贡献指南，环境需要你自己搭建：先看它的 README，通用步骤见我们的新手贡献指南。

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

nextlevelbuilder/skillx 的其他 Issue

查看 nextlevelbuilder/skillx 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。