fix(api): wake-schedule toggle requires explicit action: 'on' | 'off'

Round-2 Gemini review surfaced that the toggle endpoint derived add-vs-
remove from its own in-memory state, which combined catastrophically
with any UI desync: a user clicking to turn ON an hour the UI showed as
off, but the server had as on, would silently get the hour turned OFF.
The clicks felt 'inverted' and the only recovery was a full reload.

Fix: require an explicit `action` field on every /toggle request. The
client must declare its intent; the server is no longer allowed to guess.

Idempotency rules:
- action: 'off' on an already-off hour → 200, no-op success.
- action: 'on' on an already-on hour → 200, REPLACES timestamps (so a
  recovering UI can re-assert the user's wall-clock intent without a
  delete-then-add round trip).
- Missing or invalid action → 400.

The 'off' path no longer reads or requires `timestamps`. The 'on' path
still requires all four slot timestamps as finite Unix-ms numbers (the
skewed-toggle relaxation from round 1 is preserved).

Tests:
- toggle() helper auto-derives action from `timestamps` presence, so
  the existing 12 tests stayed terse. One test that relied on the old
  'empty body = add' behavior now passes `action: 'on'` explicitly.
- Added 4 new contract tests:
  * rejects requests missing/with-invalid action
  * action='off' on an already-off hour is idempotent
  * action='on' on an already-on hour replaces timestamps (the
    round-2 desync-recovery scenario)
  * action='off' ignores stray timestamps payloads

29 / 29 routes tests pass; 431 / 431 across the workspace.

2 files changed, 113 insertions, 9 deletions

