From bfac14108f19fb668762051f5be7732fc5cfa694 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 16:00:58 +0700 Subject: [PATCH] fleetd #544: pin the sticky-STOPPED-across-restart invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of PR #559 (issue comment #16944) found a surviving mutant: removing watchdog.reset() from StatusPoller.start() (and the identical line in SessionReaper.start()) passed the entire suite. stoppedByCaller is sticky and reset() — called only from start() — is the only thing that clears it. Both loops document start() as idempotent and loop()'s own error log says "it can be restarted", so stop() followed by start() is an anticipated path. Without reset() wired into start(), health() would report STOPPED forever after a restart even though the loop is genuinely running again. Add aRestartedLoopReportsRunningAgainNotStoppedForever to both StatusPollerWatchdogTest and SessionReaperWatchdogTest, pinning "an intentional stop must not outlive the restart that follows it". Verified via the standard mutation cycle: exact-line anchor (not regex, to avoid the \Q-style false match the reviewer flagged) counted pristine 1 -> mutated 0, test goes red with its own message, restored, shasum -a 256 byte-identical, green again. mvn clean install: exit 0, BUILD SUCCESS, Tests run: 1734, Failures: 0, Errors: 0, Skipped: 0 (cross-checked against 130 surefire report files). No production code changed — the reset() call under test was already correct; it simply had nothing pinning it. 🤖 Generated with Claude Code Co-Authored-By: Claude --- .../fleet/inject/StatusPollerWatchdogTest.java | 18 ++++++++++++++++++ .../session/SessionReaperWatchdogTest.java | 17 +++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerWatchdogTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerWatchdogTest.java index e575305..fdfb7bb 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerWatchdogTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerWatchdogTest.java @@ -84,6 +84,24 @@ class StatusPollerWatchdogTest { "stop() must report STOPPED, never STALLED, however stale the last round looks"); } + @Test + void aRestartedLoopReportsRunningAgainNotStoppedForever() throws Exception { + // fleetd #544 review (issue comment #16944): stoppedByCaller is sticky, and reset() — + // called only from start() — is the sole thing that clears it. start() is documented + // idempotent and loop()'s own error line says "it can be restarted", so stop() followed + // by start() is an anticipated path. Without the reset() call in start(), health() would + // report STOPPED forever after a restart even though the loop is genuinely running again. + AtomicLong clock = new AtomicLong(0); + StatusPoller poller = new StatusPoller(new AgentControl(new IdleHerdr()), new Injector(new AgentControl(new IdleHerdr())), + new StatusRefiner(new AgentControl(new IdleHerdr())), INTERVAL_MILLIS, clock::get); + poller.start(); + poller.stop(); + poller.start(); + + assertEquals(LoopWatchdog.State.RUNNING, poller.health(), + "an intentional stop must not outlive the restart that follows it"); + } + @Test void aHealthyLoopReportsRunning() throws Exception { // Real elapsed time on purpose, unlike the other tests here: a clock frozen at 0 would read diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/SessionReaperWatchdogTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/SessionReaperWatchdogTest.java index 9993db1..afaa622 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/session/SessionReaperWatchdogTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/session/SessionReaperWatchdogTest.java @@ -88,6 +88,23 @@ class SessionReaperWatchdogTest { "stop() must report STOPPED, never STALLED, however stale the last round looks"); } + @Test + void aRestartedLoopReportsRunningAgainNotStoppedForever() throws Exception { + // fleetd #544 review (issue comment #16944): stoppedByCaller is sticky, and reset() — + // called only from start() — is the sole thing that clears it. start() is documented + // idempotent and loop()'s own error line says "it can be restarted", so stop() followed + // by start() is an anticipated path. Without the reset() call in start(), health() would + // report STOPPED forever after a restart even though the loop is genuinely running again. + AtomicLong watchdogClock = new AtomicLong(0); + SessionReaper reaper = new SessionReaper(sessionManager(System::nanoTime), 60, INTERVAL_MILLIS, watchdogClock::get); + reaper.start(); + reaper.stop(); + reaper.start(); + + assertEquals(LoopWatchdog.State.RUNNING, reaper.health(), + "an intentional stop must not outlive the restart that follows it"); + } + @Test void aHealthyLoopReportsRunning() throws Exception { // Real elapsed time on purpose, unlike the other tests here: a clock frozen at 0 would read