From 91022c42ca47e6567930469ce09a576377c1444e Mon Sep 17 00:00:00 2001
From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com>
Date: Thu, 27 Aug 2026 12:12:37 +0200
Subject: [PATCH] fix(frontend): make the variable expiry field state its own
 status

The expiry field rendered a DateTimeInput whose time defaults to 12:00 when it
holds no value, so a variable with no expiry looked like one already configured,
and "cleared" looked identical to "never set". Setting an expiry is optional, so
the field has to say which it is.

Rebuild it on the shape ScriptAdvancedSettings already uses for Timeout, Cache and
Concurrency limits: a toggle whose checked state is the stored value being present,
seeding a date on enable rather than revealing an empty picker, with the input
under its own label. Drop `clearable` with it, so the toggle is the single way to
mean "no expiry" instead of competing with an X.

Stop the surrounding copy promising a run that may not happen. A handler is only
dispatched if the workspace has one configured and does not skip u/ paths, so the
list tooltip now states the date alone, and the form hedges both conditions,
naming the u/ mute only on personal paths where it can apply.

Also anchor the mute toggle's tooltip: Popover's trigger is a span around a
block-level label, which stretched to the full width of its SettingCard and left
the popup floating beside the card rather than under the control.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/lib/components/VariableForm.svelte    | 61 +++++++++++++------
 .../(root)/(logged)/variables/+page.svelte    |  3 +-
 .../(logged)/workspace_settings/+page.svelte  | 32 ++++++----
 3 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/frontend/src/lib/components/VariableForm.svelte b/frontend/src/lib/components/VariableForm.svelte
index 7064e6e359..af8bb01919 100644
--- a/frontend/src/lib/components/VariableForm.svelte
+++ b/frontend/src/lib/components/VariableForm.svelte
@@ -56,6 +56,17 @@
 
 	let ws = $derived(workspace ?? $workspaceStore)
 
+	// Enabling seeds a date rather than revealing an empty picker, mirroring how
+	// `ScriptAdvancedSettings` seeds a timeout of 300s. An empty DateTimeInput renders a time
+	// of 12:00 beside its buttons, which reads as a date already configured.
+	const DEFAULT_EXPIRY_DAYS = 90
+
+	function defaultValueExpiry(): string {
+		const d = new Date()
+		d.setDate(d.getDate() + DEFAULT_EXPIRY_DAYS)
+		return d.toISOString()
+	}
+
 	// Loading the deployed secret overwrites the draft row this form shares with the AI
 	// chat, so every path that would trigger it has to be blocked while that row stages a
 	// value — otherwise the staged one is replaced and the next deploy carries the old one.
@@ -211,26 +222,38 @@
 
 <Label
 	label="Value expiration"
-	tooltip="When the value stored here stops working, such as the expiry date of an API key. Windmill runs the workspace's variable expiration handler an hour before it so the value can be rotated. The variable itself is never deleted."
+	tooltip="When the value stored here stops working, such as the expiry date of an API key. An hour before it, Windmill runs the workspace's variable expiration handler if one is set, so the value can be rotated. The variable itself is never deleted."
 >
-	<!-- Unbound: DateTimeInput writes `null` on clear, and a `null` where the deployed
-	baseline has nothing reads as an edit, so every empty state has to normalize to
-	`undefined` before it reaches the draft row. -->
-	<DateTimeInput
-		value={valueExpiresAt}
-		clearable
-		useDropdown
+	<Toggle
+		size="sm"
+		checked={Boolean(valueExpiresAt)}
 		disabled={!can_write}
-		on:change={(e) => (valueExpiresAt = e.detail ?? undefined)}
-		on:clear={() => (valueExpiresAt = undefined)}
+		on:change={() => (valueExpiresAt = valueExpiresAt ? undefined : defaultValueExpiry())}
+		options={{ right: 'Set a date on which this value expires' }}
 	/>
