Found during a source audit of 9bf8f4c. Verified by reading the code. A PR is incoming.
Reporting this as a gap rather than a design error: the eviction path in packages/opencode/src/core/refresh-file-lock.ts is fenced and correct. Renewal and release are its unswept twins.
The existing design, for context
Eviction is built around a fencing token, and the source says why:
Fencing-token eviction: a directory-based marker (mkdir O_EXCL) serializes destructive removal to one contender at a time. The marker holds an owner file so ownership survives a stale-marker recovery rename [...]
The underlying constraint is that a filesystem gives atomic claim but no atomic conditional-delete, so read-check-then-delete cannot be made single-winner by the check alone. The acceptable worst case is zero winners; two is ruled out.
Renewal writes across an await without fencing
const owner = await readOwner()
const currentNow = now()
if (released || owner?.ownerId !== ownerId || Number(owner?.expiresAt) <= currentNow) {
return
}
await writeOwner()P1 passes the ownership check with its lease still valid, then stalls. The lease expires. P2 evicts and legitimately acquires. P1 resumes and writeOwner() lands unconditionally, overwriting P2's owner record — two processes now believe they hold the lock.
Release deletes by pathname across an await without fencing
try {
const owner = await readOwner()
if (owner?.ownerId !== ownerId) return
} catch { return }
await rm(lockPath, { recursive: true, force: true }).catch(() => {})Same shape, destructive. P1 confirms itself as owner, stalls past expiry, P2 evicts and acquires, P1's queued rm removes P2's live lock and admits a third holder.
A renewal in flight can also resurrect a released lock
A renewal callback that passes its check before release() runs can land its writeOwner() afterwards, recreating an orphan lock nobody owns and blocking contenders until the TTL expires.
Severity
Lower than it first looks. Every one of these needs the holder to stall past the 120s TTL between the check and the act, which in practice means severe event-loop starvation. Worth fixing because it breaks the invariant the eviction fencing exists to hold, not because it is likely to fire.
Affected
packages/opencode/src/core/refresh-file-lock.ts at 9bf8f4c, lines 94-113 and 265-279.
Found during a source audit of
9bf8f4c. Verified by reading the code. A PR is incoming.Reporting this as a gap rather than a design error: the eviction path in
packages/opencode/src/core/refresh-file-lock.tsis fenced and correct. Renewal and release are its unswept twins.The existing design, for context
Eviction is built around a fencing token, and the source says why:
The underlying constraint is that a filesystem gives atomic claim but no atomic conditional-delete, so read-check-then-delete cannot be made single-winner by the check alone. The acceptable worst case is zero winners; two is ruled out.
Renewal writes across an await without fencing
P1 passes the ownership check with its lease still valid, then stalls. The lease expires. P2 evicts and legitimately acquires. P1 resumes and
writeOwner()lands unconditionally, overwriting P2's owner record — two processes now believe they hold the lock.Release deletes by pathname across an await without fencing
Same shape, destructive. P1 confirms itself as owner, stalls past expiry, P2 evicts and acquires, P1's queued
rmremoves P2's live lock and admits a third holder.A renewal in flight can also resurrect a released lock
A renewal callback that passes its check before
release()runs can land itswriteOwner()afterwards, recreating an orphan lock nobody owns and blocking contenders until the TTL expires.Severity
Lower than it first looks. Every one of these needs the holder to stall past the 120s TTL between the check and the act, which in practice means severe event-loop starvation. Worth fixing because it breaks the invariant the eviction fencing exists to hold, not because it is likely to fire.
Affected
packages/opencode/src/core/refresh-file-lock.tsat9bf8f4c, lines 94-113 and 265-279.