feat(scheduler): support occurrence lifecycle - #290
Conversation
yordis
commented
Jun 16, 2026
- Scheduler state needs durable occurrence progress so recurring schedules can recover and continue correctly across command boundaries.
- The aggregate needs explicit events for planned, recorded, and exhausted occurrences so delivery timing remains auditable and deterministic.
PR SummaryMedium Risk Overview Protos: Domain/commands: New State evolution: Occurrence events update progress and pending; pause keeps pending for race-safe recording; resume clears unrecorded pending so re-arming works; create resets occurrence fields. Execution reconciliation treats occurrence lifecycle events as non-reconciled (no delivery-definition changes). Reviewed by Cursor Bugbot for commit fe46982. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reached
More reviews will be available in 52 minutes and 41 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds schedule occurrence lifecycle support to the scheduler: three new proto messages ( ChangesSchedule Occurrence Tracking and Recurrence Commands
Sequence Diagram(s)sequenceDiagram
participant Processor
participant ScheduleNextOccurrenceDecider
participant Recurrence
participant StateEvolve
rect rgba(100, 149, 237, 0.5)
note over Processor,StateEvolve: Arm next occurrence
Processor->>ScheduleNextOccurrenceDecider: decide(state, ScheduleNextOccurrence{id, now})
ScheduleNextOccurrenceDecider->>ScheduleNextOccurrenceDecider: validate state value, check pending_occurrence_at, check completed
ScheduleNextOccurrenceDecider->>Recurrence: TryFrom(stored schedule definition)
Recurrence-->>ScheduleNextOccurrenceDecider: Recurrence or RecurrenceError
ScheduleNextOccurrenceDecider->>Recurrence: plan_next(at_or_after last_occurrence_at - grace)
Recurrence-->>ScheduleNextOccurrenceDecider: Occurrence{at} or Exhausted
alt Occurrence found
ScheduleNextOccurrenceDecider-->>Processor: emit ScheduleOccurrenceScheduled{sequence++, occurrence_at, scheduled_at=now}
else Exhausted
ScheduleNextOccurrenceDecider-->>Processor: emit ScheduleCompleted{last_occurrence_sequence}
end
Processor->>StateEvolve: evolve(state, emitted event)
StateEvolve-->>Processor: updated State{pending_occurrence_at set or completed=true}
end
rect rgba(144, 238, 144, 0.5)
note over Processor,StateEvolve: Record occurrence
Processor->>ScheduleNextOccurrenceDecider: decide(state, RecordScheduleOccurrence{id, occurrence_at, recorded_at})
ScheduleNextOccurrenceDecider->>ScheduleNextOccurrenceDecider: guard duplicate, enforce pending match, advance sequence
ScheduleNextOccurrenceDecider-->>Processor: emit ScheduleOccurrenceRecorded
ScheduleNextOccurrenceDecider->>Recurrence: plan_next(after occurrence_at)
Recurrence-->>ScheduleNextOccurrenceDecider: Occurrence or Exhausted
ScheduleNextOccurrenceDecider-->>Processor: emit ScheduleOccurrenceScheduled or ScheduleCompleted
Processor->>StateEvolve: evolve(state, events)
StateEvolve-->>Processor: updated State{last_occurrence advanced, pending cleared or set}
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
a81b957 to
74a107e
Compare
Code Coverage SummaryDetailsDiff against mainResults for commit: fe46982 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
74a107e to
fac5e38
Compare
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
fac5e38 to
da2e793
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rsworkspace/crates/trogon-scheduler/src/commands/rrule.rs`:
- Around line 88-93: The RRuleCursor::AtOrAfter case on line 91 incorrectly
skips boundary occurrences because set.after() has exclusive semantics and only
returns dates strictly after the provided instant. Fix the AtOrAfter branch by
checking if the cursor instant itself is a valid occurrence first before falling
back to the next candidate from the after() call. This ensures that AtOrAfter
includes an occurrence exactly at the cursor instant, not just after it.
- Around line 45-47: The Expansion variant in the error enum is storing a String
message instead of preserving the typed error from the rrule crate, which
discards error context. Refactor the Expansion variant to store the actual
source error as a typed field (matching the pattern used by the Timestamp
variant) with the #[source] attribute, rather than converting it to a String via
to_string(). Update the error display message to reference the source error
appropriately, and modify the code that constructs this error to pass the typed
error directly instead of converting it to a String.
In
`@rsworkspace/crates/trogon-scheduler/src/commands/schedule_next_occurrence.rs`:
- Around line 105-127: Replace the saturating_add(1) call on last_sequence with
checked arithmetic that returns an error when overflow would occur, rather than
silently saturating to u64::MAX. When calling schedule_or_complete_event around
line 126, use checked_add instead of saturating_add and map any overflow
condition to a ScheduleNextOccurrenceError variant (you may need to add a new
error variant if one doesn't exist for sequence overflow). This ensures that
duplicate occurrence sequences cannot be emitted when the sequence number
reaches its maximum value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b569e2cd-4913-4c2a-b04e-d240bed3b7d9
⛔ Files ignored due to path filters (13)
rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.state.v1.state.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.state.v1.state.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.events.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.events.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.events.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.events.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.mod.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_completed.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_occurrence_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_occurrence_recorded.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_occurrence_scheduled.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.schedule_occurrence_scheduled.rsis excluded by!**/gen/**
📒 Files selected for processing (18)
proto/trogonai/scheduler/schedules/state/v1/state.protoproto/trogonai/scheduler/schedules/v1/events.protoproto/trogonai/scheduler/schedules/v1/schedule_completed.protoproto/trogonai/scheduler/schedules/v1/schedule_occurrence_recorded.protoproto/trogonai/scheduler/schedules/v1/schedule_occurrence_scheduled.protorsworkspace/crates/trogon-scheduler/src/commands/create_schedule.rsrsworkspace/crates/trogon-scheduler/src/commands/domain/mod.rsrsworkspace/crates/trogon-scheduler/src/commands/domain/schedule_occurrence_sequence.rsrsworkspace/crates/trogon-scheduler/src/commands/mod.rsrsworkspace/crates/trogon-scheduler/src/commands/pause_schedule.rsrsworkspace/crates/trogon-scheduler/src/commands/record_schedule_occurrence.rsrsworkspace/crates/trogon-scheduler/src/commands/remove_schedule.rsrsworkspace/crates/trogon-scheduler/src/commands/resume_schedule.rsrsworkspace/crates/trogon-scheduler/src/commands/rrule.rsrsworkspace/crates/trogon-scheduler/src/commands/schedule_next_occurrence.rsrsworkspace/crates/trogon-scheduler/src/commands/state.rsrsworkspace/crates/trogon-scheduler/src/processor/execution/reconciliation/recorded_events.rsrsworkspace/crates/trogonai-proto/src/scheduler/schedules/codec.rs
Boundary occurrences, durable sequence integrity, and recurrence error context must survive so armed wakeups and gapless sequencing stay trustworthy. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
A finished recurrence must stay finished: without a durable completion marker an idle re-arm could append duplicate completion events and resurrect planning. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
A finished recurrence is finished regardless of pause state; deferring completion until resume would leave a done schedule durably marked incomplete. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c8afffc. Configure here.
Recurrence is domain: keeping expansion and the next-step decision in the aggregate, with proto event translation confined to the command boundary, keeps the deciders thin and the domain free of wire types. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
An exhausted schedule is terminal until re-created; resuming it would re-enable delivery downstream for a schedule that can never fire again. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
