Repository navigation
fix: reject re-enabling an automation whose one-time run has passed - #70
rudycelekli wants to merge 1 commit into
Conversation
Updating only enabled:true skipped the past-time check that creating or rescheduling applies, so a parked one-time automation was silently marked completed and left disabled while the request succeeded. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
rociiu
left a comment
rociiu
left a comment
There was a problem hiding this comment.
The fix looks right for the toggle path. Two smaller points inline. One more outside the diff:
The root cause is still reachable through other paths. The silent conversion to completed happens in arm() when computeNextRun returns null. rearmAll() (run after an import or a timezone change) can still enable a past one-time automation, and it then quietly flips to completed/disabled, the same symptom this PR describes. It may be worth surfacing this where arm() returns null, or validating on import, so every path is covered. A follow-up would be fine.
There was a problem hiding this comment.
When re-enabling, this parses the stored schedule again. If a stored row no longer matches the current schema (legacy or imported data, or a schema tightened later), the update throws a raw ZodError instead of a ValidationError, and turning the automation back on fails with an unclear error. arm() uses existing.schedule without parsing, so passing existing.schedule straight to computeNextRun (and parsing only patch.schedule) avoids this.
| if (patch.schedule !== undefined && (patch.enabled ?? existing.enabled)) { | ||
| const schedule = scheduleSpecSchema.parse(patch.schedule); | ||
| const enabling = patch.enabled === true && !existing.enabled; | ||
| if ((patch.schedule !== undefined || enabling) && (patch.enabled ?? existing.enabled)) { |
There was a problem hiding this comment.
Nit: when enabling is true, (patch.enabled ?? existing.enabled) is always true, and patch.schedule is parsed again in the repo.update call below. Computing the effective schedule once (patch.schedule ? scheduleSpecSchema.parse(patch.schedule) : existing.schedule) and reusing it for both the check and the update would make this easier to read.
Problem
AutomationService.updateonly checks that the next run is in the future whenscheduleis part of the patch. Re-enabling an automation with{ enabled: true }skips the check. For a parked one-time automation whose time has passed (which the existing "allowed while being disabled" test creates), the request succeeds with HTTP 200 but the automation is silently markedcompletedandenabled: false, so the toggle bounces back with no explanation. Creating or rescheduling the same schedule is rejected with "That time is in the past."Evidence (base)
New test in
apps/agent/test/automation-lifecycle.test.ts:After: that file and
test/api.test.tspass (28 tests).Change
In
update, the past-time check now also runs when the patch turns a disabled automation on, validating the existing schedule when none is supplied. Same error message andValidationErroras create/reschedule.Verified / not verified
Verified:
automation-lifecycleandapitests,tsc --noEmitforapps/agent. Not verified:biome lint(not installed in my minimal workspace install) and the web UI toast for the new error. In a fullapps/agentrun, one unrelated semantic-search test (search to fetch ...) timed out at 34s on my loaded machine; I did not check it on base.