diff options
| author | Adam Malczewski <[email protected]> | 2026-06-01 11:34:40 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-01 11:34:40 +0900 |
| commit | 1210d6257a60ab859425557274b58d8c9ec3f2fa (patch) | |
| tree | 25042e334a8d705ff308e9094dbf9556165932b5 /packages/api/src | |
| parent | 64d2a8118535234212dd4b8dbb8b226fd5575607 (diff) | |
| download | dispatch-1210d6257a60ab859425557274b58d8c9ec3f2fa.tar.gz dispatch-1210d6257a60ab859425557274b58d8c9ec3f2fa.zip | |
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.
Diffstat (limited to 'packages/api/src')
| -rw-r--r-- | packages/api/src/routes/models.ts | 32 |
1 files changed, 26 insertions, 6 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( |
