activeloopai / activeloopai/hivemind

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

Abierto
#340 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
TypeScript
Estrellas
1.6k
Forks
107
Merge medio
17 h 30 min
PR fusionados (30 d)
6

Descripción

`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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.