From f379a6d059e8ebf3b5a519be840402b64149b7f3 Mon Sep 17 00:00:00 2001 From: typebasedio Date: Fri, 15 May 2026 14:26:15 -0400 Subject: [PATCH] feat(v0.6): IsStale supplements wall-clock freshness with PID liveness --- internal/session/isstale_test.go | 101 +++++++++++++++++++++++++++++++ internal/session/lease.go | 35 +++++++++++ 2 files changed, 136 insertions(+) create mode 100644 internal/session/isstale_test.go diff --git a/internal/session/isstale_test.go b/internal/session/isstale_test.go new file mode 100644 index 0000000..4325097 --- /dev/null +++ b/internal/session/isstale_test.go @@ -0,0 +1,101 @@ +package session + +import ( + "testing" + "time" +) + +// withCheckProcess swaps the package-level checkProcess seam for the +// duration of a test and restores it on cleanup. Tests using it must not +// run with t.Parallel(). +func withCheckProcess(t *testing.T, fn func(pid int) ProcessState) { + t.Helper() + orig := checkProcess + checkProcess = fn + t.Cleanup(func() { checkProcess = orig }) +} + +func freshLocalLease(pid int) *Lease { + now := time.Now().UTC() + return &Lease{ + PID: pid, + Hostname: currentHostname(), + StartedAt: now, + LastHeartbeatAt: now, + } +} + +// Wall-clock staleness wins unconditionally — existing behavior preserved. +func TestIsStaleByWallClock(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessAlive }) + now := time.Now().UTC() + l := &Lease{PID: 999, Hostname: currentHostname(), LastHeartbeatAt: now.Add(-90 * time.Second)} + if !IsStale(l, now, StaleLeaseAfter) { + t.Error("a lease 90s past heartbeat must be stale even if PID is alive") + } +} + +// Fresh wall-clock + local host + dead owner PID => stale immediately. +func TestIsStaleByDeadPID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessDead }) + l := freshLocalLease(4242) + if !IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with a dead PID must be stale") + } +} + +// Fresh wall-clock + local host + live owner PID => not stale. +func TestIsStaleFreshWithLivePID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessAlive }) + l := freshLocalLease(4242) + if IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with a live PID must not be stale") + } +} + +// Remote lease => PID check skipped entirely; wall-clock only. +func TestIsStaleRemoteIgnoresPID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { + t.Error("checkProcess must not be called for a remote lease") + return ProcessDead + }) + now := time.Now().UTC() + l := &Lease{PID: 4242, Hostname: "some-other-host", LastHeartbeatAt: now} + if l.Hostname == currentHostname() { + t.Skip("test hostname collision; cannot construct a remote lease") + } + if IsStale(l, now, StaleLeaseAfter) { + t.Error("fresh remote lease must not be stale (PID check skipped)") + } +} + +// Inconclusive PID check => conservative; fall back to wall-clock fresh. +func TestIsStaleUnknownPIDConservative(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessUnknown }) + l := freshLocalLease(4242) + if IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with inconclusive PID check must not be stale") + } +} + +// pid <= 0 => skip PID liveness entirely; wall-clock only. +func TestIsStaleInvalidPIDSkipsCheck(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { + t.Error("checkProcess must not be called when pid <= 0") + return ProcessDead + }) + now := time.Now().UTC() + for _, pid := range []int{0, -1} { + l := &Lease{PID: pid, Hostname: currentHostname(), LastHeartbeatAt: now} + if IsStale(l, now, StaleLeaseAfter) { + t.Errorf("fresh local lease with pid=%d must not be stale (wall-clock only)", pid) + } + } +} + +// Defensive: a nil lease is stale. +func TestIsStaleNilLease(t *testing.T) { + if !IsStale(nil, time.Now().UTC(), StaleLeaseAfter) { + t.Error("nil lease must be reported stale") + } +} diff --git a/internal/session/lease.go b/internal/session/lease.go index b12efa2..86c0850 100644 --- a/internal/session/lease.go +++ b/internal/session/lease.go @@ -131,6 +131,41 @@ func IsFresh(l *Lease, now time.Time, threshold time.Duration) bool { return now.Sub(l.LastHeartbeatAt) <= threshold } +// IsStale reports whether the lease should be treated as stale. It +// supplements the wall-clock heartbeat threshold with PID liveness: +// +// - A heartbeat older than threshold is stale (existing behavior). +// - Otherwise, for a lease whose hostname matches the current host and +// whose PID is valid (> 0), a dead owner PID makes the lease stale +// immediately. This is the v0.6 lazy-cleanup signal: a Ctrl-C'd or +// terminal-closed session is recognized without the 60s wall-clock +// wait. +// - Remote leases, leases with pid <= 0, and inconclusive PID checks +// (ProcessUnknown) fall back to wall-clock freshness only. +// +// PID liveness can only flip a wall-clock-fresh lease to stale; it never +// revives a wall-clock-stale lease. IsStale is the freshness predicate +// for adoption and Layer-1 decisions — prefer it over a bare IsFresh +// call at any site that decides whether a session is still owned. +func IsStale(l *Lease, now time.Time, threshold time.Duration) bool { + if l == nil { + return true + } + if !IsFresh(l, now, threshold) { + return true // wall-clock stale — existing behavior + } + if l.PID <= 0 { + return false // no usable PID — wall-clock only + } + if l.Hostname != currentHostname() { + return false // remote lease — wall-clock only + } + if checkProcess(l.PID) == ProcessDead { + return true // local owner confirmed dead + } + return false // alive or inconclusive — conservative +} + // CleanupStaleLease inspects the lease at path: // - missing file: no-op, returns (nil, nil) // - corrupt / unparseable: removes the file, returns (nil, nil)