fix(fleet): guard `mosaic fleet restart` against tight-loop re-entry race

Conversation 3 Commits 3 Files Changed 2

+691 -1

3 Commits

Author SHA1 Message Date

Author	SHA1	Message	Date
jarvisandClaude Opus 4.8	786762587d	fix(fleet): serialize restart-lock transitions to close concurrent-breaker race (review #680 ) ci/woodpecker/pr/ci Pipeline was successful Details ci/woodpecker/push/ci Pipeline was successful Details Stale/max-wait takeover was not safe against concurrent breakers: two breakers could both judge the lock stale and both proceed, re-introducing the tight-loop. POSIX/Node has no content- or inode-conditional unlink or rename, so "judge stale, then replace" can never be atomic with pure path ops. Serialize ALL lock transitions (acquire, release, takeover) under one short-lived registry mutex held only across a few fs ops, never across the restart itself. This makes check-then-mutate atomic, so exactly one breaker can take over a stale lock while the others wait and re-evaluate. The mutex itself uses mtime-based staleness (open('wx') creates an empty inode before the token is written; a content check would reap a lock that is still being acquired). The mutex populates-or-cleans-up on write failure so a half-created mutex never leaks. Regression coverage at two widths: a 2-breaker barrier test (exactly one takes over, the other waits) and the existing 3-breaker test (maxActive===1, distinct tokens, final lock released). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>	2026-06-24 20:26:39 -05:00
jarvisandClaude Opus 4.8	43ad813e0d	fix(fleet): make restart-lock release/break ownership-safe (review #680 ) ci/woodpecker/push/ci Pipeline was successful Details ci/woodpecker/pr/ci Pipeline was canceled Details Addresses the reviewer's blocker (comment 15915): release() unconditionally unlinked restart.lock, so after a stale/max-wait break an OLD owner could delete a NEWER owner's lock, letting a third restart interleave and defeating the guard. - Each acquire writes a unique owner token (randomUUID) into the lock file. - release() only unlinks while that token is still on disk; once another caller has broken and re-owned the lock, the timed-out original owner's release() is a no-op and leaves the new owner's lock intact. - Breaking a stale/timed-out lock now takes ownership atomically via write-temp + rename (atomic replace) instead of a blind unlink-then-recreate; a breaker that loses a concurrent takeover reads back a foreign token and keeps waiting rather than assuming ownership. Regression test (does not let a timed-out owner drop a lock another restart broke and re-owned) reproduces the three-restart interleave: R1 hangs (stale), R2 breaks + re-owns, R1.release() must NOT drop R2's lock. Fails on the old blind-unlink path (ENOENT), passes now. Also adds explicit single-agent restart-path guard coverage (review should-fix). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>	2026-06-24 19:18:38 -05:00
jarvisandClaude Opus 4.8	9c2e4f0b2d	fix(fleet): guard `mosaic fleet restart` against tight-loop re-entry race `mosaic fleet restart` runs as a fresh process each invocation, issuing `systemctl --user restart` for the tmux holder and then each agent. The agent sessions live inside the holder's tmux, so restarting the holder tears them down. With no mutual exclusion, a second restart entering while the first is still mid-teardown (upgrade `--relaunch`, a watchdog, or a hurried operator) interleaves: agents relaunch against a half-torn-down holder, fail, and tight-loop. Add a cross-process teardown-settle guard: a lock file under `<mosaicHome>/fleet/run/restart.lock` acquired with O_CREAT\|O_EXCL. A re-entrant restart waits (bounded, injectable sleep) for the in-flight restart to release the lock before relaunching, breaks a stale lock left by a crashed owner, and after a max wait breaks the lock to avoid a permanent deadlock. Both full-fleet and single-agent restart paths are guarded; start/stop/status are unchanged. Regression test reproduces the race: with an in-flight lock held, the restart must wait before issuing any systemctl command — fails on the unguarded code path, passes with the guard. Adds stale-lock-break and lock-release coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>	2026-06-24 19:18:38 -05:00

jarvisandClaude Opus 4.8

786762587d

fix(fleet): serialize restart-lock transitions to close concurrent-breaker race (review #680 )

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/push/ci Pipeline was successful

Details

Stale/max-wait takeover was not safe against concurrent breakers: two
breakers could both judge the lock stale and both proceed, re-introducing
the tight-loop. POSIX/Node has no content- or inode-conditional unlink or
rename, so "judge stale, then replace" can never be atomic with pure path
ops.

Serialize ALL lock transitions (acquire, release, takeover) under one
short-lived registry mutex held only across a few fs ops, never across the
restart itself. This makes check-then-mutate atomic, so exactly one breaker
can take over a stale lock while the others wait and re-evaluate.

The mutex itself uses mtime-based staleness (open('wx') creates an empty
inode before the token is written; a content check would reap a lock that is
still being acquired). The mutex populates-or-cleans-up on write failure so a
half-created mutex never leaks.

Regression coverage at two widths: a 2-breaker barrier test (exactly one
takes over, the other waits) and the existing 3-breaker test (maxActive===1,
distinct tokens, final lock released).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

2026-06-24 20:26:39 -05:00

jarvisandClaude Opus 4.8

43ad813e0d

fix(fleet): make restart-lock release/break ownership-safe (review #680 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was canceled

Details

Addresses the reviewer's blocker (comment 15915): release() unconditionally
unlinked restart.lock, so after a stale/max-wait break an OLD owner could
delete a NEWER owner's lock, letting a third restart interleave and defeating
the guard.

- Each acquire writes a unique owner token (randomUUID) into the lock file.
- release() only unlinks while that token is still on disk; once another caller
  has broken and re-owned the lock, the timed-out original owner's release() is
  a no-op and leaves the new owner's lock intact.
- Breaking a stale/timed-out lock now takes ownership atomically via
  write-temp + rename (atomic replace) instead of a blind unlink-then-recreate;
  a breaker that loses a concurrent takeover reads back a foreign token and
  keeps waiting rather than assuming ownership.

Regression test (does not let a timed-out owner drop a lock another restart
broke and re-owned) reproduces the three-restart interleave: R1 hangs (stale),
R2 breaks + re-owns, R1.release() must NOT drop R2's lock. Fails on the old
blind-unlink path (ENOENT), passes now. Also adds explicit single-agent
restart-path guard coverage (review should-fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

2026-06-24 19:18:38 -05:00

jarvisandClaude Opus 4.8

9c2e4f0b2d

fix(fleet): guard mosaic fleet restart against tight-loop re-entry race

`mosaic fleet restart` runs as a fresh process each invocation, issuing
`systemctl --user restart` for the tmux holder and then each agent. The
agent sessions live inside the holder's tmux, so restarting the holder
tears them down. With no mutual exclusion, a second restart entering
while the first is still mid-teardown (upgrade `--relaunch`, a watchdog,
or a hurried operator) interleaves: agents relaunch against a
half-torn-down holder, fail, and tight-loop.

Add a cross-process teardown-settle guard: a lock file under
`<mosaicHome>/fleet/run/restart.lock` acquired with O_CREAT|O_EXCL.
A re-entrant restart waits (bounded, injectable sleep) for the in-flight
restart to release the lock before relaunching, breaks a stale lock left
by a crashed owner, and after a max wait breaks the lock to avoid a
permanent deadlock. Both full-fleet and single-agent restart paths are
guarded; start/stop/status are unchanged.

Regression test reproduces the race: with an in-flight lock held, the
restart must wait before issuing any systemctl command — fails on the
unguarded code path, passes with the guard. Adds stale-lock-break and
lock-release coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

2026-06-24 19:18:38 -05:00

fix(fleet): guard mosaic fleet restart against tight-loop re-entry race #680

fix(fleet): guard `mosaic fleet restart` against tight-loop re-entry race #680