activeloopai / activeloopai/hivemind
skillopt: worker lock is per-machine — concurrent improvements to the same skill from two machines can silently lose an edit
- 主要言語
- TypeScript
- スター
- 1.6k
- フォーク
- 107
- 平均マージ
- 17時間 30分
- マージ済み PR(30日)
- 6
説明
`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.
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
評価
この issue はまだ評価されていません。