fix: scope reminder test clock to reminder service

[ReubenBond]

ReminderTestClock replaced the unkeyed TimeProvider, so Orleans' keyed fallback routed unrelated silo background timers through FakeTimeProvider. Advancing reminder time could therefore synchronously execute membership, health, and other timer callbacks and hang functional reminder tests.

Register the fake provider specifically for ReminderTimeProviderNames.Reminders and verify that unkeyed silo time remains TimeProvider.System.

The scoped clock also makes table refreshes deterministic at exact reminder cadence boundaries. Load table entries only when their next tick is in the future and within the loading window, deliberately skipping an exact-due table load instead of reloading an occurrence which just fired. The controlled stale-read tests use one reminder owner so their blocked table read and direct local mutation share the sequence whose ordering they validate.

[Copilot]

Pull request overview

Scopes the reminders test-time clock to the reminders subsystem’s keyed TimeProvider so that advancing reminder time does not inadvertently advance unrelated silo background timers (membership/health/etc.), preventing functional reminder test hangs.

Changes:

• Register ReminderTestClock’s FakeTimeProvider as a keyed TimeProvider under ReminderTimeProviderNames.Reminders instead of replacing the unkeyed TimeProvider.
• Update ReminderTestClock documentation to reflect keyed scoping and reduced side effects.
• Add a functional test asserting the unkeyed silo TimeProvider remains TimeProvider.System while the reminders provider is separately keyed.

Show a summary per file

File	Description
test/Orleans.Reminders.Tests/TimerTests/ReminderTests_TableGrain.cs	Adds coverage verifying reminder time is keyed/scoped and the unkeyed silo clock remains TimeProvider.System.
src/Orleans.Testing.Reminders/ReminderTestClock.cs	Changes the test clock registration to a reminders-specific keyed TimeProvider and updates remarks accordingly.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

Suppressed comments (1)

src/Orleans.Reminders/ReminderService/LocalReminderService.cs:889

• The new IsReminderWithinLoadingWindow(..., DateTime? nextTickTime) overload skips entry/Period validation when nextTickTime is provided (because CalculateNextTickTime is bypassed). This can allow invalid reminder entries to pass silently in the keyed-path calls. Consider validating entry and entry.Period regardless of whether nextTickTime is supplied.

            return (nextTickTime ?? CalculateNextTickTime(entry, now)) <= now.AddClamped(loadingWindow);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

Suppressed comments (2)

src/Orleans.Reminders/ReminderService/LocalReminderService.cs:598

• With shouldLoad currently depending on nextTick > now, tombstones are treated specially at exact cadence boundaries. If shouldLoad is made inclusive (to allow due-now loads), this condition should instead explicitly guard tombstones using nextTick <= now to prevent resurrecting an occurrence which just fired.

            if (!isWithinLoadingWindow || (state is LocalReminderState.Tombstone && !shouldLoad))

src/Orleans.Reminders/ReminderService/LocalReminderService.cs:567

• shouldLoad is false when CalculateNextTickTime returns now (exact cadence boundary). For entries which are not yet in localReminders, this means the service will skip loading them from storage even though they are within the loading window, delaying delivery until a later refresh. If the intent is only to avoid resurrecting tombstones at exact boundaries, consider allowing due-now loads here and handling the tombstone case separately.

This issue also appears on line 598 of the same file.

            var nextTick = CalculateNextTickTime(entry, now);
            // Keep distant schedules in storage, and skip exact-due table loads to simplify fake-time tests.
            var isWithinLoadingWindow = nextTick <= now.AddClamped(reminderOptions.ReminderLoadingWindow);
            var shouldLoad = nextTick > now && isWithinLoadingWindow;

• Files reviewed: 3/3 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 1

[ReubenBond]

This is an intentional trade-off for deterministic fake-time behavior: a table entry whose next occurrence is exactly now is skipped, so it will resume at the following cadence boundary. I expanded the inline comment to make that semantic caveat explicit.

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 0 new