feat(scheduler): support schedule definition history - #183
Conversation
yordis
commented
May 27, 2026
- Preserve schedule lifecycle decisions as replayable domain history without mixing delivery records into command-state streams.
- Keep recorder-assigned time and caller context on the event envelope so scheduler payloads remain focused on business facts.
PR SummaryMedium Risk Overview Architecture docs now state that Reviewed by Cursor Bugbot for commit 3e9f209. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reached
More reviews will be available in 48 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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 ignored due to path filters (1)
📒 Files selected for processing (11)
WalkthroughThis PR introduces a complete scheduler service protobuf schema suite, Buf v2 configuration for proto management, and a documentation clarification. It defines scheduling primitives (message templates, status states), four schedule trigger variants (at, every, cron, rrule), NATS delivery configuration with optional source sampling, and stream events for schedule creation and lifecycle transitions (paused, resumed, removed). ChangesScheduler Service Protobuf Schemas
Event Envelope Metadata Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Code Coverage SummaryDetailsDiff against mainResults for commit: 3e9f209 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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 3a6f203. Configure here.
4754e16 to
33fa5b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@proto/trogonai/scheduler/schedules/v1/message.proto`:
- Line 8: The Message.proto field Message.content should be changed from string
to bytes because the payload is transport-opaque/binary; update the field
declaration from "string content = 1 [features.field_presence =
LEGACY_REQUIRED];" to "bytes content = 1 [features.field_presence =
LEGACY_REQUIRED];" (preserving the field number and options), then regenerate
the protobuf artifacts and update any call sites that treated content as a UTF-8
string to use bytes/byte[]/Buffer as appropriate (e.g., places referencing
Message.content, serialization/deserialization helpers, and any validation
assuming UTF-8).
In `@proto/trogonai/scheduler/schedules/v1/schedule_added.proto`:
- Around line 5-8: The proto imports in
trogonai/scheduler/schedules/v1/schedule_added.proto (the lines importing
"trogonai/scheduler/schedules/v1/delivery.proto",
"trogonai/scheduler/schedules/v1/message.proto",
"trogonai/scheduler/schedules/v1/schedule.proto", and
"trogonai/scheduler/schedules/v1/schedule_status.proto") fail because Buf's
module root doesn't map "trogonai/..." to the repo's proto/ folder; add or
update your Buf configuration (e.g., create/update buf.yaml or buf.work.yaml) to
set the build root(s) to include "proto" (build.roots: ["proto"]) or otherwise
configure the module/import mapping so imports starting with "trogonai/..."
resolve to proto/trogonai/..., then re-run buf generate/compile.
In `@proto/trogonai/scheduler/schedules/v1/schedule_paused.proto`:
- Line 3: The proto package declaration in schedule_paused.proto ("package
trogonai.scheduler.schedules.v1;") does not match the file directory per Buf’s
PACKAGE_DIRECTORY_MATCH rule; fix by either updating the package declaration to
match the actual module root layout (e.g., adjust the package to reflect the
module root you want) or move the file into the directory that matches that
package, or alternatively update the Buf module root configuration so proto/ is
treated as the module root; locate this in
proto/trogonai/scheduler/schedules/v1/schedule_paused.proto and change the
package line or adjust Buf module configuration accordingly to restore
consistency.
In `@proto/trogonai/scheduler/schedules/v1/schedule.proto`:
- Around line 34-35: The timezone fields currently declare
[features.field_presence = LEGACY_REQUIRED] which enforces proto2-style required
presence and contradicts the intended semantics "empty omits the header"; update
the schema for the timezone fields (the `string timezone = 2` and the other
`timezone = 3` declaration) to use optional presence (remove the LEGACY_REQUIRED
option or replace it with [features.field_presence = OPTIONAL] / standard proto3
optional semantics) so an omitted/empty field can be omitted on the wire and
consumers/producers behave as documented.
🪄 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: 9db56a01-b1d3-4aa7-8a39-7529fa39a587
📒 Files selected for processing (9)
docs/architecture/event-metadata.mdproto/trogonai/scheduler/schedules/v1/delivery.protoproto/trogonai/scheduler/schedules/v1/message.protoproto/trogonai/scheduler/schedules/v1/schedule.protoproto/trogonai/scheduler/schedules/v1/schedule_added.protoproto/trogonai/scheduler/schedules/v1/schedule_paused.protoproto/trogonai/scheduler/schedules/v1/schedule_removed.protoproto/trogonai/scheduler/schedules/v1/schedule_resumed.protoproto/trogonai/scheduler/schedules/v1/schedule_status.proto
5ad1cd0 to
ad539d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
proto/trogonai/scheduler/schedules/v1/schedule.proto (1)
35-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve required-presence contradiction for timezone fields.
Line 35/Line 48 describe optional/empty timezone behavior, but Line 36 and Line 49 enforce
LEGACY_REQUIRED, which makes omission invalid on the wire.Suggested schema fix
- google.type.TimeZone timezone = 2 [features.field_presence = LEGACY_REQUIRED]; + google.type.TimeZone timezone = 2; ... - google.type.TimeZone timezone = 3 [features.field_presence = LEGACY_REQUIRED]; + google.type.TimeZone timezone = 3;In Protobuf Editions (edition 2024), does [features.field_presence = LEGACY_REQUIRED] on a singular message field require wire presence, and therefore conflict with comments that describe the field as optional/omittable?Also applies to: 48-49
🤖 Prompt for 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. In `@proto/trogonai/scheduler/schedules/v1/schedule.proto` around lines 35 - 37, The timezone field declaration uses "google.type.TimeZone timezone" but sets [features.field_presence = LEGACY_REQUIRED], which forces wire presence and contradicts the comment that the timezone is optional/omittable; remove the LEGACY_REQUIRED option (or replace it with no field_presence option) on the "google.type.TimeZone timezone" declarations so the timezone can be omitted, and make the same change for the second occurrence of "google.type.TimeZone timezone" in this proto to keep semantics consistent.
🤖 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 `@proto/trogonai/scheduler/schedules/v1/schedule_created.proto`:
- Around line 14-22: The persistence/rehydration mapping must be extended to
include the new proto type trogonai.scheduler.schedules.v1.ScheduleCreated so
created events are encoded/decoded and persisted like ScheduleAdded; in
rsworkspace/crates/trogon-decider-nats/src/stream_store.rs add the
ScheduleCreated type to the event-type/header assertions and to the dispatch +
codec tables used for encoding/decoding (the same spots where
trogonai.scheduler.schedules.v1.ScheduleAdded is registered), ensuring the
decode/encode handlers and any persistence keys or enum variants referencing
ScheduleAdded are duplicated/handled for ScheduleCreated so reads and writes
round-trip correctly.
---
Duplicate comments:
In `@proto/trogonai/scheduler/schedules/v1/schedule.proto`:
- Around line 35-37: The timezone field declaration uses "google.type.TimeZone
timezone" but sets [features.field_presence = LEGACY_REQUIRED], which forces
wire presence and contradicts the comment that the timezone is
optional/omittable; remove the LEGACY_REQUIRED option (or replace it with no
field_presence option) on the "google.type.TimeZone timezone" declarations so
the timezone can be omitted, and make the same change for the second occurrence
of "google.type.TimeZone timezone" in this proto to keep semantics consistent.
🪄 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: 4c6a1bbb-051d-4629-ac8f-ccbcb4a753e1
⛔ Files ignored due to path filters (1)
buf.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
buf.yamldocs/architecture/event-metadata.mdproto/trogonai/scheduler/schedules/v1/delivery.protoproto/trogonai/scheduler/schedules/v1/message.protoproto/trogonai/scheduler/schedules/v1/schedule.protoproto/trogonai/scheduler/schedules/v1/schedule_created.protoproto/trogonai/scheduler/schedules/v1/schedule_paused.protoproto/trogonai/scheduler/schedules/v1/schedule_removed.protoproto/trogonai/scheduler/schedules/v1/schedule_resumed.protoproto/trogonai/scheduler/schedules/v1/schedule_status.proto
✅ Files skipped from review due to trivial changes (1)
- docs/architecture/event-metadata.md
🚧 Files skipped from review as they are similar to previous changes (3)
- proto/trogonai/scheduler/schedules/v1/schedule_paused.proto
- proto/trogonai/scheduler/schedules/v1/schedule_removed.proto
- proto/trogonai/scheduler/schedules/v1/schedule_resumed.proto
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
ad539d4 to
3e9f209
Compare
