From a76c51dfc45a165753f911cc68e75fddd9fbe484 Mon Sep 17 00:00:00 2001 From: Jake Howell Date: Mon, 3 Aug 2026 19:11:11 +1000 Subject: [PATCH] fix(site): correct autostop restart prompt on the workspace schedule page (#27632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 🤖 This PR was written by Coder Agents on behalf of Jake Howell. ## Problem Enabling autostop on a running workspace never showed the "restart now to apply?" dialog. The new TTL was saved, but a running build's deadline is only calculated when the build starts, so autostop silently did not take effect until a manual restart. Changing an already-enabled value did prompt, so the behavior was inconsistent. Separately, dismissing that dialog with **Apply later** navigated the user away to the workspace, exactly like confirming did. ## Root cause The dialog was gated on `getAutostop(workspace).autostopEnabled`, which derives from `workspace.ttl_ms` — the **pre-submit** state (the workspace is not refetched until after the mutation). That expression means "was autostop enabled *before* this change", which maps to: | Transition | Pre-submit enabled | Dialog | Correct? | |---|---|---|---| | Disabled → enabled (add) | `false` | not shown | ❌ the reported bug | | Enabled → new value (modify) | `true` | shown | ✅ | | Enabled → disabled (remove) | `true` | shown | ❌ (nothing to apply) | This guard came from #16085, which intended to *skip* the prompt when **removing** autostop (safe, because that PR also made the backend clear a running build's deadline server-side when TTL is set to null). By keying off the old state it actually skipped the prompt on **add** and still showed it on **remove** — the opposite of its intent. ## Fix - Key the prompt off the submitted form value (`values.autostopEnabled`), with an inline comment documenting the trigger conditions. This fixes the reported bug and restores #16085's intent: - add → prompt ✅ - modify → prompt ✅ - remove → no prompt ✅ (deadline cleared server-side) - stopped / unchanged → no prompt ✅ - **Apply later** now just closes the dialog and keeps the user on the schedule page; the saved value still applies on the next start. **Restart** is unchanged (restarts and navigates to the workspace). ## Testing Storybook `play`-function coverage on `WorkspaceSchedulePage` for all cases: enable, change value, disable, enable-while-stopped, autostart-only, and Apply-later-stays-on-page (the last also asserts `restartWorkspace` is not called). A prior story that asserted the pre-fix behavior (prompting on disable) was reworked. `tsc` and `biome` clean.
Investigation notes History of the gating condition: - Originally `if (data.autostopChanged) { ... }` — prompted on every autostop change (enable included). - #16085 ("allow removing deadline for running workspace", fixes #9775) added `&& getAutostop(workspace).autostopEnabled`. Its description states the goal was to "not show a confirmation dialog when the change is to remove autostop", and the same PR added backend logic in `putWorkspaceTTL` to clear a running build's deadline when TTL is null. - A later refactor added `&& workspace.latest_build.status === "running"`. Because the backend already clears the deadline live on removal, no restart is needed there; the frontend only needs to prompt when autostop ends up enabled on a running workspace. The fix keys off the submitted state so all three transitions behave correctly.
--- .../WorkspaceSchedulePage.stories.tsx | 105 +++++++++++++++--- .../WorkspaceSchedulePage.tsx | 16 ++- 2 files changed, 105 insertions(+), 16 deletions(-) diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.stories.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.stories.tsx index d6a2fbcf82..1af2905904 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.stories.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, spyOn, userEvent, within } from "storybook/test"; +import { expect, spyOn, userEvent, waitFor, within } from "storybook/test"; import { reactRouterOutlet, reactRouterParameters, @@ -73,13 +73,15 @@ export const EnablingAutostopUsesTemplateDefault: Story = { }, }; -export const ChangingAutostopShowsRestartDialog: Story = { +export const EnablingAutostopShowsRestartDialog: Story = { parameters: { - reactRouter: workspaceRouterParameters(MockWorkspace), - queries: workspaceQueries(MockWorkspace), + reactRouter: workspaceRouterParameters(autostopDisabledWorkspace), + queries: workspaceQueries(autostopDisabledWorkspace), }, beforeEach: () => { - spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace); + spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue( + autostopDisabledWorkspace, + ); }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -94,19 +96,94 @@ export const ChangingAutostopShowsRestartDialog: Story = { }, }; -const stoppedWorkspace: Workspace = { - ...MockWorkspace, - latest_build: { ...MockWorkspaceBuild, status: "stopped" }, -}; - -export const ChangingAutostopWhileStoppedSkipsDialog: Story = { +export const ApplyLaterKeepsUserOnSchedulePage: Story = { parameters: { - reactRouter: workspaceRouterParameters(stoppedWorkspace), - queries: workspaceQueries(stoppedWorkspace), + reactRouter: workspaceRouterParameters(autostopDisabledWorkspace), + queries: workspaceQueries(autostopDisabledWorkspace), }, beforeEach: () => { spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue( - stoppedWorkspace, + autostopDisabledWorkspace, + ); + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const body = within(document.body); + const user = userEvent.setup(); + const restartSpy = spyOn(API, "restartWorkspace"); + await user.click(await canvas.findByLabelText("Enable Autostop")); + await user.click(await canvas.findByRole("button", { name: /save/i })); + await body.findByText("Restart workspace?"); + await user.click(await body.findByRole("button", { name: /apply later/i })); + // The dialog closes without restarting or leaving the schedule page. + await waitFor(() => + expect(body.queryByText("Restart workspace?")).not.toBeInTheDocument(), + ); + expect(restartSpy).not.toHaveBeenCalled(); + await canvas.findByLabelText("Enable Autostop"); + }, +}; + +export const ChangingAutostopValueShowsRestartDialog: Story = { + parameters: { + reactRouter: workspaceRouterParameters(MockWorkspace), + queries: workspaceQueries(MockWorkspace), + }, + beforeEach: () => { + spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace); + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const body = within(document.body); + const user = userEvent.setup(); + const ttlInput = await canvas.findByLabelText( + "Time until shutdown (hours)", + ); + await user.clear(ttlInput); + await user.type(ttlInput, "4"); + await user.click(await canvas.findByRole("button", { name: /save/i })); + await body.findByText( + `Schedule for workspace "${MockWorkspace.name}" updated successfully.`, + ); + await body.findByText("Restart workspace?"); + }, +}; + +export const DisablingAutostopSkipsRestartDialog: Story = { + parameters: { + reactRouter: workspaceRouterParameters(MockWorkspace), + queries: workspaceQueries(MockWorkspace), + }, + beforeEach: () => { + spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace); + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const body = within(document.body); + const user = userEvent.setup(); + // MockWorkspace has autostop enabled, so clicking the toggle disables it. + await user.click(await canvas.findByLabelText("Enable Autostop")); + await user.click(await canvas.findByRole("button", { name: /save/i })); + await body.findByText( + `Schedule for workspace "${MockWorkspace.name}" updated successfully.`, + ); + expect(body.queryByText("Restart workspace?")).not.toBeInTheDocument(); + }, +}; + +const stoppedAutostopDisabledWorkspace: Workspace = { + ...autostopDisabledWorkspace, + latest_build: { ...MockWorkspaceBuild, status: "stopped" }, +}; + +export const EnablingAutostopWhileStoppedSkipsDialog: Story = { + parameters: { + reactRouter: workspaceRouterParameters(stoppedAutostopDisabledWorkspace), + queries: workspaceQueries(stoppedAutostopDisabledWorkspace), + }, + beforeEach: () => { + spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue( + stoppedAutostopDisabledWorkspace, ); }, play: async ({ canvasElement }) => { diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index 68c2ab0fd3..91fda3f556 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -140,9 +140,19 @@ const WorkspaceSchedulePage: FC = () => { await submitScheduleMutation.mutateAsync(data); + // A running build's autostop deadline is calculated when the + // build starts, so updating the TTL does not retroactively + // change it. Prompt the user to restart so the new value takes + // effect immediately, but only when all of the following hold: + // - autostop actually changed (toggled or new TTL value), + // - autostop is enabled after the change; disabling clears the + // running build's deadline server-side, so no restart is + // needed, and + // - the workspace is running; a stopped workspace picks up the + // new value on its next start. if ( data.autostopChanged && - getAutostop(workspace).autostopEnabled && + values.autostopEnabled && workspace.latest_build.status === "running" ) { setIsConfirmingApply(true); @@ -163,7 +173,9 @@ const WorkspaceSchedulePage: FC = () => { navigate(`/@${username}/${workspaceName}`); }} onClose={() => { - navigate(`/@${username}/${workspaceName}`); + // Keep the user on the schedule page; the saved value still + // applies on the next workspace start. + setIsConfirmingApply(false); }} />