diff --git a/packages/api/src/routes/models.ts b/packages/api/src/routes/models.ts
index 5f47231..03c079a 100644
--- a/packages/api/src/routes/models.ts
+++ b/packages/api/src/routes/models.ts
@@ -901,6 +901,7 @@ function scheduleSnapshot(): {
 modelsRoutes.post("/wake-schedule/toggle", async (c) => {
 	const body = await c.req.json<{
 		hour?: unknown;
+		action?: unknown;
 		timestamps?: unknown;
 	}>();
 	const hour = body.hour;
@@ -911,13 +912,32 @@ modelsRoutes.post("/wake-schedule/toggle", async (c) => {
 		return c.json({ error: "hour must be an integer 0-23" }, 400);
 	}
 
-	if (wakeSchedule[hour] !== undefined) {
-		// Toggle off — remove all 4 slots for this hour.
-		deleteHour(wakeSchedule, hour);
+	// The action is the CLIENT'S DECLARED INTENT. Previously the server
+	// derived add-vs-remove from its own in-memory state, which meant a UI
+	// that had become stale (e.g. due to a snapshot race) would have its
+	// clicks silently inverted: user clicks to turn ON an hour the UI shows
+	// as OFF, server sees it as already-ON, deletes it. Requiring an explicit
+	// action makes the request idempotent and self-describing — a stale UI's
+	// click is now either a redundant no-op (action matches server state) or
+	// a recoverable replace (action="on" against an already-on hour just
+	// refreshes its timestamps to the new values).
+	const action = body.action;
+	if (action !== "on" && action !== "off") {
+		return c.json({ error: "action must be 'on' or 'off'" }, 400);
+	}
+
+	if (action === "off") {
+		// Idempotent: removing an already-removed hour is a no-op success.
+		if (wakeSchedule[hour] !== undefined) {
+			deleteHour(wakeSchedule, hour);
+		}
 	} else {
-		// Toggle on — require a `timestamps` object with one absolute Unix ms
-		// per probe slot (0, 15, 30, 45). The client is the source of truth
-		// for the *local* wall-clock intent of each probe.
+		// action === "on" — require a `timestamps` object with one absolute
+		// Unix ms per probe slot (0, 15, 30, 45). The client is the source
+		// of truth for the *local* wall-clock intent of each probe.
+		// Idempotent: turning ON an already-on hour replaces its timestamps
+		// (so a UI recovering from a desync can re-assert the correct wall-
+		// clock intent without first deleting).
 		const timestamps = body.timestamps;
 		if (timestamps === null || typeof timestamps !== "object") {
 			return c.json(
diff --git a/packages/api/tests/routes.test.ts b/packages/api/tests/routes.test.ts
index c44f473..f92b94c 100644
--- a/packages/api/tests/routes.test.ts
+++ b/packages/api/tests/routes.test.ts
@@ -410,11 +410,21 @@ describe("Wake schedule routes", () => {
 		};
 	}
 
+	/**
+	 * Auto-derives `action` from `timestamps` presence:
+	 *   - body has `timestamps` → action = "on"
+	 *   - body has no `timestamps` → action = "off"
+	 * Tests can override by passing `action` explicitly. This keeps the
+	 * intent-vs-state contract enforced (every request carries an explicit
+	 * action) while keeping the existing test bodies short.
+	 */
 	async function toggle(body: Record<string, unknown>) {
+		const withAction: Record<string, unknown> =
+			"action" in body ? body : { ...body, action: body.timestamps !== undefined ? "on" : "off" };
 		return app.request("/models/wake-schedule/toggle", {
 			method: "POST",
 			headers: { "Content-Type": "application/json" },
-			body: JSON.stringify(body),
+			body: JSON.stringify(withAction),
 		});
 	}
 
@@ -533,8 +543,11 @@ describe("Wake schedule routes", () => {
 		}
 	});
 
-	it("POST toggle rejects missing timestamps on add", async () => {
-		const res = await toggle({ hour: 8 });
+	it("POST toggle rejects action='on' with missing timestamps", async () => {
+		// Under the explicit-action contract, the helper would otherwise
+		// auto-derive { action: 'off' } for a body with no timestamps. Pass
+		// action explicitly so we exercise the on-without-timestamps reject.
+		const res = await toggle({ hour: 8, action: "on" });
 		expect(res.status).toBe(400);
 	});
 
@@ -616,4 +629,75 @@ describe("Wake schedule routes", () => {
 		await toggle({ hour: 1 });
 		await toggle({ hour: 3 });
 	});
+
+	it("POST toggle requires explicit action: 'on' | 'off' (Gemini round-2 #2)", async () => {
+		// The server must reject a request that omits `action`. This is the
+		// contract that closes the desync-causes-inverted-clicks failure mode:
+		// the server is no longer allowed to guess the user's intent from its
+		// own (possibly stale-relative-to-UI) in-memory state.
+		const now = Date.now();
+		const raw = await app.request("/models/wake-schedule/toggle", {
+			method: "POST",
+			headers: { "Content-Type": "application/json" },
+			body: JSON.stringify({ hour: 6, timestamps: buildTimestamps(now + 60_000) }),
+		});
+		expect(raw.status).toBe(400);
+
+		for (const bad of ["toggle", "ON", "OFF", "", true, 1, null]) {
+			const res = await app.request("/models/wake-schedule/toggle", {
+				method: "POST",
+				headers: { "Content-Type": "application/json" },
+				body: JSON.stringify({ hour: 6, action: bad, timestamps: buildTimestamps(now + 60_000) }),
+			});
+			expect(res.status, `bad action: ${JSON.stringify(bad)}`).toBe(400);
+		}
+	});
+
+	it("POST toggle action='off' is idempotent on an already-off hour (no error)", async () => {
+		// Stale UI scenario: the UI thinks hour 4 is on and clicks to turn it
+		// off, but server state had already deleted it. Must succeed quietly,
+		// not 400 or change anything else.
+		const before = await getSchedule();
+		expect(before.schedule["4"]).toBeUndefined();
+		const res = await toggle({ hour: 4 }); // helper auto-derives action='off'
+		expect(res.status).toBe(200);
+		const body = (await res.json()) as { schedule: Record<string, unknown> };
+		expect(body.schedule["4"]).toBeUndefined();
+	});
+
+	it("POST toggle action='on' on an already-on hour REPLACES timestamps (recovery from desync)", async () => {
+		// Stale UI scenario: the UI thinks hour 12 is off and clicks to turn
+		// it on, but server state had it already on (from a snapshot the UI
+		// missed). Old behavior would have INVERTED the click (turning it
+		// off); new behavior replaces the timestamps with the user's freshly
+		// computed wall-clock intent and keeps the hour on.
+		const base1 = Date.now() + 60 * 60 * 1000;
+		const base2 = base1 + 7 * 60 * 60 * 1000;
+		const addRes = await toggle({ hour: 12, timestamps: buildTimestamps(base1) });
+		expect(addRes.status).toBe(200);
+		const reAddRes = await toggle({ hour: 12, timestamps: buildTimestamps(base2) });
+		expect(reAddRes.status).toBe(200);
+		const body = (await reAddRes.json()) as {
+			schedule: Record<string, Record<string, number>>;
+		};
+		// Hour still present (NOT inverted to off), AND timestamps refreshed.
+		expect(body.schedule["12"]?.["0"]).toBe(base2);
+		expect(body.schedule["12"]?.["45"]).toBe(base2 + 180_000);
+		await toggle({ hour: 12 }); // cleanup
+	});
+
+	it("POST toggle action='off' ignores timestamps payload (off doesn't need them)", async () => {
+		// Accepting extra fields on an off request is fine; we only fail if
+		// an action='on' lacks timestamps.
+		const base = Date.now() + 60 * 60 * 1000;
+		await toggle({ hour: 13, timestamps: buildTimestamps(base) });
+		const res = await app.request("/models/wake-schedule/toggle", {
+			method: "POST",
+			headers: { "Content-Type": "application/json" },
+			body: JSON.stringify({ hour: 13, action: "off", timestamps: buildTimestamps(base) }),
+		});
+		expect(res.status).toBe(200);
+		const body = (await res.json()) as { schedule: Record<string, unknown> };
+		expect(body.schedule["13"]).toBeUndefined();
+	});
 });