fix(daemon): enforce single-owner machine lock and loud version-skew#5494
fix(daemon): enforce single-owner machine lock and loud version-skew#5494fsafey wants to merge 1 commit into
Conversation
|
@fsafey is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
88b8f1b to
d529be2
Compare
|
Updated: rebased onto current main ( |
Two daemon processes on one machine — the CLI-spawned daemon and the Desktop-spawned daemon under a different profile — could register the same identity and runtime IDs and, because the local-directory guard is a process-local mutex, both write the same checkout at once (VWO-364/VWO-365). The per-profile health-port and PID-file guards never see each other. Enforce a single authoritative owner with a machine-global advisory file lock at ~/.multica/daemon.lock, taken at the top of daemon.Run before any health bind or registration, so a second process never advertises. The OS releases the lock on process death, so crash recovery and stale-owner recovery are automatic with no lease TTL, no stale window, and no manual database or file surgery. The launcher additionally: refuses to start while a daemon that does NOT hold the lock is alive on any known profile's health port (a pre-lock release or a break-glass daemon), closing the rolling-upgrade window; distinguishes a live incumbent (fast actionable conflict naming pid/profile/port, with a profile-scoped stop hint) from a shutting-down one (bounded wait sized to the full 30s task-drain + 5s deregister shutdown bound, so restart never spuriously conflicts); and supports `daemon start --takeover` for a graceful cross-profile handoff via the incumbent's recorded health port. MULTICA_DAEMON_ALLOW_MULTIPLE is the documented break-glass / rollback. Also make a client/server version skew fail loudly instead of 404-looping: the server reports server_version in the register response, the daemon gates on it after the initial register (with runtimes deregistered on a version-mismatch exit), and a missing /prepare-lease route now logs one actionable error and stops instead of retrying every 15s forever. Cross-platform lock via x/sys (flock on unix, LockFileEx on windows; no new dependency). Covered by unit and cross-process tests for simultaneous start, clean shutdown, crash recovery, stale lock, duplicate identity, version mismatch, supported takeover, and the unlocked-daemon upgrade sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d529be2 to
a2004f8
Compare
|
Second review pass (Claude Opus 4.8, high effort) on the previous revision surfaced four issues, all fixed in |
Problem
On a host running both the CLI-spawned daemon (default profile) and the
Desktop-spawned daemon (a
--profile desktop-<host>daemon), two daemonprocesses register the same identity and runtime IDs and both write the same
checkout at once. The local-directory guard that is supposed to prevent
concurrent writes is a process-local
sync.Mutex, so each daemon holds anindependent mutex and neither sees the other. The two existing single-instance
guards — the per-profile health-port bind in
daemon.Runand the per-profilePID file — key on the profile, so a daemon under a different profile is invisible
to them.
This was observed in production: CLI daemon (0.3.22) and Desktop daemon (0.4.2)
serving the same daemon id, both advertising the same Claude/Codex/Pi runtime
IDs, and both acquiring the same checkout for different tasks within the same
minute. The Desktop daemon also 404-looped on
/prepare-lease— a route theolder server lacked — with no loud signal.
The upstream direction (
fix/daemon-id-machine-scoped) deliberately makes CLI +Desktop share one machine-scoped identity. This change is what makes that
shared-identity model safe: it enforces a single live owner of the machine.
Design decision (please weigh in)
A single authoritative ownership mechanism: a machine-global advisory file
lock at
~/.multica/daemon.lock, taken at the very top ofdaemon.Runbeforethe health bind or any registration.
daemon process per machine's shared checkouts — which the process-local mutex
cannot enforce. Keying on the base config dir (shared by every profile) is also
what catches the bug on today's
main, where CLI and Desktop still mintdifferent per-profile identities. It composes with
fix/daemon-id-machine-scopedwhether or not that lands.window, needs no server round-trip, and the OS releases it on process death —
so crash recovery and stale-owner recovery are automatic, with no TTL to tune
and no stale window, and no manual database/file surgery.
The trade-off to confirm: this makes two concurrent per-profile daemons on
one machine mutually exclusive. The per-profile health-port scheme hints the
project may have intended to allow that (e.g. one daemon per backend). If you
want per-backend concurrency, the lock should be keyed differently and I'll
adjust. The break-glass
MULTICA_DAEMON_ALLOW_MULTIPLE=1is the escape hatch fora deliberate multi-backend setup and the documented rollback.
What changed
Ownership (the core fix)
server/internal/daemon/ownership.go+ownership_unix.go(flock) +ownership_windows.go(LockFileExon a high sentinel byte so content readsstay unblocked). No new dependency — uses
golang.org/x/sys, already vendored.daemon.Runacquires the lock first and releases it last (afterderegisterRuntimes). A second daemon fails with an actionable error namingthe incumbent (pid, version, profile, health port) and a profile-scoped stop
hint (
multica --profile <p> daemon stop) so the remedy actually reaches across-profile owner.
does NOT hold the lock is alive on any known profile's health port. A
lock-aware daemon acquires before binding health, so lock-free + health-alive
can only be a pre-lock release (or a break-glass daemon) — without this, a new
daemon would silently start alongside a legacy one and recreate the
two-writer hazard.
daemon start --takeover/daemon restartperforms a graceful cross-profilehandoff via the incumbent's recorded health port (cross-platform; no OS
signals). The launcher distinguishes a live competitor (health up → fast
conflict) from a shutting-down incumbent (health down → bounded wait sized to
the full graceful-shutdown bound: 30s in-flight task drain + 5s deregister +
margin = 45s) so
restartnever spuriously conflicts mid-shutdown.MULTICA_DAEMON_ALLOW_MULTIPLEbreak-glass / rollback.Version compatibility (loud, not a 404 loop)
server_versionin the/api/daemon/registerresponse.preflight, before claiming any task; a version-mismatch exit deregisters its
runtimes on the way out so the server does not keep routing tasks to a dead
process until the sweeper catches up. Unreported/dev/unparsable versions are
treated as unknown (warn + proceed, no false rejection).
/prepare-lease404 that means "route missing" logs oneactionable error and stops the refresh loop, instead of Warn-ing every 15s
forever.
MinServerVersion = 0.3.28is the evidence-backed floor (firstrelease carrying
/prepare-lease) — a "required route exists" floor, not afull compatibility matrix.
Docs:
docs/daemon-single-owner.md— operator verification, takeover,upgrade ordering, and rollback.
Cross-model review
The change was independently reviewed by GPT-5.6 and Claude Opus 4.8 (both high
effort); all findings from both are addressed in this revision:
daemon is alive on any profile port), which also skips foreign-environment
daemons (a WSL2/other-OS daemon visible via localhost forwarding, per the
[Bug]: Desktop "Auto-start on launch" / "Auto-stop on quit" don't control the daemon running in WSL2 #3916 topology) rather than refusing with a stop hint the user can't act on.
router catch-all ("404 page not found") instead of excluding known bodies, so
a transient handler-level 404 (JSON
{"error":...}— the server maps DBhiccups in access checks to not-found) stays retryable and can never
permanently stop the lease extender with a false version-skew verdict.
incumbent is already draining: it closes its health listener at the start of
shutdown) now falls through to the bounded wait; a wedged incumbent fails
loudly with a manual remedy instead of a SIGKILL that would skip the drain and
orphan agent subprocesses mid-write.
the direct
--foregroundpath too.Known follow-up (deliberately out of scope): when the Desktop app's spawn of
daemon startfails with the ownership conflict, the app shows "stopped"without the reason —
startDaemoninapps/desktop/src/main/daemon-manager.tsresolves{success:false, error: err.message}(the actionable text is in there via stderr) butsendStatus({state:"stopped"})carries no reason, andDaemonStatushas noerror field. Behavior is safe (the second daemon correctly does not start);
surfacing the reason needs a small
DaemonStatus.reason?+ UI change, best doneas a Desktop-side follow-up.
Test evidence
Rebased onto current
main(ad7b23896, v0.4.3); merges clean. Ran locallywith Go 1.26.1 on darwin/arm64:
go build ./...— pass (unix andGOOS=windowscross-build).go vet+gofmt -lon all changed files — clean.go test -count=1):internal/daemon,cmd/multica,pkg/agent— pass.go test -raceon the ownership / version-gate / takeover / sweep tests — pass.TestAcquireOwnership_SecondFailsWhileHeldTestAcquireOwnership_ReleaseAllowsReacquireTestAcquireOwnership_CrashRecoveryTestAcquireOwnership_StaleLockFileReclaimedTestCheckMinServerVersion,TestServerCompatibilityGate,TestPrepareLeaseExtender_UnsupportedRouteStopsLoudlyTestTakeoverDaemonOwner_ShutsDownAndWaitsTestDetectUnlockedDaemonTestOwnershipConflict_ProfileAwareStopHintTestOwnershipBypassedScope note: this is code-level root-cause analysis plus a cross-process test
proving the lock mutually excludes two real OS processes on an isolated temp
base dir. Not run locally:
go test -race ./...and the DB-gated handlerregister tests (no local test Postgres) — both covered by this repo's
backendCI job, green on this PR.
Rollout
refuses to start while a pre-lock daemon is alive on any profile.
against an old server that predates
server_versionwarns and relies on the/prepare-leasebackstop rather than refusing startup.export MULTICA_DAEMON_ALLOW_MULTIPLE=1and restart — restores theprior no-guard behaviour without redeploying.
🤖 Generated with Claude Code