From 7179be24fa4efa2b15fe1c4613cd3219912d9c7c Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 30 Jun 2026 00:24:29 -0500 Subject: [PATCH] feat: skip autostop reminder for active workspaces (#26772) --- coderd/autobuild/lifecycle_executor.go | 49 ++++++-- .../lifecycle_executor_internal_test.go | 74 +++++++++++- coderd/autobuild/lifecycle_executor_test.go | 114 ++++++++++++++++-- coderd/database/queries.sql.go | 25 +++- coderd/database/queries/workspaces.sql | 25 +++- 5 files changed, 264 insertions(+), 23 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 668772f05b..c2dee11f92 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -323,7 +323,7 @@ func (e *Executor) runOnce(t time.Time) Stats { // A deadline change (e.g. activity bump) re-arms the reminder; users near // the boundary may receive one reminder per bump. Intentional: one-per-build // would leave stale reminders after a bump. - if shouldRemindAutostop(latestBuild, templateSchedule, currentTick) { + if shouldRemindAutostop(latestBuild, ws.LastUsedAt, templateSchedule, currentTick) { if err := tx.UpdateWorkspaceBuildNotifiedAutostopDeadline(e.ctx, database.UpdateWorkspaceBuildNotifiedAutostopDeadlineParams{ ID: latestBuild.ID, NotifiedAutostopDeadline: latestBuild.Deadline, @@ -598,17 +598,30 @@ func (e *Executor) runOnce(t time.Time) Stats { return stats } -// shouldRemindAutostop reports whether a reminder notification should be sent -// for the workspace's latest build at currentTick. +// autostopReminderActiveThreshold is how recently a workspace must have been +// used to count as "active" and suppress the autostop reminder. A default +// deployment refreshes last_used_at within ~90s, but the agent stats interval +// is configurable up to a few minutes, so we stay conservative. It must remain +// well below activity_bump (default 1h): an idle user's now-last_used_at is +// bounded by activity_bump, so a larger threshold would make idle users look +// active forever and never be reminded. Keep in sync with the INTERVAL '15 +// minutes' literal in GetWorkspacesEligibleForLifecycleAction (workspaces.sql). +const autostopReminderActiveThreshold = 15 * time.Minute + +// shouldRemindAutostop reports whether an autostop reminder should be sent for +// the build at currentTick. It skips genuinely-active workspaces only when +// activity can still move the deadline out of the lead window. // -// time_til_autostop_notify has no upper bound. If it exceeds a -// workspace's remaining lifetime, the notify window already covers "now" at -// build creation. This is still safe: we require deadline > now (so we never -// remind once the stop is due) and the marker (NotifiedAutostopDeadline == -// Deadline, stamped in the transaction before the send attempt) filters every -// subsequent tick. The result is exactly one reminder per deadline, never one -// per tick. -func shouldRemindAutostop(build database.WorkspaceBuild, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool { +// time_til_autostop_notify has no upper bound, so the lead window can already +// cover "now" at build creation. The result is still exactly one reminder per +// deadline (never one per tick): we require deadline > now, and the marker +// (NotifiedAutostopDeadline == Deadline, stamped before the send attempt) +// filters every subsequent tick. +// +// The skip-guard below is the exact complement of the keep-condition in the +// reminder arm of GetWorkspacesEligibleForLifecycleAction, so a row that passes +// the SQL pre-filter also passes this re-check (and vice versa). +func shouldRemindAutostop(build database.WorkspaceBuild, lastUsedAt time.Time, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool { if templateSchedule.TimeTilAutostopNotify <= 0 { return false } @@ -627,6 +640,20 @@ func shouldRemindAutostop(build database.WorkspaceBuild, templateSchedule schedu return false } + // Skip the reminder only for an active user whose deadline can still be + // bumped out of the lead window. + userActive := currentTick.Sub(lastUsedAt) < autostopReminderActiveThreshold + bumpEnabled := templateSchedule.ActivityBump > 0 + // The hard ceiling traps the workspace inside the window: a non-zero + // max_deadline at or before now+ttl means no bump can push the stop out of + // the lead window, so it WILL stop regardless of activity. + maxDeadlineTraps := !build.MaxDeadline.IsZero() && + !build.MaxDeadline.After(currentTick.Add(templateSchedule.TimeTilAutostopNotify)) + + if userActive && bumpEnabled && !maxDeadlineTraps { + return false + } + // Idempotence: a reminder has not yet been sent for THIS deadline. The // marker re-arms automatically when the deadline changes (e.g. an activity // bump), so a new reminder fires once the new deadline re-enters the window. diff --git a/coderd/autobuild/lifecycle_executor_internal_test.go b/coderd/autobuild/lifecycle_executor_internal_test.go index 3505ae0705..6410e21108 100644 --- a/coderd/autobuild/lifecycle_executor_internal_test.go +++ b/coderd/autobuild/lifecycle_executor_internal_test.go @@ -153,9 +153,16 @@ func TestShouldRemindAutostop(t *testing.T) { } } + // idle places last_used_at well outside the 15-minute active threshold so the + // active-user guard never trips. It is the default for cases that leave + // LastUsedAt unset; cases that exercise the active-user guard set LastUsedAt + // explicitly. + idle := currentTick.Add(-2 * ttl) + testCases := []struct { Name string Build database.WorkspaceBuild + LastUsedAt time.Time TemplateSchedule schedule.TemplateScheduleOptions Expected bool }{ @@ -246,13 +253,78 @@ func TestShouldRemindAutostop(t *testing.T) { TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl}, Expected: true, }, + { + // ActiveUser: the workspace was used within the 15-minute active + // threshold and activity bumps are enabled with no max_deadline + // ceiling, so the deadline can keep getting bumped out of the window + // and the reminder is suppressed. + Name: "ActiveUser", + Build: inWindow(), + LastUsedAt: currentTick.Add(-1 * time.Minute), + TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl, ActivityBump: time.Hour}, + Expected: false, + }, + { + // ActiveButMaxDeadlineWithinWindow: the user is active and bumps are + // enabled, but the hard max_deadline ceiling sits inside the lead + // window, so a bump cannot push the stop out. The workspace will stop + // regardless of activity, so we still remind. + Name: "ActiveButMaxDeadlineWithinWindow", + Build: func() database.WorkspaceBuild { + b := inWindow() + b.MaxDeadline = currentTick.Add(ttl / 2) + return b + }(), + LastUsedAt: currentTick.Add(-1 * time.Minute), + TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl, ActivityBump: time.Hour}, + Expected: true, + }, + { + // ActiveButBumpDisabled: the user is active, but activity bumps are + // disabled (activity_bump == 0), so the deadline cannot move. The + // workspace will stop, so we still remind. + Name: "ActiveButBumpDisabled", + Build: inWindow(), + LastUsedAt: currentTick.Add(-1 * time.Minute), + TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl, ActivityBump: 0}, + Expected: true, + }, + { + // IdleUser: the workspace was last used 20 minutes ago, outside the + // 15-minute active threshold, so it is not active and the reminder + // fires. Activity bumps are enabled here, so the true result is + // genuinely due to idleness and not to disabled bumping. + Name: "IdleUser", + Build: inWindow(), + LastUsedAt: currentTick.Add(-20 * time.Minute), + TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl, ActivityBump: time.Hour}, + Expected: true, + }, + { + // Exactly at the threshold: the Go guard (< threshold) treats + // the user as not active and reminds; the SQL complement + // (>= threshold) keeps the row. Both agree; pins the boundary + // against a "<"/">=" off-by-one regression. + Name: "ActiveThresholdBoundary", + Build: inWindow(), + LastUsedAt: currentTick.Add(-autostopReminderActiveThreshold), + TemplateSchedule: schedule.TemplateScheduleOptions{TimeTilAutostopNotify: ttl, ActivityBump: time.Hour}, + Expected: true, + }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { t.Parallel() - require.Equal(t, tc.Expected, shouldRemindAutostop(tc.Build, tc.TemplateSchedule, currentTick)) + // Cases that do not exercise the active-user guard leave LastUsedAt + // unset; default those to an idle time outside the lead window. + lastUsedAt := tc.LastUsedAt + if lastUsedAt.IsZero() { + lastUsedAt = idle + } + + require.Equal(t, tc.Expected, shouldRemindAutostop(tc.Build, lastUsedAt, tc.TemplateSchedule, currentTick)) }) } } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index bda41dc3f3..973043d69d 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1898,6 +1898,7 @@ func setupTestDBPrebuiltWorkspace( // and observe notifications. func setupAutostopReminderWorkspace(t *testing.T, timeTilAutostopNotify time.Duration, enq notifications.Enqueuer) ( client *codersdk.Client, + db database.Store, tickCh chan time.Time, statsCh chan autobuild.Stats, workspace codersdk.Workspace, @@ -1906,7 +1907,7 @@ func setupAutostopReminderWorkspace(t *testing.T, timeTilAutostopNotify time.Dur tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, AutobuildStats: statsCh, IncludeProvisionerDaemon: true, @@ -1930,7 +1931,19 @@ func setupAutostopReminderWorkspace(t *testing.T, timeTilAutostopNotify time.Dur // The build must have a non-zero deadline for a reminder to ever fire. require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) require.NotZero(t, workspace.LatestBuild.Deadline) - return client, tickCh, statsCh, workspace + + // Age last_used_at far before every tick so the active-user guard never + // trips for the default subtests. A freshly created workspace has a recent + // last_used_at, which would otherwise look "active" and suppress the + // reminder. Subtests that exercise the active-user guard reset last_used_at + // to a recent value via db. + ctx := dbauthz.AsSystemRestricted(context.Background()) + require.NoError(t, db.UpdateWorkspaceLastUsedAt(ctx, database.UpdateWorkspaceLastUsedAtParams{ + ID: workspace.ID, + LastUsedAt: workspace.LatestBuild.Deadline.Time.Add(-365 * 24 * time.Hour), + })) + + return client, db, tickCh, statsCh, workspace } // failOnceEnqueuer fails its first Enqueue call and delegates every subsequent @@ -1964,7 +1977,7 @@ func TestExecutorAutostopReminder(t *testing.T) { timeTilNotify := 30 * time.Minute notifyEnq := ¬ificationstest.FakeEnqueuer{} - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time go func() { @@ -1988,13 +2001,96 @@ func TestExecutorAutostopReminder(t *testing.T) { require.Contains(t, sent[0].Targets, workspace.OrganizationID) }) + // ActiveWorkspaceNotReminded: a workspace used within the 15-minute active + // threshold keeps getting its deadline bumped, so no reminder is sent even + // though the tick lands inside the window. This is the active-user guard, the + // exact complement of the Sent subtest. + t.Run("ActiveWorkspaceNotReminded", func(t *testing.T) { + t.Parallel() + + timeTilNotify := 30 * time.Minute + notifyEnq := ¬ificationstest.FakeEnqueuer{} + _, db, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + deadline := workspace.LatestBuild.Deadline.Time + + // Tick halfway into the lead window, exactly as the Sent subtest does. + tick := deadline.Add(-timeTilNotify / 2) + + // Mark the workspace as recently used: last_used_at within the 15-minute + // active threshold of the tick makes currentTick - last_used_at < + // autostopReminderActiveThreshold, so the active-user guard suppresses the + // reminder (and the SQL pre-filter drops the row). + ctx := dbauthz.AsSystemRestricted(context.Background()) + require.NoError(t, db.UpdateWorkspaceLastUsedAt(ctx, database.UpdateWorkspaceLastUsedAtParams{ + ID: workspace.ID, + LastUsedAt: tick.Add(-time.Minute), + })) + + go func() { + tickCh <- tick + close(tickCh) + }() + + stats := testutil.TryReceive(testutil.Context(t, testutil.WaitShort), t, statsCh) + require.Len(t, stats.Errors, 0) + require.Empty(t, notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceAutostopReminder))) + }) + + // ActiveWorkspaceAtMaxDeadlineReminded: an active workspace is still + // reminded when the hard max_deadline ceiling sits inside the lead window. + // Activity bumps cannot push the stop past max_deadline, so the workspace + // will stop regardless of activity and the reminder must fire. This is the + // max_deadline override of the active-user guard. + t.Run("ActiveWorkspaceAtMaxDeadlineReminded", func(t *testing.T) { + t.Parallel() + + ctx := dbauthz.AsSystemRestricted(context.Background()) + timeTilNotify := 30 * time.Minute + notifyEnq := ¬ificationstest.FakeEnqueuer{} + _, db, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + deadline := workspace.LatestBuild.Deadline.Time + + // Tick halfway into the lead window, exactly as the Sent subtest does. + tick := deadline.Add(-timeTilNotify / 2) + + // Mark the workspace as recently used (active): without the max_deadline + // ceiling this would suppress the reminder, see ActiveWorkspaceNotReminded. + require.NoError(t, db.UpdateWorkspaceLastUsedAt(ctx, database.UpdateWorkspaceLastUsedAtParams{ + ID: workspace.ID, + LastUsedAt: tick.Add(-time.Minute), + })) + + // Pin the build's max_deadline inside the lead window (max_deadline <= + // tick + ttl). A bump cannot move the stop past this ceiling, so the + // workspace will stop even though the user is active and the reminder + // must still fire. The deadline itself is left unchanged. + require.NoError(t, db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ + ID: workspace.LatestBuild.ID, + Deadline: deadline, + MaxDeadline: deadline, + UpdatedAt: tick, + })) + + go func() { + tickCh <- tick + close(tickCh) + }() + + stats := testutil.TryReceive(testutil.Context(t, testutil.WaitShort), t, statsCh) + require.Len(t, stats.Errors, 0) + + sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceAutostopReminder)) + require.Len(t, sent, 1) + require.Equal(t, workspace.OwnerID, sent[0].UserID) + }) + // NotBeforeWindow: no reminder when the tick precedes the lead window. t.Run("NotBeforeWindow", func(t *testing.T) { t.Parallel() timeTilNotify := 30 * time.Minute notifyEnq := ¬ificationstest.FakeEnqueuer{} - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time go func() { @@ -2013,7 +2109,7 @@ func TestExecutorAutostopReminder(t *testing.T) { t.Parallel() notifyEnq := ¬ificationstest.FakeEnqueuer{} - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, 0, notifyEnq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, 0, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time go func() { @@ -2033,7 +2129,7 @@ func TestExecutorAutostopReminder(t *testing.T) { timeTilNotify := 30 * time.Minute notifyEnq := ¬ificationstest.FakeEnqueuer{} - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time // First tick: reminder fires. Receiving from statsCh acts as the @@ -2063,7 +2159,7 @@ func TestExecutorAutostopReminder(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) timeTilNotify := 30 * time.Minute notifyEnq := ¬ificationstest.FakeEnqueuer{} - client, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + client, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time // First tick: reminder fires for the original deadline. @@ -2105,7 +2201,7 @@ func TestExecutorAutostopReminder(t *testing.T) { // includes "now" at build creation. timeTilNotify := 100 * time.Hour notifyEnq := ¬ificationstest.FakeEnqueuer{} - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, notifyEnq) deadline := workspace.LatestBuild.Deadline.Time // First tick: a single reminder fires. @@ -2138,7 +2234,7 @@ func TestExecutorAutostopReminder(t *testing.T) { fake := ¬ificationstest.FakeEnqueuer{} enq := &failOnceEnqueuer{Enqueuer: fake} timeTilNotify := 2 * time.Hour - _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, enq) + _, _, tickCh, statsCh, workspace := setupAutostopReminderWorkspace(t, timeTilNotify, enq) deadline := workspace.LatestBuild.Deadline.Time // Tick 1 inside the window: the enqueue fails. Because the marker is diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 44c0b73b20..64fe9b53dd 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -38278,6 +38278,10 @@ WHERE -- * The workspace is not dormant and its owner is not suspended. -- * The build has a deadline in the future (we never remind about a stop already due). -- * The template opts in (time_til_autostop_notify > 0) and now is within the lead window. + -- * The owner is not active in a way that can keep the workspace + -- alive: either they have not used it within the active threshold + -- (15 minutes), or activity bumps are disabled, or the max_deadline + -- ceiling pins the stop inside the lead window so a bump cannot save it. -- * A reminder has not yet been sent for THIS deadline. -- -- NOTE: time_til_autostop_notify has no upper bound. If it exceeds a @@ -38298,7 +38302,26 @@ WHERE workspace_builds.deadline > $1::timestamptz AND templates.time_til_autostop_notify > 0 AND workspace_builds.deadline <= ($1::timestamptz) + (INTERVAL '1 millisecond' * (templates.time_til_autostop_notify / 1000000)) AND - workspace_builds.notified_autostop_deadline != workspace_builds.deadline + workspace_builds.notified_autostop_deadline != workspace_builds.deadline AND + -- Keep the reminder unless the user is active AND an activity bump can + -- still move the deadline out of the lead window. This block is the + -- exact complement of the skip-guard in shouldRemindAutostop (Go) + -- (userActive AND bumpEnabled AND NOT maxDeadlineTraps), so the + -- pre-filter and the re-check agree on the boundary. + ( + -- Not used within the active threshold (15 minutes). This is the exact + -- complement of the < autostopReminderActiveThreshold guard in + -- shouldRemindAutostop (Go); keep the two in sync. + ($1 :: timestamptz) - workspaces.last_used_at >= INTERVAL '15 minutes' + -- ...or activity bumps are disabled (deadline can't move)... + OR templates.activity_bump <= 0 + -- ...or the hard max_deadline ceiling is within the lead window, so + -- the workspace will stop regardless of activity. + OR ( + workspace_builds.max_deadline != '0001-01-01 00:00:00+00'::timestamptz + AND workspace_builds.max_deadline <= ($1::timestamptz) + (INTERVAL '1 millisecond' * (templates.time_til_autostop_notify / 1000000)) + ) + ) ) ) AND workspaces.deleted = 'false' diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 9be84a6b6f..e8b1885a2d 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -873,6 +873,10 @@ WHERE -- * The workspace is not dormant and its owner is not suspended. -- * The build has a deadline in the future (we never remind about a stop already due). -- * The template opts in (time_til_autostop_notify > 0) and now is within the lead window. + -- * The owner is not active in a way that can keep the workspace + -- alive: either they have not used it within the active threshold + -- (15 minutes), or activity bumps are disabled, or the max_deadline + -- ceiling pins the stop inside the lead window so a bump cannot save it. -- * A reminder has not yet been sent for THIS deadline. -- -- NOTE: time_til_autostop_notify has no upper bound. If it exceeds a @@ -893,7 +897,26 @@ WHERE workspace_builds.deadline > @now::timestamptz AND templates.time_til_autostop_notify > 0 AND workspace_builds.deadline <= (@now::timestamptz) + (INTERVAL '1 millisecond' * (templates.time_til_autostop_notify / 1000000)) AND - workspace_builds.notified_autostop_deadline != workspace_builds.deadline + workspace_builds.notified_autostop_deadline != workspace_builds.deadline AND + -- Keep the reminder unless the user is active AND an activity bump can + -- still move the deadline out of the lead window. This block is the + -- exact complement of the skip-guard in shouldRemindAutostop (Go) + -- (userActive AND bumpEnabled AND NOT maxDeadlineTraps), so the + -- pre-filter and the re-check agree on the boundary. + ( + -- Not used within the active threshold (15 minutes). This is the exact + -- complement of the < autostopReminderActiveThreshold guard in + -- shouldRemindAutostop (Go); keep the two in sync. + (@now :: timestamptz) - workspaces.last_used_at >= INTERVAL '15 minutes' + -- ...or activity bumps are disabled (deadline can't move)... + OR templates.activity_bump <= 0 + -- ...or the hard max_deadline ceiling is within the lead window, so + -- the workspace will stop regardless of activity. + OR ( + workspace_builds.max_deadline != '0001-01-01 00:00:00+00'::timestamptz + AND workspace_builds.max_deadline <= (@now::timestamptz) + (INTERVAL '1 millisecond' * (templates.time_til_autostop_notify / 1000000)) + ) + ) ) ) AND workspaces.deleted = 'false'