fix: record the enabled state a schedule write leaves, not the one sent

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015vn1jCHUcAdaG9C4NsUehF

frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte

@@ -55,9 +55,9 @@
 	import { onUserInput } from '$lib/userDraftEditGate'
 	import { isTemporaryPath, newItemPath, useItem, type ItemAdapter } from '$lib/itemStore.svelte'
 	import {
-		draftOnlyScheduleCfg,
 		newScheduleCfg,
 		normalizeScheduleCfg,
+		scheduleCfgAfterWrite,
 		scheduleCfgOf,
 		scheduleFormOf,
 		type NewScheduleOptions,
@@ -198,7 +198,7 @@
 					: undefined
 				return {
 					deployed: no_deployed ? undefined : normalizeScheduleCfg(deployedSchedule),
-					draft: no_deployed && loadedDraft ? draftOnlyScheduleCfg(loadedDraft) : loadedDraft,
+					draft: loadedDraft,
 					draftSavedAt: draft_saved_at
 				}
 			} catch (err) {
@@ -207,11 +207,9 @@
 			}
 		},
 		async write({ workspace, path, value, deployed }) {
-			await writeScheduleCfg(
-				value,
-				deployed !== undefined || standsForDeployed.has(path),
-				workspace
-			)
+			const update = deployed !== undefined || standsForDeployed.has(path)
+			await writeScheduleCfg(value, update, workspace)
+			return scheduleCfgAfterWrite(value, update, deployed)
 		}
 	}
 

frontend/src/lib/components/triggers/schedules/scheduleCfg.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'vitest'
 import {
-	draftOnlyScheduleCfg,
 	normalizeScheduleCfg,
+	scheduleCfgAfterWrite,
 	scheduleCfgOf,
 	scheduleFormOf
 } from './scheduleCfg'
@@ -43,15 +43,18 @@ describe('schedule config normalization', () => {
 		expect(once).not.toHaveProperty('edited_by')
 	})
 
