From 2ff05608d2d921ba3cda2f145a9ed78178d9d23c Mon Sep 17 00:00:00 2001
From: Ethan <39577870+ethanndickson@users.noreply.github.com>
Date: Thu, 7 May 2026 17:12:14 +1000
Subject: [PATCH] test: stabilize chatdebug heartbeat threshold test (#25022)

`launchHeartbeat` could miss a stale-threshold update during startup if
`SetStaleAfter` ran after the heartbeat ticker was created but before
the goroutine subscribed to `thresholdChan`. In that case, the heartbeat
kept the old interval until a future tick, and the mock-clock test could
time out waiting for `Ticker.Reset` without advancing time.

Subscribe to `thresholdChan` before reading the heartbeat interval so
the channel consistently invalidates the interval. The regression test
now changes the threshold while ticker creation is trapped, making the
startup race deterministic.

Closes https://github.com/coder/internal/issues/1513
---
 coderd/x/chatd/chatdebug/model.go             | 22 +++++++++++--------
 .../x/chatd/chatdebug/model_internal_test.go  | 12 +++++-----
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/coderd/x/chatd/chatdebug/model.go b/coderd/x/chatd/chatdebug/model.go
index 0ac7326080..e30a8a21e5 100644
--- a/coderd/x/chatd/chatdebug/model.go
+++ b/coderd/x/chatd/chatdebug/model.go
@@ -361,10 +361,20 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
 		return
 	}
 	go func() {
+		// Subscribe before reading the interval. The channel invalidates
+		// the interval, so any concurrent SetStaleAfter either happened
+		// before this interval read or will close thresholdCh below.
+		thresholdCh := svc.thresholdChan()
 		interval := svc.heartbeatInterval()
 		ticker := svc.clock.NewTicker(interval, "chatdebug", "heartbeat")
 		defer ticker.Stop()
-		thresholdCh := svc.thresholdChan()
+		resetTicker := func() {
+			if newInterval := svc.heartbeatInterval(); newInterval != interval {
+				interval = newInterval
+				ticker.Reset(interval, "chatdebug", "heartbeat")
+			}
+		}
+
 		for {
 			select {
 			case <-ctx.Done():
@@ -375,10 +385,7 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
 				// SetStaleAfter was called; re-read the interval
 				// and reset the ticker immediately.
 				thresholdCh = svc.thresholdChan()
-				if newInterval := svc.heartbeatInterval(); newInterval != interval {
-					interval = newInterval
-					ticker.Reset(interval, "chatdebug", "heartbeat")
-				}
+				resetTicker()
 			case <-ticker.C:
 				if err := svc.TouchStep(ctx, stepID, runID, chatID); err != nil {
 					svc.log.Debug(ctx, "heartbeat touch failed",
@@ -388,10 +395,7 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
 				}
 				// Also re-read interval on every tick as a
 				// secondary check.
-				if newInterval := svc.heartbeatInterval(); newInterval != interval {
-					interval = newInterval
-					ticker.Reset(interval, "chatdebug", "heartbeat")
-				}
+				resetTicker()
 			}
 		}
 	}()
diff --git a/coderd/x/chatd/chatdebug/model_internal_test.go b/coderd/x/chatd/chatdebug/model_internal_test.go
index a3386a7058..03bb51cab8 100644
--- a/coderd/x/chatd/chatdebug/model_internal_test.go
+++ b/coderd/x/chatd/chatdebug/model_internal_test.go
@@ -1361,13 +1361,15 @@ func TestLaunchHeartbeat(t *testing.T) {
 		// threshold/2 interval.
 		newCall := tickerTrap.MustWait(ctx)
 		require.Equal(t, 30*time.Second, newCall.Duration)
+
+		// Reduce the threshold while NewTicker is trapped. This
+		// simulates SetStaleAfter racing with heartbeat startup before
+		// the goroutine can select on thresholdCh.
+		svc.SetStaleAfter(10 * time.Second)
 		newCall.MustRelease(ctx)
 
-		// Reducing the threshold must wake the heartbeat via the
-		// thresholdChan close and trigger a ticker reset to
-		// newThreshold/2 without advancing the mock clock.
-		svc.SetStaleAfter(10 * time.Second)
-
+		// The heartbeat must still reset to newThreshold/2 without
+		// advancing the mock clock.
 		resetCall := resetTrap.MustWait(ctx)
 		require.Equal(t, 5*time.Second, resetCall.Duration,
 			"ticker should reset to newThreshold/2 when SetStaleAfter"+