-	<span class="text-2xs font-normal text-hint">
-		{#if valueExpiresAt && new Date(valueExpiresAt).getTime() <= Date.now()}
-			This date has passed. Set a new one once the value is rotated.
-		{:else}
-			Runs the workspace's <a href="{base}/workspace_settings?tab=variable_expiration"
-				>variable expiration handler</a
-			> an hour before this date. Without one, nothing happens.
-		{/if}
-	</span>
+	{#if valueExpiresAt}
+		<span class="text-xs font-semibold text-emphasis leading-none mt-2">Expires on</span>
+		<!-- Unbound: DateTimeInput writes `null` when its date is emptied, and a `null` where
+		the deployed baseline has nothing reads as an edit, so every empty state has to
+		normalize to `undefined` before it reaches the draft row. Not `clearable`: the toggle
+		above is the one way to mean "no expiry". -->
+		<DateTimeInput
+			value={valueExpiresAt}
+			useDropdown
+			disabled={!can_write}
+			on:change={(e) => (valueExpiresAt = e.detail ?? undefined)}
+		/>
+		<span class="text-2xs font-normal text-hint">
+			{#if new Date(valueExpiresAt).getTime() <= Date.now()}
+				This date has passed. Set a new one once the value is rotated.
+			{:else}
+				Runs the workspace's <a href="{base}/workspace_settings?tab=variable_expiration"
+					>variable expiration handler</a
+				>
+				an hour before this date, if the workspace has one{path.startsWith('u/')
+					? ' and does not skip u/ paths'
+					: ''}.
+			{/if}
+		</span>
+	{/if}
 </Label>
diff --git a/frontend/src/routes/(root)/(logged)/variables/+page.svelte b/frontend/src/routes/(root)/(logged)/variables/+page.svelte
index b51ddf886d..a85cbcafd2 100644
--- a/frontend/src/routes/(root)/(logged)/variables/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/variables/+page.svelte
@@ -488,8 +488,7 @@
 														{#snippet text()}
 															<div>
 																The value {expired ? 'expired' : 'expires'} on
-																{new Date(value_expires_at).toLocaleString()}. The workspace's
-																variable expiration handler runs an hour before that.
+																{new Date(value_expires_at).toLocaleString()}.
 															</div>
 														{/snippet}
 													</Popover>
diff --git a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
index 4301630ed5..d960019bbd 100644
--- a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
@@ -46,6 +46,7 @@
 
 	import PremiumInfo from '$lib/components/settings/PremiumInfo.svelte'
 	import Toggle from '$lib/components/Toggle.svelte'
+	import Popover from '$lib/components/Popover.svelte'
 
 	import ChangeWorkspaceName from '$lib/components/settings/ChangeWorkspaceName.svelte'
 	import ChangeWorkspaceId from '$lib/components/settings/ChangeWorkspaceId.svelte'
@@ -2160,22 +2161,31 @@ export async function main(
 									</div>
 
 									<SettingCard class="gap-2">
-										<Toggle
-											bind:checked={variableExpirationMutedOnUserPath}
-											disabled={!variableExpirationHasHandler}
-											options={{
-												right: 'Do not run the handler for variables in u/ paths',
-												rightTooltip: variableExpirationHasHandler
-													? undefined
-													: 'Select a handler first — this setting is stored with it.'
-											}}
-										/>
+										<!-- `w-fit`: the trigger is a flex item of the card, so it would otherwise
+										stretch the full width and anchor the popup away from the toggle. -->
+										<Popover
+											notClickable
+											disablePopup={variableExpirationHasHandler}
+											placement="bottom-start"
+											class="w-fit"
+										>
+											<Toggle
+												bind:checked={variableExpirationMutedOnUserPath}
+												disabled={!variableExpirationHasHandler}
+												options={{
+													right: 'Do not run the handler for variables in u/ paths'
+												}}
+											/>
+											{#snippet text()}
+												Select a handler first
+											{/snippet}
+										</Popover>
 									</SettingCard>
 
 									<Alert type="info" title="Runs as g/variable_expiration_handler">
 										The handler is executed by the automatically created group
 										g/variable_expiration_handler. Rotating a variable means writing to it, so grant
-										that group write access to the variables it is meant to rotate — it starts with
+										that group write access to the variables it is meant to rotate. It starts with
 										none. A script that carries an on-behalf-of identity is the exception: it runs
 										as that user, with their permissions rather than the group's.
 									</Alert>