-	// Deploying a draft-only schedule creates it, and a create always enables it. A draft that
-	// kept its stored state would show a disabled schedule the server has enabled, and later
-	// updates omit the field, so nothing would ever correct it.
-	it('records a draft-only schedule as enabled, however it was stored', () => {
-		expect(
-			draftOnlyScheduleCfg(normalizeScheduleCfg({ ...deployed, enabled: false }))
-		).toMatchObject({ enabled: true })
+	// Neither endpoint takes `enabled` from the config, and the item records this as its deployed
+	// side: a disabled config recorded as sent would show a running schedule as disabled for good.
+	it('records the enabled state a write leaves on the server, not the one sent', () => {
+		const disabled = normalizeScheduleCfg({ ...deployed, enabled: false })
+		const created = scheduleCfgAfterWrite(disabled, false, undefined)
+		// Serialized like the form's own config, since saves compare them exactly.
+		expect(JSON.stringify(created)).toBe(JSON.stringify(normalizeScheduleCfg(deployed)))
 		const { enabled: _dropped, ...withoutEnabled } = deployed
-		expect(draftOnlyScheduleCfg(normalizeScheduleCfg(withoutEnabled))).toMatchObject({
+		expect(
+			scheduleCfgAfterWrite(normalizeScheduleCfg(withoutEnabled), false, undefined)
+		).toMatchObject({ enabled: true })
+		expect(scheduleCfgAfterWrite(disabled, true, normalizeScheduleCfg(deployed))).toMatchObject({
 			enabled: true
 		})
 	})

frontend/src/lib/components/triggers/schedules/scheduleCfg.ts

@@ -174,13 +174,18 @@ export function normalizeScheduleCfg(raw: ScheduleCfg): ScheduleCfg {
 }
 
 /**
- * A schedule that exists only as a draft, as deploying it will leave it: deploying one creates
- * it, and a create always enables it (`writeScheduleCfg`), so the draft has to carry the state
- * that create produces. Left as stored, a disabled draft would record `enabled: false` against
- * a server that says otherwise, and later updates omit the field, so nothing corrects it.
+ * What the server holds once `writeScheduleCfg` has written `cfg`. Neither endpoint takes
+ * `enabled` from it (a create enables the schedule, an update leaves the field as it was), and
+ * a later update cannot correct it, so recording what was sent could show a running schedule as
+ * disabled for good. `deployed` is the schedule an update was measured against, if known.
  */
-export function draftOnlyScheduleCfg(cfg: ScheduleCfg): ScheduleCfg {
-	return { ...cfg, enabled: true }
+export function scheduleCfgAfterWrite(
+	cfg: ScheduleCfg,
+	update: boolean,
+	deployed: ScheduleCfg | undefined
+): ScheduleCfg {
+	const enabled = !update ? true : deployed ? deployed.enabled : cfg.enabled
+	return normalizeScheduleCfg({ ...cfg, enabled })
 }
 
 /** The handlers a new schedule starts with, from the workspace's defaults. */
@@ -263,8 +268,7 @@ export async function newScheduleCfg(opts: NewScheduleOptions): Promise<Schedule
 		cron_version: s?.cron_version ?? 'v2',
 		timezone: s?.timezone ?? Intl.DateTimeFormat().resolvedOptions().timeZone,
 		paused_until: s?.paused_until ?? undefined,
-		// A create always enables the schedule (`writeScheduleCfg`), so that is where a new one
-		// starts: the deployed side a create records is the value it sent.
+		// A create always enables the schedule (`writeScheduleCfg`), so that is where a new one starts.
 		enabled: true,
 		summary: s?.summary ?? '',
 		labels: s?.labels ?? undefined,

frontend/src/lib/itemStore.svelte.ts

@@ -58,7 +58,10 @@ export type ItemWriteContext<V> = ItemKey & {
 
 export type ItemAdapter<V> = {
 	load?: (key: ItemKey) => Promise<ItemLoad<V>>
-	write?: (ctx: ItemWriteContext<V>) => Promise<void>
+	/** Resolves to what the server holds after the write, when that is not `value`: an endpoint
+	 *  that sets a field on its own (a schedule create enabling it) would otherwise be recorded
+	 *  as holding what was sent. */
+	write?: (ctx: ItemWriteContext<V>) => Promise<V | void>
 	/** Where a save puts `value`. Defaults to its `path` field. */
 	pathOf?: (value: V) => string
 	/**
@@ -351,20 +354,23 @@ class Entry<V> {
 				return { ok: true, path: from.path, moved: false }
 			}
 			const to = (adapter.pathOf ?? ((v: V) => (v as { path: string }).path))(sent)
+			let held: V
 			try {
 				if (!adapter.write) throw new Error('This item cannot be saved')
-				await adapter.write({
-					...from,
-					value: sent,
-					deployed: this.origin === 'deployed' ? snapshot(this.deployed) : undefined,
-					meta: this.meta
-				})
+				held =
+					(await adapter.write({
+						...from,
+						value: sent,
+						deployed: this.origin === 'deployed' ? snapshot(this.deployed) : undefined,
+						meta: this.meta
+					})) ?? sent
 			} catch (e) {
 				this.error = errorMessage(e)
 				return { ok: false, error: this.error }
 			}
 			this.error = undefined
-			this.deployed = { ...sent, ...this.patched } as V
+			if (held !== sent) this.adopt(sent, held)
+			this.deployed = { ...held, ...this.patched } as V
 			this.origin = 'deployed'
 			this.template = undefined
 			const moved = to !== from.path
@@ -379,6 +385,22 @@ class Entry<V> {
 		return result
 	}
 
+	/** Give the value each field the server set otherwise than `sent` asked, unless the user has
+	 *  changed that field since: left as sent, it would read as a draft of a change nobody made. */
+	private adopt(sent: V, held: V): void {
+		if (this.value === undefined) return
+		const s = sent as Record<string, unknown>
+		const h = held as Record<string, unknown>
+		const next = snapshot(this.value) as Record<string, unknown>
+		let changed = false
+		for (const k of new Set([...Object.keys(s), ...Object.keys(h)])) {
+			if (deepEqual(s[k], h[k]) || !deepEqual(next[k], s[k])) continue
+			next[k] = h[k]
+			changed = true
+		}
+		if (changed) this.replaceValue(next as V)
+	}
+
 	discard(): Promise<DiscardOutcome> {
 		return this.run(async () => {
 			if (!this.loaded) return { removed: false }

frontend/src/lib/itemStore.test.ts

@@ -308,6 +308,35 @@ describe('item store: commands', () => {
 		expect(await item.save()).toMatchObject({ ok: true })
 		expect(writes).toEqual([{ path: 's', permissioned_as: 'u/b' }])
 	})
+
+	it('records what a write left on the server, and gives the value the fields it set', async () => {
+		type Sched = { path: string; enabled: boolean; summary: string }
+		const rows = fakeRows()
+		const store = createItemStore(rows.port)
+		const gate = deferred()
+		const { handle: item } = store.acquire(
+			{ workspace: 'w', kind: 'trigger_schedule', path: 's' },
+			{ workspace: 'w', path: 's' },
+			{
+				load: async () => ({ draft: { path: 's', enabled: false, summary: '' } }),
+				write: async (ctx) => {
+					await gate.promise
+					return { ...ctx.value, enabled: true }
+				}
+			} as ItemAdapter<Sched>
+		)
+		await settle()
+
+		const saving = item.save()
+		item.value = { path: 's', enabled: false, summary: 'typed during the save' }
+		gate.resolve()
+		expect(await saving).toMatchObject({ ok: true })
+
+		const kept = { path: 's', enabled: true, summary: 'typed during the save' }
+		expect(item.deployed).toEqual({ path: 's', enabled: true, summary: '' })
+		expect(item.value).toEqual(kept)
+		expect(rows.writes.at(-1)).toEqual({ path: 's', value: kept })
+	})
 })
 
 describe('item store: outside writes', () => {