Skip to content

feat(automations): fire trigger.schedule on a cron schedule (#3653)#3726

Merged
Yeraze merged 1 commit into
mainfrom
feat/automation-schedule
Jun 24, 2026
Merged

feat(automations): fire trigger.schedule on a cron schedule (#3653)#3726
Yeraze merged 1 commit into
mainfrom
feat/automation-schedule

Conversation

@Yeraze

@Yeraze Yeraze commented Jun 24, 2026

Copy link
Copy Markdown
Owner

First of the two documented post-#3721 follow-ups (AUTOMATION_ENGINE_PLAN.md §11).

Problem

trigger.schedule was a catalog-only stub — the engine never fired it. Automations with a schedule trigger could be built and dry-run in the Test panel, but never ran live.

Change

The engine now arms a croner job per enabled trigger.schedule automation inside load() (rescheduleCron()). Because reloadAutomations()load() runs after every CRUD, create/update/enable/disable/delete/reload all re-arm correctly (old jobs stopped first — no stale or duplicate jobs). Each job calls engine.onSchedule(id)buildScheduleContext → graph run, honoring the per-automation cooldown.

  • New buildScheduleContext (no mesh payload, no subject node).
  • The cron backing is injectable (EngineServiceOptions.cron: CronScheduler) — prod uses cronScheduler.ts (croner); tests use a fake to assert registration/cancellation without real timers.
  • Invalid/missing crons are skipped + logged, and rejected at save (validateForm via cron-validator, matching the rest of the app: { seconds:false, alias:true, allowBlankDay:true }).

Tests

Arm-one-job-per-automation + onSchedule fires the graph (directly and via the registered cron callback), invalid cron not armed, reload stops the old job and re-arms, cooldown honored, and buildScheduleContext shape. Full suite 7513 passed, 0 failures; tsc + build clean.

Follow-up still open

The second §11 item — smarter {{ }} token entry (highlight trigger/var tokens, flag unrecognized) — is a separate upcoming PR.

🤖 Generated with Claude Code

trigger.schedule was a catalog stub the engine never fired. The engine now
arms a croner job per enabled schedule automation in load() (rescheduleCron),
so create/update/enable/disable/reload all re-arm correctly (load() is called
from reloadAutomations after every CRUD). Each job calls
engine.onSchedule(id) → buildScheduleContext → graph run, honoring the
per-automation cooldown.

- New buildScheduleContext in triggerContext.ts (no mesh payload).
- CronScheduler is injectable (EngineServiceOptions.cron) so tests use a fake
  and assert registration/cancellation without real timers; prod uses croner.
- Invalid/missing crons are skipped + logged, and rejected at save
  (validateForm via cron-validator).

Tests: arm-per-automation + onSchedule fires the graph (incl. via the
registered cron callback), invalid cron not armed, reload stops old + re-arms,
cooldown honored, buildScheduleContext shape.

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

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: feat(automations): fire trigger.schedule on a cron schedule

Tasks:

  • Gather context and understand the PR
  • Read changed files
  • Analyze code quality, bugs, performance, security, test coverage
  • Post review findings

Summary

This is a clean, well-scoped implementation. The core design choices are sound and the tests are comprehensive. Below are a few observations worth considering, ordered by severity.


Potential Issue: Cron Validator Mismatch Between Frontend and Backend

File: src/components/automations/AutomationsPage.tsx:52 and src/server/utils/cronScheduler.ts:22-29

The frontend validates using cron-validator with { seconds: false, alias: true, allowBlankDay: true }, while the backend validates by actually constructing a croner Cron object. These two libraries may not agree on all edge cases — particularly around allowBlankDay and alias support.

A cron expression accepted by the frontend form could be rejected silently by the backend's validateCron() (logging a warning and skipping scheduling), or vice versa. Since rescheduleCron() at line 151 logs a warning and skips invalid crons rather than surfacing the error to the user, a user could save an automation that silently never fires.

Suggestion: Either add a server-side validation step in the save route that uses the same validateCron (croner) before persisting, or align both to use the same library. The current setup means "form accepted → saved → silently never arms" is a reachable state if the two validators diverge.


Minor: buildScheduleContext Passes null sourceId But Runs Can Have a Source

File: src/server/services/automation/automationEngineService.ts:178

await this.fireAutomation(a, buildScheduleContext(null, now), now);

The sourceId is hardcoded to null. Per the AUTOMATION_ENGINE_PLAN, schedule triggers have no ambient source — this is documented and correct. However, fireAutomation writes a run-log row with sourceId: ctx.sourceId (line 252), so all schedule runs will have null sourceId in automation_runs, which is the intended behavior.

This is fine as-is, just noting it's intentional per the plan. The action.sendMessage hint in the catalog (AutomationsPage.tsx:291) already calls this out to users. No change needed.


Minor: stop() Not Called on Server Shutdown

File: src/server/services/automation/automationEngineSingleton.ts

The singleton exports startAutomationEngine() but there's no corresponding shutdown path that calls engine.stop(). The newly added stop() method on the service correctly stops all cron jobs, but it appears to be unreachable in production — only accessible in tests.

This isn't a functional bug (Node exits clean the timers anyway), but on a graceful shutdown sequence croner jobs could fire briefly during drain. Worth exposing a stopAutomationEngine() from the singleton and hooking it into the server teardown if one exists.


Minor: Async Floating Promise in Cron Callback Is Caught but Re-entry Isn't Guarded

File: src/server/services/automation/automationEngineService.ts:155-157

const job = this.cron.schedule(cron, () => {
  this.onSchedule(a.id).catch((e) => logger.error(...));
});

The .catch() handles the error path correctly. However, if onSchedule takes longer than the cron interval (e.g., the graph run stalls due to a slow action), croner will fire the callback again while the previous invocation is still running. The cooldown in onSchedule will prevent a second fire from being processed (because lastFired is set before fireAutomation at line 177), so actions won't double-execute. This is actually a neat emergent behavior from the cooldown-first design — worth a comment noting the cooldown doubles as a re-entry guard.


Test Coverage: Good

The test suite covers the critical paths well:

  • Job registration per automation ✓
  • onSchedule fires the graph ✓
  • Invalid cron not armed ✓
  • Reload stops old job and re-arms ✓
  • Cooldown honored on onSchedule
  • buildScheduleContext shape ✓
  • The cb() → async call pattern (line 358-360) is a nice touch to verify the registered callback works end-to-end

One gap: there's no test for the stop() method itself (verifying that stop() actually halts the jobs). Minor given the fakeCron already tracks stopped state.


Nit: Duplicate JSDoc Block in triggerContext.ts

File: src/server/services/automation/triggerContext.ts:168-176

There are two consecutive /** JSDoc blocks before messageMatchesFilter — the first one (line 168-170) is a standalone comment "Tight pre-filter for trigger.message..." followed immediately by a second /** block (line 172-176) that has the actual @param doc. The first block will be silently dropped by TypeScript's doc tooling. Should be merged into one.


Verdict

The implementation is solid. The cron-validator mismatch is the one item worth addressing before merge to avoid a silent "saves but never fires" edge case. All other items are minor/nit. The injectable CronScheduler interface is a good pattern and keeps the service fully testable.


@Yeraze Yeraze merged commit 331d082 into main Jun 24, 2026
16 checks passed
@Yeraze Yeraze deleted the feat/automation-schedule branch June 25, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant