[Bug]: keep.is_business_hours can never be true for a window that crosses midnight

[chiruu12]

The final check is start_hour <= hour < end_hour, so a window whose start is after its end is never satisfied:

is_business_hours(dt, start_hour=20, end_hour=8)
  22:00 Mon -> False    02:00 Tue -> False    12:00 Mon -> False

Validation accepts any hour in 0-23 for both, so an after-hours or on-call window is configured happily and then never fires. A workflow gated on it does nothing and reports nothing.

Two ways to read this, and they lead to different fixes:

1. A start after the end means the window wraps midnight. 20-8 is the night shift. This makes an existing config start working.
2. It is a configuration error and should raise, the way in_range refuses a reversed interval.

I lean towards the first, since the second still leaves no way to express a night window and the docstring does not say the window has to sit inside one day. Happy to write whichever you prefer.

[dosubot]

@chiruu12 — you've read the code correctly. The validation only rejects hours outside 0–23 [1], and the final check is a plain start_hour <= hour < end_hour [2] with no branch for the case where start_hour > end_hour, so a window like start_hour=20, end_hour=8 can never be satisfied.

The docstring states that start_hour/end_hour must be between 0 and 23, but says nothing requiring start_hour < end_hour or that the window must sit inside a single day [3]. The public docs describe the function simply as "Checks whether a given time falls within business hours" with no day-boundary caveat. Existing tests only exercise same-day windows (start_hour=9, end_hour=17; start_hour=0, end_hour=23) and none assert that a reversed range should raise [4].

Given all of that, option 1 — treating start_hour > end_hour as a wrap-around/overnight window (hour >= start_hour or hour < end_hour) — seems like the more useful fix, since rejecting it still leaves users with no way to express a night-shift or on-call window. Updating the docstring to explicitly call out the wrap-around semantics and adding tests for the boundary hours (at start_hour, just before end_hour, and mid-crossing) alongside the existing same-day cases would round out the change nicely.

To reply, just mention @dosu.

---

Share context across your team and agents. Try Dosu.