activeloopai / activeloopai/hivemind

skillopt: worker lock is per-machine — concurrent improvements to the same skill from two machines can silently lose an edit

Đang mở
#340 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
TypeScript
Star
1.6k
Fork
107
Merge trung bình
17 giờ 30 phút
Pull request đã merge (30 ngày)
6

Mô tả

`tryAcquireWorkerLock(projectKey)` (`src/skillify/state.ts:193`) is a **local lockfile** under the state dir. It correctly serializes SkillOpt workers on one machine — including careful TOCTOU handling for the stale-lock-as-directory case (lines 204-223).

It does **not** serialize across machines, and the skills table is org-wide.

**Scenario.** Skill `X--alice` is at v7. Two engineers on different machines invoke it and both react negatively within the same window:

1. Worker A (machine 1) acquires its local lock, reads current row → v7, proposes edit E1.
2. Worker B (machine 2) acquires *its own* local lock — unaware of A — reads current row → v7, proposes edit E2.
3. Both call `publishImprovedSkill`, both compute `version = 7 + 1`.

Result: two rows at v8 for the same `(project_key, name)`. Readers use `ORDER BY version DESC LIMIT 1` (`src/skillify/skills-table.ts:14-17`), so **one of the two edits is silently discarded** — and which one wins depends on the row ordering for the tie, which isn't specified. Both edits were grounded in genuine confirmed failures; one just disappears.

The append-only design means nothing is destroyed on disk (both rows persist), so this is a **lost update at the read layer**, not data loss. That also makes it cheap to detect after the fact.

**Options:**
1. **Conditional publish (compare-and-set):** re-read the max version immediately before insert and fail/retry if it moved. Doesn't fully close the race without a unique constraint, but shrinks the window a lot.
2. **Unique index on `(project_key, name, version)`** so the second insert fails loudly, then retry the propose against the new current body (the natural fix — the loser's edit gets re-proposed against v8 instead of vanishing).
3. **Deterministic tie-break** on the read side (e.g. `ORDER BY version DESC, updated_at DESC, id ASC`) so at minimum the winner is stable and reproducible — same rationale as the recall query's tie-break (`src/hooks/shared/recall-query.ts:27`).
4. Accept it and document it: concurrent improvements to the same skill are rare, and the next invocation's judge will catch a still-failing skill anyway (the loop is self-healing over time).

Option 3 is cheap and worth doing on its own; option 2 is the real fix.

Note this is only reachable for **org-scope** skills invoked by multiple people — the local lock already covers the common single-user case.

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.