Advanced date override handling and timezone compatibility improvements#5
Advanced date override handling and timezone compatibility improvements#5Frawatson wants to merge 1 commit into
Conversation
* fix date override for fixed round robin + time zone in date override * check if slot is within working hours of fixed hosts * add test for date override in different time zone * fix date overrides for not fixed hosts (round robin) * code clean up * fix added test * use the correct timezone of user for date overrides --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
🤖 Grapple PR ReviewSummaryThis PR modifies 4 files. Title: "Advanced date override handling and timezone compatibility improvements" Changes: 4 files (+111 / -10 lines)
Resolves: #7 (feat: convert InsightsBookingService to use Prisma.sql raw queries) CI: ⏳ 0 passed, 0 failed Impact Diagramgraph TD
pr_0[/"🔀 PR Changes: 4 files"/]
f1["🟡 getSchedule.test.ts +18/-0"]
pr_0 --> f1
f2["🟡 slots.ts +16/-5"]
pr_0 --> f2
f3["🟡 slots.ts +76/-5"]
pr_0 --> f3
f4["🟡 schedule.d.ts +1/-0"]
pr_0 --> f4
style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f3 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f4 fill:#7c3aed,color:#fff,stroke:#7c3aed
Review details
Powered by Grapple PR — AI that understands your code. |
![grapple-pr[bot]](https://avatars.githubusercontent.com/in/3205222?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Grapple PR Review
Summary
This PR modifies date override handling and timezone compatibility in the scheduling system, specifically adjusting how date overrides are calculated relative to organizer and invitee timezones, and adding date override awareness to the checkIfIsAvailable function in the slots router.
Changes: 4 files (+111 / -10 lines)
- 🟡 4 files modified:
apps/web/test/lib/getSchedule.test.ts,packages/lib/slots.ts,packages/trpc/server/routers/viewer/slots.ts,packages/types/schedule.d.ts
Resolves: #7 (feat: convert InsightsBookingService to use Prisma.sql raw queries)
CI: ⏳ 0 passed, 0 failed
Impact Diagram
graph TD
pr_0[/"🔀 PR Changes: 4 files"/]
f1["🟡 getSchedule.test.ts +18/-0"]
pr_0 --> f1
f2["🟡 slots.ts +16/-5"]
pr_0 --> f2
f3["🟡 slots.ts +76/-5"]
pr_0 --> f3
f4["🟡 schedule.d.ts +1/-0"]
pr_0 --> f4
style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f3 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f4 fill:#7c3aed,color:#fff,stroke:#7c3aed
2 critical issues found — please address before merging.
| Severity | Count |
|---|---|
| 🔴 Critical | 2 |
| 🟠 Major | 4 |
| 🟡 Minor | 3 |
| 🔵 Info | 2 |
Review details
- Reviewed at: 2026-04-07T18:34:41.515Z
- Agents used: security, logic, style, architecture, performance, verification
- Context level: full
- Execution time: 99.3s
- Low-confidence findings suppressed: 2
- Blast radius: 0 (contained)
Powered by Grapple PR — AI that understands your code.
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
🔴 CRITICAL — Reference Equality Bug (confidence: 100%)
Day-off detection uses === to compare two different Dayjs objects, which compares by reference, not by value. This will always evaluate to false, meaning day-off date overrides (where start equals end) will never be detected. Slots on a day-off will incorrectly be shown as available.
Evidence:
- dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes') creates two distinct Dayjs objects; JavaScript
===compares object references, not values - The intent specification explicitly calls this out: 'A day-off date override (start === end) correctly causes the slot to be unavailable'
- The edge cases section confirms: 'uses reference equality between two different Dayjs objects, which will always be false — day-off overrides will never be detected correctly'
Agent: logic
| } | ||
|
|
||
| if (dateOverrideExist) { | ||
| return true; |
There was a problem hiding this comment.
🔴 CRITICAL — Copy-Paste Bug (confidence: 100%)
The end variable in the working hours check is computed using slotStartTime instead of slotEndTime. Both start and end will always have the same value, making the end-boundary check against workingHour.endTime incorrect. A slot that extends past working hours will not be correctly rejected.
Evidence:
- Line 133:
const start = slotStartTime.hour() * 60 + slotStartTime.minute(); - Line 134:
const end = slotStartTime.hour() * 60 + slotStartTime.minute();— should useslotEndTime - The intent specification edge cases explicitly note: '
endshould useslotEndTime, notslotStartTime'
Agent: logic
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟠 MAJOR — Incorrect Availability Logic (confidence: 100%)
When dateOverrideExist is true (an override exists for this date and the slot falls within it), the function immediately returns true without checking busy times. This means a slot that overlaps with an existing booking will still be shown as available if it falls within a date override window.
Evidence:
- The intent specification states: 'If a date override exists and the slot falls within it, availability should be determined solely by busy times without further working-hours checks'
- Lines 125-126 return
trueunconditionally, skipping thebusy.every(...)check that follows - The busy time check on lines 141-155 is never reached when a date override exists and the slot is within the override window
Agent: logic
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( | ||
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { |
There was a problem hiding this comment.
🟠 MAJOR — Missing Working Hours Check for Non-Override Days (confidence: 100%)
The working hours check only rejects a slot if workingHours.find() returns a matching working hour entry where the slot is outside bounds. If a slot falls on a day that has no matching working hour entry at all (i.e., the organizer doesn't work that day of the week), no working hour object is found, find returns undefined (falsy), and the slot is NOT rejected. This means slots on non-working days will pass through to the busy-time check and may be shown as available.
Evidence:
- The
findcallback only enters the comparison block whenworkingHour.days.includes(slotStartTime.day())— if no working hour includes this day,findreturns undefined - The
if (workingHours.find(...))block only returns false when find returns truthy, meaning 'no matching working hours at all' is treated the same as 'slot is within working hours' - The intent says: 'A slot outside working hours should be marked unavailable (return false)'
Agent: logic
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; |
There was a problem hiding this comment.
🟠 MAJOR — Inconsistent UTC Offset Application (confidence: 100%)
In checkIfIsAvailable, the UTC offset is negated (* -1) when computing date override boundaries, but in getSlots (packages/lib/slots.ts), the offset is not negated — it uses inviteeUtcOffset - organizerUtcOffset directly. These two code paths handle the same date override data but apply offsets in opposite directions, which will produce inconsistent slot boundaries between slot generation and availability checking.
Evidence:
- slots.ts line 211-213:
offset = inviteeUtcOffset - organizerUtcOffset(no negation) - slots.ts line 103:
utcOffset = dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1(negated, and only organizer offset, no invitee offset) - The intent specification flags this: 'The
utcOffsetis negated (* -1) insidecheckIfIsAvailablebut not negated insidegetSlots; verify this is intentional and consistent across both code paths' - These two functions handle the same date overrides for different purposes (slot generation vs availability checking) and must agree on time boundaries
Agent: logic
| ) { | ||
| return true; | ||
| } | ||
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { |
There was a problem hiding this comment.
🟡 MINOR — Multiple Date Overrides (confidence: 100%)
Using dateOverrides.find() means only the first matching date override for a given date is evaluated. If a user has multiple overlapping date overrides on the same date, subsequent overrides are ignored, which could lead to incorrect availability.
Evidence:
- Line 101:
dateOverrides.find((date) => { ... })— returns on first match - The edge cases section notes: 'Multiple date overrides on the same date:
dateOverrides.find()returns on the first match; if a user has overlapping overrides, only the first is evaluated'
Agent: logic
| if ( | ||
| dateOverrides.find((date) => { | ||
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | ||
|
|
There was a problem hiding this comment.
🟡 MINOR — Code Organization (confidence: 95%)
Day-off date override detection uses reference equality (===) between two different Dayjs objects, which will always be false
Evidence:
- Line 106:
if (dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes'))— creates two separate Dayjs instances - Reference equality checks (===) on Dayjs objects will always be false even if they represent the same time
- Day-off overrides (where start === end) will never be detected correctly and slots will not be marked unavailable
Agent: style
| ) { | ||
| // slot is not within the date override | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🟡 MINOR — Day-of-Week Timezone Mismatch (confidence: 93%)
The working hours check uses slotStartTime.day() which returns the UTC day of week (since slotStartTime = time.utc()). Working hours are typically defined in the organizer's local timezone, where the day of the week may differ from UTC near midnight boundaries. This can cause slots near midnight UTC to be checked against the wrong day's working hours.
Evidence:
- Line 100:
const slotStartTime = time.utc();— day() returns UTC day - Working hours
daysarray is typically based on the organizer's local day of week - The edge cases section notes: '
slotStartTime.day()(0-6 UTC day of week) may differ from the organizer's local day of week — could cause incorrect working-hours matching near midnight'
Agent: logic
| @@ -423,13 +489,15 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |||
| time: slot.time, | |||
| ...schedule, | |||
There was a problem hiding this comment.
🔵 INFO — Code Organization (confidence: 81%)
Inconsistent organizerTimeZone parameter passing: passing schedule.timeZone to checkIfIsAvailable which expects organizerTimeZone
Evidence:
- Line 489:
organizerTimeZone: schedule.timeZone,— passes schedule.timeZone - Line 440:
organizerTimeZoneis already computed from eventType/schedule/userAvailability - This creates two separate sources of truth for the organizer's timezone and could lead to inconsistency if schedule.timeZone differs from the computed organizerTimeZone
Agent: style
| const slotEndTime = time.add(eventLength, "minutes").utc(); | ||
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists |
There was a problem hiding this comment.
🔵 INFO — Naming Conventions (confidence: 76%)
Variable name 'dateOverrideExist' uses singular 'Exist' but should follow camelCase conventions with consistent tense
Evidence:
- Line 100:
let dateOverrideExist = false;— mixes past tense 'dateOverride' with present tense 'Exist' - More consistent naming would be
dateOverrideExists(third-person singular present) orisDateOverride - This minor inconsistency affects readability when scanning related variable names
Agent: style
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (5 lines, 1 file))
Day-off detection uses
===to compare two different Dayjs objects, which compares by reference, not by value. This will always evaluate tofalse, meaning day-off date overrides (where start equals end) will never be detected. Slots on a day-off will incorrectly be shown as available.
| slotStartTime.format("YYYY MM DD") | |
| // Compare date values using .isSame(), not reference equality (===), | |
| // since two Dayjs objects are never === even if they represent the same instant. | |
| // A day-off override is represented as start === end (same timestamp). | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| } | ||
|
|
||
| if (dateOverrideExist) { | ||
| return true; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
The
endvariable in the working hours check is computed usingslotStartTimeinstead ofslotEndTime. Bothstartandendwill always have the same value, making the end-boundary check againstworkingHour.endTimeincorrect. A slot that extends past working hours will not be correctly rejected.
| return true; | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (8 lines, 1 file))
When
dateOverrideExistis true (an override exists for this date and the slot falls within it), the function immediately returnstruewithout checking busy times. This means a slot that overlaps with an existing booking will still be shown as available if it falls within a date override window.
| } | |
| // if no date override for slot exists, check if it is within normal work hours | |
| // if a date override exists, skip working hours check and fall through to busy times | |
| !dateOverrideExist && |
🤖 Grapple PR auto-fix • major • confidence: 100%
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
In
checkIfIsAvailable, the UTC offset is negated (* -1) when computing date override boundaries, but ingetSlots(packages/lib/slots.ts), the offset is not negated — it usesinviteeUtcOffset - organizerUtcOffsetdirectly. These two code paths handle the same date override data but apply offsets in opposite directions, which will produce inconsistent slot boundaries between slot generation and availability checking.
| let dateOverrideExist = false; | |
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() : 0; |
🤖 Grapple PR auto-fix • major • confidence: 100%
| return true; | ||
| } | ||
| if ( | ||
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
The date override boundary check marks a slot as outside the override when
slotEndTimeequals the override start (viaisSame), but does not apply the sameisSamecheck forslotStartTimevs override end. This creates an asymmetric boundary condition: a slot ending exactly at override start is rejected, but a slot starting exactly at override end is not rejected (since onlyisAfteris checked, notisSame).
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | |
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes")) || slotStartTime.isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • major • confidence: 100%
| ) { | ||
| return true; | ||
| } | ||
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (8 lines, 1 file))
Using
dateOverrides.find()means only the first matching date override for a given date is evaluated. If a user has multiple overlapping date overrides on the same date, subsequent overrides are ignored, which could lead to incorrect availability.
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| // check if any date override for slot exists | |
| // Use `some()` instead of `find()` so that ALL date overrides on the same date | |
| // are evaluated. With `find()`, only the first matching override was considered, | |
| // causing subsequent overlapping overrides to be silently ignored. | |
| dateOverrides.some((date) => { | |
| return false; |
🤖 Grapple PR auto-fix • minor • confidence: 100%
| if ( | ||
| dateOverrides.find((date) => { | ||
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | ||
|
|
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — style agent (Small fix (2 lines, 1 file))
Day-off date override detection uses reference equality (===) between two different Dayjs objects, which will always be false
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • minor • confidence: 95%
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( | ||
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (25 lines, 1 file) — review recommended)
The working hours check only rejects a slot if
workingHours.find()returns a matching working hour entry where the slot is outside bounds. If a slot falls on a day that has no matching working hour entry at all (i.e., the organizer doesn't work that day of the week), no working hour object is found,findreturnsundefined(falsy), and the slot is NOT rejected. This means slots on non-working days will pass through to the busy-time check and may be shown as available.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -137,14 +137,17 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
+ // A slot with no matching working hour entry means the organizer doesn't work that day at all.
+ const workingHoursForDay = workingHours.filter((workingHour) =>
+ workingHour.days.includes(slotStartTime.day())
+ );
+
+ // No working hour entry covers this day — organizer is not available
+ if (workingHoursForDay.length === 0) {
+ return false;
+ }
+
if (
- workingHours.find((workingHour) => {
- if (workingHour.days.includes(slotStartTime.day())) {
- const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
- if (start < workingHour.startTime || end > workingHour.endTime) {
- return true;
- }
- }
- })
+ workingHoursForDay.every((workingHour) => {
+ const start = slotStartTime.hour() * 60 + slotStartTime.minute();
+ const end = slotEndTime.hour() * 60 + slotEndTime.minute();
+ // Return true (slot is "outside") if it falls outside this entry's bounds
+ return start < workingHour.startTime || end > workingHour.endTime;
+ })
) {
// slot is outside of working hours
return false;
}🤖 Grapple PR auto-fix • major • Review this diff before applying
| ) { | ||
| // slot is not within the date override | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (12 lines, 1 file) — review recommended)
The working hours check uses
slotStartTime.day()which returns the UTC day of week (sinceslotStartTime = time.utc()). Working hours are typically defined in the organizer's local timezone, where the day of the week may differ from UTC near midnight boundaries. This can cause slots near midnight UTC to be checked against the wrong day's working hours.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -137,7 +137,10 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
- if (workingHour.days.includes(slotStartTime.day())) {
- const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ // Convert to organizer's local time for day-of-week comparison to avoid
+ // UTC midnight boundary mismatch (slotStartTime is UTC, but working hours
+ // days[] are defined in the organizer's local timezone).
+ const slotStartLocal = organizerTimeZone
+ ? slotStartTime.tz(organizerTimeZone)
+ : slotStartTime;
+ if (workingHour.days.includes(slotStartLocal.day())) {
+ const start = slotStartLocal.hour() * 60 + slotStartLocal.minute();
+ const end = slotStartLocal.hour() * 60 + slotStartLocal.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}🤖 Grapple PR auto-fix • minor • Review this diff before applying
There was a problem hiding this comment.
🤖 Grapple PR Review
Summary
This PR modifies date override and timezone handling in the scheduling system, adjusting how active overrides are calculated in slots.ts to account for timezone offsets between the organizer and invitee, and extending checkIfIsAvailable in the TRPC slots router to validate slots against date overrides and working hours.
Changes: 4 files (+111 / -10 lines)
- 🟡 4 files modified:
apps/web/test/lib/getSchedule.test.ts,packages/lib/slots.ts,packages/trpc/server/routers/viewer/slots.ts,packages/types/schedule.d.ts
Resolves: #7 (feat: convert InsightsBookingService to use Prisma.sql raw queries)
CI: ⏳ 0 passed, 0 failed
Impact Diagram
graph TD
pr_0[/"🔀 PR Changes: 4 files"/]
f1["🟡 getSchedule.test.ts +18/-0"]
pr_0 --> f1
f2["🟡 slots.ts +16/-5"]
pr_0 --> f2
f3["🟡 slots.ts +76/-5"]
pr_0 --> f3
f4["🟡 schedule.d.ts +1/-0"]
pr_0 --> f4
style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f3 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f4 fill:#7c3aed,color:#fff,stroke:#7c3aed
10 critical issues found — please address before merging.
| Severity | Count |
|---|---|
| 🔴 Critical | 10 |
| 🟠 Major | 8 |
| 🟡 Minor | 4 |
Review details
- Reviewed at: 2026-04-08T02:32:07.839Z
- Agents used: security, logic, style, architecture, performance, verification
- Context level: full
- Execution time: 122.0s
- Low-confidence findings suppressed: 7
- Blast radius: 0 (contained)
Powered by Grapple PR — AI that understands your code.
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
🔴 CRITICAL — Dayjs Reference Equality Bug (confidence: 100%)
Using === to compare two Dayjs objects compares object references, not values. This condition will NEVER be true, even when the start and end times are identical. This means zero-length date overrides (used to mark a day as unavailable) will not be detected, and slots on those days will incorrectly be marked as available.
Evidence:
- dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes') creates two distinct Dayjs objects — reference equality always returns false
- Intent specification explicitly calls this out: 'uses reference equality (===) on Dayjs objects, which will never be true — this condition is a logic bug and should use .isSame()'
- The purpose of this check is to detect empty-range overrides (day marked unavailable), which will now silently pass through
Agent: logic
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🔴 CRITICAL — Duplicate Variable Bug (confidence: 100%)
Both start and end are computed from slotStartTime, making them always identical. The end variable should use slotEndTime instead. This means the working hours end-time boundary check is always wrong — it checks the slot's start time against the working hour end time instead of the slot's end time, allowing slots that extend beyond working hours to be incorrectly marked as available.
Evidence:
- Line 134:
const start = slotStartTime.hour() * 60 + slotStartTime.minute(); - Line 135:
const end = slotStartTime.hour() * 60 + slotStartTime.minute(); - Both use
slotStartTime—endshould useslotEndTime(defined on line 100 astime.add(eventLength, 'minutes').utc()) - Intent specification confirms: '
endshould use slotEndTime instead'
Agent: logic
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; |
There was a problem hiding this comment.
🔴 CRITICAL — Sign Inversion Inconsistency (confidence: 100%)
In checkIfIsAvailable, the utcOffset is computed as dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1, negating the offset. However, in packages/lib/slots.ts (getSlots), the organizerUtcOffset is computed as dayjs(...).tz(override.timeZone).utcOffset() WITHOUT negation, and then the difference inviteeUtcOffset - organizerUtcOffset is used. These two offset sign conventions are inconsistent and will produce incorrect date matching or incorrect slot times in one of the two code paths.
Evidence:
- slots.ts line ~210:
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();— no negation - viewer/slots.ts line 103:
dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1— negated - Intent specification flags this: 'utcOffset * -1 in checkIfIsAvailable vs. no negation in slots.ts offset calculation — one of these is likely incorrect'
- dayjs .utcOffset() returns a positive value for timezones east of UTC (e.g., +330 for IST). The negation in checkIfIsAvailable converts this to -330, which when added to the date.start shifts it in the wrong direction for eastern timezones.
- slots.ts line 211:
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();— no negation - slots.ts line 213:
const offset = inviteeUtcOffset - organizerUtcOffset;— subtraction, no negation - slots.ts (checkIfIsAvailable) line 103:
dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1— explicit negation - dayjs .utcOffset() returns positive values for timezones east of UTC (e.g., +330 for IST). Negating this gives a negative number. The slots.ts getSlots path does not negate, creating inconsistency.
- Both paths aim to convert organizer-local override times to UTC-comparable values
Agent: logic
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { | ||
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); |
There was a problem hiding this comment.
🔴 CRITICAL — Logic Bug - Variable Assignment (confidence: 100%)
In the working hours validation block, the end variable is assigned the same value as start. Both use slotStartTime.hour() * 60 + slotStartTime.minute(), but end should use slotEndTime instead. This makes the end-time boundary check always incorrect.
Evidence:
- Line 140:
const start = slotStartTime.hour() * 60 + slotStartTime.minute(); - Line 141:
const end = slotStartTime.hour() * 60 + slotStartTime.minute();— uses slotStartTime, not slotEndTime - This identical assignment will cause slots to always fail the end-time check
end > workingHour.endTimeincorrectly, since end equals start
Agent: style
|
|
||
| return { | ||
| userIds: override.userId ? [override.userId] : [], | ||
| startTime: |
There was a problem hiding this comment.
🔴 CRITICAL — Sign Convention Inconsistency (confidence: 100%)
UTC offset calculation is inconsistent between checkIfIsAvailable and getSlots. In checkIfIsAvailable (slots.ts line 109), offset is negated with utcOffset() * -1. In getSlots (slots.ts lines 212-213), the offset is used without negation: inviteeUtcOffset - organizerUtcOffset. This sign inversion inconsistency will produce incorrect timezone adjustments.
Evidence:
- checkIfIsAvailable line 109:
dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1— negates the offset - getSlots lines 212-213:
const offset = inviteeUtcOffset - organizerUtcOffset;— no negation applied - These two code paths handle the same timezone offset adjustment but with opposite signs, leading to incorrect override times
Agent: style
| const organizerTimeZone = | ||
| eventType.timeZone || eventType?.schedule?.timeZone || userAvailability?.[0]?.timeZone; | ||
|
|
||
| for ( |
There was a problem hiding this comment.
🟠 MAJOR — Unnecessary work (confidence: 99%)
organizerTimeZone is derived from eventType.timeZone, eventType?.schedule?.timeZone, or userAvailability?.[0]?.timeZone once correctly before the loop (good), but at lines 489 and 514 it is overridden per-slot with schedule.timeZone or userSchedule.timeZone. The per-slot lookup userAvailability.find(({ user: { id: userId } }) => userId === slotUserId) at line ~573 is O(users) and executed for every slot in the collective availability path.
Evidence:
- Line ~573:
const userSchedule = userAvailability.find(({ user: { id: userId } }) => userId === slotUserId);inside a slot-level loop - For N slots × U users this is O(N×U) linear scans; should be a Map built once
- userAvailability is constant across all slot iterations — the Map can be built before the loop
Agent: performance
| organizerTimeZone: schedule.timeZone, | ||
| }); | ||
| const endCheckForAvailability = performance.now(); | ||
| checkForAvailabilityCount++; |
There was a problem hiding this comment.
🟡 MINOR — Missing Propagation of New Parameters (confidence: 100%)
The checkIfIsAvailable call at line 489 (and at line 514) passes organizerTimeZone: schedule.timeZone or organizerTimeZone: userSchedule.timeZone, but dateOverrides and workingHours are not explicitly passed — they rely on spread from availabilityCheckProps. It is unclear from the truncated diff whether availabilityCheckProps includes dateOverrides and workingHours. If it does not, the new date override and working hours checks in checkIfIsAvailable will silently use empty defaults and have no effect.
Evidence:
- availabilityCheckProps is defined around line 436 but its contents are not fully visible in the diff
- The new parameters have defaults:
dateOverrides = []andworkingHours = [], so missing props won't throw — they'll silently skip the checks - Intent specification notes: 'Truncated diff: cannot confirm checkIfIsAvailable is actually invoked with the new parameters'
Agent: logic
| } | ||
|
|
||
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( |
There was a problem hiding this comment.
🟡 MINOR — Working Hours Day Check Missing slotEndTime (confidence: 100%)
The working hours check only verifies that slotStartTime.day() is in the working hour days. If a slot spans midnight (e.g., 23:30-00:30), the end time falls on the next day, which might have different working hours. This edge case is not handled.
Evidence:
- Line 133:
workingHour.days.includes(slotStartTime.day())only checks the start day - For event types with long durations, the slot could span into the next calendar day
Agent: logic
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
🟡 MINOR — Type Safety - Module Boundary (confidence: 98%)
Adding timeZone?: string to the TimeRange type is a cross-cutting change that affects all consumers of this type across the codebase (Schedule type, working hours, any code destructuring TimeRange). While it's optional and non-breaking at the type level, it broadens the semantic scope of TimeRange beyond a simple time range. Consider whether a separate type like TimeRangeWithZone or extending only the date override type would be more architecturally appropriate.
Evidence:
- TimeRange is used in
Schedule = TimeRange[][]which represents weekly schedules, not just date overrides - Only date overrides need the timeZone field based on this diff
- Adding it to the base type means any TimeRange consumer could incorrectly assume timeZone is always populated
Agent: architecture
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
🟡 MINOR — Type Definition - Missing Documentation (confidence: 88%)
The new optional timeZone field on TimeRange type has no documentation. Given that this field is critical for timezone offset calculations in the override pipeline (as evidenced by its use in slots.ts and checkIfIsAvailable), it should be documented to clarify when and why it must be populated.
Evidence:
- Field added at line 5:
timeZone?: string;without JSDoc or comment - This field is essential for the date override timezone logic in getSlots and checkIfIsAvailable
- Consumers of TimeRange may not know they should populate this field for override processing to work correctly
Agent: style
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
Using
===to compare two Dayjs objects compares object references, not values. This condition will NEVER be true, even when the start and end times are identical. This means zero-length date overrides (used to mark a day as unavailable) will not be detected, and slots on those days will incorrectly be marked as available.
| slotStartTime.format("YYYY MM DD") | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
Both
startandendare computed fromslotStartTime, making them always identical. Theendvariable should useslotEndTimeinstead. This means the working hours end-time boundary check is always wrong — it checks the slot's start time against the working hour end time instead of the slot's end time, allowing slots that extend beyond working hours to be incorrectly marked as available.
| } | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
In checkIfIsAvailable, the utcOffset is computed as
dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1, negating the offset. However, in packages/lib/slots.ts (getSlots), the organizerUtcOffset is computed asdayjs(...).tz(override.timeZone).utcOffset()WITHOUT negation, and then the differenceinviteeUtcOffset - organizerUtcOffsetis used. These two offset sign conventions are inconsistent and will produce incorrect date matching or incorrect slot times in one of the two code paths.
| let dateOverrideExist = false; | |
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() : 0; |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { | ||
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — style agent (Small fix (2 lines, 1 file))
In the working hours validation block, the
endvariable is assigned the same value asstart. Both useslotStartTime.hour() * 60 + slotStartTime.minute(), butendshould useslotEndTimeinstead. This makes the end-time boundary check always incorrect.
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
|
|
||
| return { | ||
| userIds: override.userId ? [override.userId] : [], | ||
| startTime: |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — style agent (Small fix (8 lines, 1 file))
UTC offset calculation is inconsistent between checkIfIsAvailable and getSlots. In checkIfIsAvailable (slots.ts line 109), offset is negated with
utcOffset() * -1. In getSlots (slots.ts lines 212-213), the offset is used without negation:inviteeUtcOffset - organizerUtcOffset. This sign inversion inconsistency will produce incorrect timezone adjustments.
| startTime: | |
| // Negate organizer offset (matching checkIfIsAvailable sign convention) then shift to invitee local. | |
| // organizerUtcOffset * -1 converts from organizer-local to UTC; + inviteeUtcOffset shifts to invitee-local. | |
| const organizerUtcOffset = dayjs.tz(override.start, override.timeZone).utcOffset() * -1; | |
| const inviteeUtcOffset = dayjs.tz(override.start, timeZone).utcOffset(); | |
| const offset = organizerUtcOffset + inviteeUtcOffset; |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") | ||
| ) { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — architecture agent (Small fix (4 lines, 1 file))
Dayjs objects are compared using
===(reference equality), which will never be true since eachdayjs()call creates a new object. This condition is meant to detect zero-length date overrides (start === end) but is dead code. As a result, zero-length overrides will never be recognized, and the logic will fall through to the boundary checks below, potentially producing incorrect availability results.
| ) { | |
| if ( | |
| dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes")) | |
| ) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| if (dateOverrideExist) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — architecture agent (Small fix (2 lines, 1 file))
Both
startandendare computed fromslotStartTime, making the working hours end-boundary check always identical to the start-boundary check. Theendvariable should useslotEndTimeinstead. This means a slot that starts within working hours but extends past the end of working hours will incorrectly pass validation.
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| ) { | ||
| return true; | ||
| } | ||
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — performance agent (Small fix (5 lines, 1 file))
Dayjs object reference equality (===) is used to compare two distinct Dayjs instances. This condition can never be true, so the 'empty override window' early-return branch (return true inside the find, causing the outer if to return false) is permanently dead code. Any date override with start === end intended to mean 'available all day' will be misidentified as a bounded window and slots will be incorrectly marked unavailable.
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| // Use .isSame() for value equality — === on Dayjs objects tests reference identity | |
| // and always returns false for two separately constructed instances. | |
| // start === end signals an "available all day" override; return true (slot is available). | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| } | ||
| } | ||
| }) | ||
| ) { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — performance agent (Small fix (2 lines, 1 file))
In the working hours check block, both
startandendare computed from slotStartTime, making the end boundary check always compare the slot start against workingHour.endTime. This means the end-of-slot boundary is never validated — slots that extend past working hours will not be caught, and the check is logically equivalent to only checking the start boundary.
| ) { | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| return true; | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (4 lines, 1 file))
The
findcallback returnstruewhen the slot is OUTSIDE the date override window (before override start or after override end), then the outerifblock returnsfalse(unavailable). But the internaldateOverrideExistflag is set totrueinside the samefind. Iffindreturnsundefined(slot is WITHIN the override) anddateOverrideExistis true, execution falls through to line 130 which returnstrue— but this skips the busy-time check entirely. This means a slot within a date override that is already booked will still be returned as available, enabling double-booking.
| }) | |
| // slot is within the date override window — still need to check busy times below |
🤖 Grapple PR auto-fix • major • confidence: 100%
| return true; | ||
| } | ||
| if ( | ||
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (5 lines, 1 file))
The condition
slotEndTime.isBefore(overrideStart) || slotEndTime.isSame(overrideStart)marks a slot as 'outside the override' when the slot ends exactly at the override start time. However, this returns true fromfind, which then causesreturn false— marking the slot as unavailable. A slot ending exactly at the override boundary is indeed outside the override, so the logic is correct for exclusion. But if we consider the complementary case: a slot that STARTS exactly at the override END time (slotStartTime.isAfter(overrideEnd)) —isAfteris strict, so a slot starting exactly at override end is NOT excluded. This asymmetry could lead to an off-by-one slot being included at the end of an override window.
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | |
| if ( | |
| slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes")) || | |
| slotStartTime.isSame(dayjs(date.end).add(utcOffset, "minutes")) | |
| ) { |
🤖 Grapple PR auto-fix • major • confidence: 100%
| startTime: override.start.getUTCHours() * 60 + override.start.getUTCMinutes(), | ||
| endTime: override.end.getUTCHours() * 60 + override.end.getUTCMinutes(), | ||
| })); | ||
| const overrides = activeOverrides.flatMap((override) => { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (9 lines, 1 file))
When
override.timeZoneis undefined (the timeZone field on TimeRange is optional),dayjs(override.start.toString()).tz(undefined)will produce unexpected behavior. Dayjstz(undefined)may fall back to the local timezone of the server rather than UTC, leading to incorrect offset calculations.
| const overrides = activeOverrides.flatMap((override) => { | |
| // Fall back to "UTC" if override.timeZone is undefined (TimeRange.timeZone is optional). | |
| // Using UTC (offset=0) is the safest neutral default: it avoids dayjs().tz(undefined) | |
| // silently adopting the server's local timezone, which would produce incorrect offsets. | |
| const organizerTimeZone = override.timeZone || "UTC"; | |
| const organizerUtcOffset = dayjs(override.start.toString()).tz(organizerTimeZone).utcOffset(); | |
| // timeZone here is the invitee's timezone, expected to always be defined in this context. | |
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); |
🤖 Grapple PR auto-fix • major • confidence: 100%
| dateOverrideExist = true; | ||
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — performance agent (Small fix (7 lines, 1 file))
The utcOffset is recomputed via dayjs.tz() on every call to checkIfIsAvailable for every override in the dateOverrides array. Since checkIfIsAvailable is called O(slots) times per day and dateOverrides is constant across those calls, the tz computation is repeated thousands of times with identical inputs.
| } | |
| // Pre-compute utcOffset once per checkIfIsAvailable call — organizerTimeZone is constant | |
| // across all overrides, so computing it inside .find() repeats the tz parse O(overrides) times | |
| // per slot call, leading to ~14,400 redundant parses over a 30-day/48-slot/10-override window. | |
| const utcOffset = organizerTimeZone ? dayjs.tz(time.toDate(), organizerTimeZone).utcOffset() * -1 : 0; | |
🤖 Grapple PR auto-fix • major • confidence: 100%
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — architecture agent (Small fix (10 lines, 1 file))
Adding
timeZone?: stringto theTimeRangetype is a cross-cutting change that affects all consumers of this type across the codebase (Schedule type, working hours, any code destructuring TimeRange). While it's optional and non-breaking at the type level, it broadens the semantic scope of TimeRange beyond a simple time range. Consider whether a separate type likeTimeRangeWithZoneor extending only the date override type would be more architecturally appropriate.
| timeZone?: string; | |
| /** | |
| * A TimeRange that carries timezone information, used specifically for date overrides | |
| * where the organizer's or invitee's timezone must be tracked alongside the time window. | |
| * Do NOT use this for weekly schedule slots (Schedule = TimeRange[][]) where timezone | |
| * context is handled separately via WorkingHours / getWorkingHours(). | |
| */ | |
| export type TimeRangeWithZone = TimeRange & { | |
| timeZone?: string; | |
| }; | |
🤖 Grapple PR auto-fix • minor • confidence: 98%
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — style agent (Small fix (7 lines, 1 file))
The new optional
timeZonefield on TimeRange type has no documentation. Given that this field is critical for timezone offset calculations in the override pipeline (as evidenced by its use in slots.ts and checkIfIsAvailable), it should be documented to clarify when and why it must be populated.
| timeZone?: string; | |
| /** | |
| * Optional IANA timezone string (e.g. "America/New_York") associated with this time range. | |
| * Must be populated when this TimeRange represents a date override so that timezone offset | |
| * calculations between the organizer and invitee are applied correctly in the override | |
| * pipeline (see lib/slots.ts and the TRPC slots router `checkIfIsAvailable`). | |
| * When absent, no timezone conversion is performed and start/end are treated as-is. | |
| */ |
🤖 Grapple PR auto-fix • minor • confidence: 88%
|
|
||
| return busy.every((busyTime) => { | ||
| const startTime = dayjs.utc(busyTime.start).utc(); | ||
| const endTime = dayjs.utc(busyTime.end); |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (31 lines, 1 file) — review recommended)
The dateOverrides and workingHours arrays are searched with .find() inside checkIfIsAvailable, which is called inside a nested loop (per-day × per-slot × per-user). With large availability windows this is O(days × slots × users × overrides), but more critically, dayjs.tz() is called multiple times per override per invocation — each call is expensive and the result is never cached. With 30 days × 48 slots × 5 users × N overrides this becomes a significant performance regression.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -99,50 +99,56 @@ const checkIfIsAvailable = ({
const slotEndTime = time.add(eventLength, "minutes").utc();
const slotStartTime = time.utc();
+ // Pre-compute the slot's date string once, outside any loop
+ const slotStartDay = slotStartTime.format("YYYY MM DD");
+
//check if date override for slot exists
let dateOverrideExist = false;
+ // Pre-compute adjusted override boundaries once per override (not per-comparison),
+ // avoiding repeated dayjs.tz() + dayjs().add() calls inside the find() predicate.
if (
dateOverrides.find((date) => {
- const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0;
-
+ // dayjs.tz() is expensive — compute utcOffset exactly once per override entry
+ const utcOffset = organizerTimeZone
+ ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1
+ : 0;
+
+ // Compute adjusted start/end once per override, not 3-4× per predicate call
+ const adjustedStart = dayjs(date.start).add(utcOffset, "minutes");
+ const adjustedEnd = dayjs(date.end).add(utcOffset, "minutes");
+
if (
- dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") ===
- slotStartTime.format("YYYY MM DD")
+ adjustedStart.format("YYYY MM DD") === slotStartDay
) {
dateOverrideExist = true;
- if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) {
+ // All-day blocked override: start === end means fully blocked
+ if (adjustedStart.isSame(adjustedEnd)) {
return true;
}
if (
- slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) ||
- slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes"))
+ slotEndTime.isBefore(adjustedStart) ||
+ slotEndTime.isSame(adjustedStart)
) {
return true;
}
- if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) {
+ if (slotStartTime.isAfter(adjustedEnd)) {
return true;
}
}
})
) {
// slot is not within the date override
return false;
}
if (dateOverrideExist) {
return true;
}
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ // Bug fix: end should use slotEndTime, not slotStartTime (was a copy-paste error)
+ const end = slotEndTime.hour() * 60 + slotEndTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {
// slot is outside of working hours
return false;
}🤖 Grapple PR auto-fix • critical • Review this diff before applying
| }); | ||
|
|
||
| // unset all working hours that relate to this user availability override | ||
| overrides.forEach((override) => { |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (11 lines, 1 file) — review recommended)
For each active override, dayjs(override.start.toString()).tz(override.timeZone) and dayjs(override.start.toString()).tz(timeZone) are called to compute utcOffset, and then dayjs(override.start).utc().add(offset, 'minute') and dayjs(override.end).utc().add(offset, 'minute') are each called twice (once for .hour() and once for .minute()). This is 6 dayjs constructions and tz parses per override, all of which could be collapsed to 2–3.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -210,16 +210,14 @@ const getSlots = ({
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;
+ const adjustedStart = dayjs(override.start).utc().add(offset, "minute");
+ const adjustedEnd = dayjs(override.end).utc().add(offset, "minute");
+
return {
userIds: override.userId ? [override.userId] : [],
- startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
- endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ startTime: adjustedStart.hour() * 60 + adjustedStart.minute(),
+ endTime: adjustedEnd.hour() * 60 + adjustedEnd.minute(),
};
});🤖 Grapple PR auto-fix • major • Review this diff before applying
| organizerTimeZone?: string; | ||
| }): boolean => { | ||
| if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) { | ||
| return true; |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (15 lines, 1 file) — review recommended)
The
checkIfIsAvailablefunction's parameter object has been extended withdateOverrides,workingHours, andorganizerTimeZone. While these have default values, the function's behavior has fundamentally changed: it now returnsfalseearly based on date override and working hour checks before evaluating busy times. All call sites that passdateOverridesorworkingHours(lines 489-492, 514-517, 579-584) must consistently provide these parameters, but the diff is truncated and we cannot verify all invocations. Furthermore, the early-return semantics mean a slot outside a date override is rejected without checkingcurrentSeats, potentially breaking the existing booking flow for seated events.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -75,7 +75,7 @@ const checkIfIsAvailable = ({
time,
busy,
eventLength,
dateOverrides = [],
workingHours = [],
currentSeats,
organizerTimeZone,
}: {
time: Dayjs;
busy: EventBusyDate[];
eventLength: number;
dateOverrides?: {
start: Date;
end: Date;
}[];
workingHours?: WorkingHours[];
currentSeats?: CurrentSeats;
organizerTimeZone?: string;
}): boolean => {
- if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) {
- return true;
- }
-
const slotEndTime = time.add(eventLength, "minutes").utc();
const slotStartTime = time.utc();
//check if date override for slot exists
let dateOverrideExist = false;
if (
dateOverrides.find((date) => {
const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0;
if (
dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") ===
slotStartTime.format("YYYY MM DD")
) {
dateOverrideExist = true;
if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) {
return true;
}
if (
slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) ||
slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes"))
) {
return true;
}
if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) {
return true;
}
}
})
) {
// slot is not within the date override
return false;
}
if (dateOverrideExist) {
+ // slot is within a date override window — check seats and busy times
+ if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) {
+ return true;
+ }
return true;
}
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ const end = slotEndTime.hour() * 60 + slotEndTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {
// slot is outside of working hours
return false;
}
+ // After override/working-hours checks pass, a seated booking is always available
+ if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) {
+ return true;
+ }
+
return busy.every((busyTime) => {🤖 Grapple PR auto-fix • major • Review this diff before applying
| const organizerTimeZone = | ||
| eventType.timeZone || eventType?.schedule?.timeZone || userAvailability?.[0]?.timeZone; | ||
|
|
||
| for ( |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — performance agent (Small fix (9 lines, 1 file))
organizerTimeZone is derived from eventType.timeZone, eventType?.schedule?.timeZone, or userAvailability?.[0]?.timeZone once correctly before the loop (good), but at lines 489 and 514 it is overridden per-slot with
schedule.timeZoneoruserSchedule.timeZone. The per-slot lookupuserAvailability.find(({ user: { id: userId } }) => userId === slotUserId)at line ~573 is O(users) and executed for every slot in the collective availability path.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -489,6 +489,12 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
// what else are you going to call it?
const looseHostAvailability = userAvailability.filter(({ user: { isFixed } }) => !isFixed);
if (looseHostAvailability.length > 0) {
+ // Build a Map<userId, schedule> once — userAvailability is constant across all slot
+ // iterations, so repeated Array.find() inside the loop would be O(N×U). Using a Map
+ // makes each per-slot lookup O(1).
+ const userAvailabilityMap = new Map(
+ userAvailability.map((schedule) => [schedule.user.id, schedule])
+ );
+
availableTimeSlots = availableTimeSlots
.map((slot) => {
slot.userIds = slot.userIds?.filter((slotUserId) => {
@@ -576,7 +582,7 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
return false;
}
- const userSchedule = userAvailability.find(({ user: { id: userId } }) => userId === slotUserId);
+ const userSchedule = userAvailabilityMap.get(slotUserId);
return checkIfIsAvailable({
time: slot.time,🤖 Grapple PR auto-fix • major • Review this diff before applying
| organizerTimeZone: schedule.timeZone, | ||
| }); | ||
| const endCheckForAvailability = performance.now(); | ||
| checkForAvailabilityCount++; |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Small fix (4 lines, 1 file))
The
checkIfIsAvailablecall at line 489 (and at line 514) passesorganizerTimeZone: schedule.timeZoneororganizerTimeZone: userSchedule.timeZone, butdateOverridesandworkingHoursare not explicitly passed — they rely on spread fromavailabilityCheckProps. It is unclear from the truncated diff whetheravailabilityCheckPropsincludesdateOverridesandworkingHours. If it does not, the new date override and working hours checks incheckIfIsAvailablewill silently use empty defaults and have no effect.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -485,7 +485,9 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
const isAvailable = checkIfIsAvailable({
time: slot.time,
...schedule,
...availabilityCheckProps,
+ dateOverrides: schedule.dateOverrides ?? dateOverrides,
+ workingHours: schedule.workingHours ?? workingHours,
organizerTimeZone: schedule.timeZone,
});
const endCheckForAvailability = performance.now();
@@ -510,7 +512,9 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
checkIfIsAvailable({
time: slot.time,
...userSchedule,
...availabilityCheckProps,
+ dateOverrides: userSchedule.dateOverrides ?? dateOverrides,
+ workingHours: userSchedule.workingHours ?? workingHours,
organizerTimeZone: userSchedule.timeZone,
});🤖 Grapple PR auto-fix • minor • Review this diff before applying
| } | ||
|
|
||
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (21 lines, 1 file) — review recommended)
The working hours check only verifies that
slotStartTime.day()is in the working hour days. If a slot spans midnight (e.g., 23:30-00:30), the end time falls on the next day, which might have different working hours. This edge case is not handled.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -138,12 +138,30 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ const end = slotEndTime.hour() * 60 + slotEndTime.minute();
+ // If the slot spans midnight, only validate the start portion against this working hour
+ const effectiveEnd = slotEndTime.day() !== slotStartTime.day() ? 24 * 60 : end;
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {
// slot is outside of working hours
return false;
}
+
+ // If the slot spans midnight, also verify the end portion falls within working hours on the next day
+ if (slotEndTime.day() !== slotStartTime.day()) {
+ const endMinutes = slotEndTime.hour() * 60 + slotEndTime.minute();
+ const endDayHasValidWorkingHour = workingHours.some((workingHour) => {
+ if (workingHour.days.includes(slotEndTime.day())) {
+ // The slot end portion runs from midnight (0) to endMinutes
+ return endMinutes <= workingHour.endTime && workingHour.startTime === 0;
+ }
+ return false;
+ });
+ if (!endDayHasValidWorkingHour) {
+ // slot end crosses into a day with no covering working hours
+ return false;
+ }
+ }
+🤖 Grapple PR auto-fix • minor • Review this diff before applying
|
@grapple-pr review |
There was a problem hiding this comment.
🤖 Grapple PR Review
Summary
This PR adds timezone-aware date override handling to the scheduling system, ensuring that when an organizer has a date override, slots are correctly computed relative to both the organizer's and invitee's timezones. It also extends checkIfIsAvailable to validate slots against date overrides and working hours before checking busy times.
Changes: 4 files (+111 / -10 lines)
- 🟡 4 files modified:
apps/web/test/lib/getSchedule.test.ts,packages/lib/slots.ts,packages/trpc/server/routers/viewer/slots.ts,packages/types/schedule.d.ts
Resolves: #7 (feat: convert InsightsBookingService to use Prisma.sql raw queries)
CI: ⏳ 0 passed, 0 failed
Impact Diagram
graph TD
pr_0[/"🔀 PR Changes: 4 files"/]
f1["🟡 getSchedule.test.ts +18/-0"]
pr_0 --> f1
f2["🟡 slots.ts +16/-5"]
pr_0 --> f2
f3["🟡 slots.ts +76/-5"]
pr_0 --> f3
f4["🟡 schedule.d.ts +1/-0"]
pr_0 --> f4
style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f3 fill:#7c3aed,color:#fff,stroke:#7c3aed
style f4 fill:#7c3aed,color:#fff,stroke:#7c3aed
9 critical issues found — please address before merging.
| Severity | Count |
|---|---|
| 🔴 Critical | 9 |
| 🟠 Major | 9 |
| 🟡 Minor | 7 |
| 🔵 Info | 1 |
Review details
- Reviewed at: 2026-04-12T02:06:40.928Z
- Agents used: security, logic, style, architecture, performance, verification
- Context level: full
- Execution time: 98.9s
- Low-confidence findings suppressed: 4
- Blast radius: 0 (contained)
Powered by Grapple PR — AI that understands your code.
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | ||
| ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🔴 CRITICAL — Logic Bug — Reference Equality on Dayjs Objects (confidence: 100%)
Zero-duration date override detection uses JavaScript reference equality (===) between two different Dayjs object instances. Two distinct Dayjs objects will NEVER be === even if they represent the same moment. This means the zero-duration (unavailable all day) override branch can never execute, causing all-day blocked overrides to be treated as normal overrides with a valid window.
Evidence:
dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes')creates two separate Dayjs object instances.- In JavaScript, object reference equality (
===) compares memory addresses, not values. Two separately constructed objects are never===. - This means a date override where
start === end(intended to block the entire day) will silently fall through to the window-check logic, potentially allowing bookings that should be blocked. - The fix requires
.isSame():dayjs(date.start).add(utcOffset, 'minutes').isSame(dayjs(date.end).add(utcOffset, 'minutes'))
Agent: security
| if (start < workingHour.startTime || end > workingHour.endTime) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 CRITICAL — Logic Bug — Wrong Variable for End-Time Boundary Check (confidence: 100%)
In the workingHours check, both start and end are computed from slotStartTime. The variable end should be derived from slotEndTime but instead copies slotStartTime. As a result, the condition end > workingHour.endTime can never catch a slot that overruns the end of the working day — the working-hours upper boundary is completely ineffective.
Evidence:
const start = slotStartTime.hour() * 60 + slotStartTime.minute();— correctconst end = slotStartTime.hour() * 60 + slotStartTime.minute();— should beslotEndTime- Because
start === endalways,end > workingHour.endTimefires only when the slot start itself is past end-of-day, which means a 60-minute slot starting at 23:30 (extending past midnight) would not be caught. - This is a functional correctness defect that causes incorrect availability to be returned, potentially allowing overbooking outside configured working hours.
Agent: security
| let dateOverrideExist = false; | ||
|
|
||
| if ( | ||
| dateOverrides.find((date) => { |
There was a problem hiding this comment.
🔴 CRITICAL — Logic Bug — Incorrect UTC Offset Sign Convention (confidence: 100%)
The UTC offset is computed as dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1, negating the result before adding it to the date. Dayjs .utcOffset() returns a positive value for UTC+ timezones. Negating it and then adding means UTC+5 (offset=300) becomes -300 minutes, moving the time 5 hours backward instead of forward. This is the opposite sign convention used in slots.ts, creating inconsistent behavior and incorrect date comparisons for organizers in non-UTC timezones.
Evidence:
- In
slots.ts:const offset = inviteeUtcOffset - organizerUtcOffset;thendayjs(override.start).utc().add(offset, 'minute')— additive approach. - In
slots.ts(viewer):const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0;then.add(utcOffset, 'minutes')— negated then added. - For UTC+6:
.utcOffset()= 360,* -1= -360,.add(-360, 'minutes')shifts 6 hours backward. This is wrong — to convert a UTC+6 local time to UTC you subtract 6 hours, but the intent here is to find the local date, which requires adding the offset. - For UTC-5:
.utcOffset()= -300,* -1= 300,.add(300, 'minutes')shifts 5 hours forward. Also incorrect for the same reason. - This sign error means the date-matching logic (
format('YYYY MM DD')comparison) will compute the wrong local date for any organizer not in UTC, causing overrides to apply on the wrong calendar day. - Line 102:
const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; - If
organizerTimeZoneisundefinedor missing, the offset is hardcoded to 0 (UTC) - This means date overrides are always checked against UTC dates, not the organizer's local date
- Could mask bugs where organizers have no timezone set but expect local-timezone behavior
- slots.ts line 211-213:
organizerUtcOffset = dayjs(...).tz(override.timeZone).utcOffset()andinviteeUtcOffset = dayjs(...).tz(timeZone).utcOffset()thenoffset = inviteeUtcOffset - organizerUtcOffset - viewer/slots.ts line 103:
utcOffset = dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 - For UTC+5:30 (India), utcOffset() = 330, negated = -330. For UTC-5 (EST), utcOffset() = -300, negated = +300.
- The intent here appears to be converting organizer-local times to UTC, but the negation approach will produce wrong results for certain timezone/date combinations compared to the slots.ts approach
Agent: security
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
🔴 CRITICAL — Business logic correctness (confidence: 100%)
Zero-duration date override check uses === reference equality between two different Dayjs objects, which will always evaluate to false. This means zero-duration date overrides (where start equals end, typically used to mark a day as unavailable) will never be detected, and the function will incorrectly treat the slot as available within the override.
Evidence:
- The expression
dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes')compares two distinct Dayjs object references, not their underlying values. - In JavaScript,
===on objects checks reference identity, not structural equality. Two separatedayjs()calls always produce different object references. - A zero-duration override (start === end) is a common pattern to mark a day as completely unavailable. This bug causes such overrides to be silently ignored.
- The intent specification confirms this as an edge case: 'uses reference equality between two different Dayjs objects, which will always be false — this branch is a bug and will never execute'.
Agent: logic
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🔴 CRITICAL — Business logic correctness (confidence: 100%)
The end variable in the working-hours boundary check is computed from slotStartTime instead of slotEndTime, making the end-time guard completely ineffective. A slot whose end time exceeds workingHour.endTime will never be caught.
Evidence:
- Line 134:
const start = slotStartTime.hour() * 60 + slotStartTime.minute(); - Line 135:
const end = slotStartTime.hour() * 60 + slotStartTime.minute(); startandendare always identical because both useslotStartTime.- The condition
end > workingHour.endTimewill only trigger whenstart > workingHour.endTime, meaning slots that start within working hours but extend past the end will incorrectly pass the check. - The intent specification identifies this: '
endshould useslotEndTime— meaningend > workingHour.endTimewill never catch a slot that overruns end-of-day'.
Agent: logic
| })); | ||
| const overrides = activeOverrides.flatMap((override) => { | ||
| const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset(); | ||
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); |
There was a problem hiding this comment.
🟡 MINOR — Code patterns (confidence: 100%)
The UTC offset is calculated only once from override.start, but if a DST (Daylight Saving Time) transition occurs between the override's start and end times, the offset may differ. Using a single offset value could produce incorrect minute-of-day calculations for overrides that span a DST boundary.
Evidence:
- Line 210-211: offset is computed from
override.startonly - Line 212-216: the same offset is applied to both
override.startandoverride.end - In regions with DST,
dayjs.tz(date, tz).utcOffset()can differ between two times if DST changes between them - For a 2-hour override spanning a DST transition, the end time could be offset incorrectly by ±60 minutes
Agent: style
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); | ||
| const offset = inviteeUtcOffset - organizerUtcOffset; | ||
|
|
||
| return { |
There was a problem hiding this comment.
🟡 MINOR — Code patterns (confidence: 100%)
Adding the offset delta and converting to minute-of-day does not use modular arithmetic. If the offset delta is large enough to shift a time past midnight, the minute-of-day calculation will wrap incorrectly (e.g., hour() will become negative or exceed 23). This is especially problematic for midnight-spanning overrides or large UTC offset differences.
Evidence:
- Lines 212-213 and 215-216 compute
hour() * 60 + minute()directly on offset-adjusted times - If an override ends at 23:00 UTC and offset is +120 minutes, the adjusted end time is 01:00 the next day
- Calling
.hour() * 60 + .minute()on a time past midnight returns an incorrect minute-of-day value - Should use
.startOf('day').diff(adjustedTime, 'minutes')or similar to get minute-of-day with proper wrapping
Agent: style
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
🟡 MINOR — Missing Type Propagation (confidence: 98%)
The timeZone field is added as optional to TimeRange, but the dateOverrides parameter type in checkIfIsAvailable (viewer/slots.ts line 82-85) is defined inline as { start: Date; end: Date; }[] without including timeZone. This means the type system does not enforce that timezone context is carried through to checkIfIsAvailable, and the function cannot access timeZone from individual overrides even though it's available in the data.
Evidence:
- packages/types/schedule.d.ts line 5:
timeZone?: string;added to TimeRange - packages/trpc/server/routers/viewer/slots.ts line 82-85:
dateOverrides?: { start: Date; end: Date; }[]— does not include timeZone - The function uses a single
organizerTimeZoneparameter instead, which means per-override timezone info is lost if multiple users have different timezones
Agent: architecture
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; |
There was a problem hiding this comment.
🟡 MINOR — Code patterns (confidence: 88%)
Object reference comparison used for Dayjs instances will always return false. The condition dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes') creates two separate Dayjs objects and compares them by reference, not by value. This zero-duration override detection will never work as intended.
Evidence:
- Dayjs objects are created fresh each time; reference equality (
===) between two different instances always fails - The intent appears to be detecting when an override has start === end (zero duration), but the current code creates new Dayjs instances that can never be reference-equal
- Should use
.isSame()method:dayjs(date.start).add(utcOffset, 'minutes').isSame(dayjs(date.end).add(utcOffset, 'minutes'))
Agent: style
| endTime: override.end.getUTCHours() * 60 + override.end.getUTCMinutes(), | ||
| })); | ||
| const overrides = activeOverrides.flatMap((override) => { | ||
| const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset(); |
There was a problem hiding this comment.
🔵 INFO — DST Sensitivity (confidence: 100%)
The UTC offset is computed using only override.start for both organizerUtcOffset and inviteeUtcOffset. If a date override spans a DST transition (e.g., clocks change between override.start and override.end), the offset applied to override.end will be incorrect by the DST delta (typically ±60 minutes). This is an inherent limitation of the single-offset approach.
Evidence:
- Line 211:
dayjs(override.start.toString()).tz(override.timeZone).utcOffset()— uses override.start - Line 212:
dayjs(override.start.toString()).tz(timeZone).utcOffset()— also uses override.start - The end time on line 219-221 applies the same offset computed from start
- DST transitions during an override window (rare but possible) would cause the end time to be off by the DST shift amount
Agent: architecture
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | ||
| ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — security agent (Small fix (2 lines, 1 file))
Zero-duration date override detection uses JavaScript reference equality (
===) between two different Dayjs object instances. Two distinct Dayjs objects will NEVER be===even if they represent the same moment. This means the zero-duration (unavailable all day) override branch can never execute, causing all-day blocked overrides to be treated as normal overrides with a valid window.
| } | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| if (start < workingHour.startTime || end > workingHour.endTime) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — security agent (Small fix (2 lines, 1 file))
In the
workingHourscheck, bothstartandendare computed fromslotStartTime. The variableendshould be derived fromslotEndTimebut instead copiesslotStartTime. As a result, the conditionend > workingHour.endTimecan never catch a slot that overruns the end of the working day — the working-hours upper boundary is completely ineffective.
| } | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| let dateOverrideExist = false; | ||
|
|
||
| if ( | ||
| dateOverrides.find((date) => { |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — security agent (Small fix (2 lines, 1 file))
The UTC offset is computed as
dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1, negating the result before adding it to the date. Dayjs.utcOffset()returns a positive value for UTC+ timezones. Negating it and then adding means UTC+5 (offset=300) becomes -300 minutes, moving the time 5 hours backward instead of forward. This is the opposite sign convention used inslots.ts, creating inconsistent behavior and incorrect date comparisons for organizers in non-UTC timezones.
| dateOverrides.find((date) => { | |
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() : 0; |
🤖 Grapple PR auto-fix • critical • confidence: 100%
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
Zero-duration date override check uses
===reference equality between two different Dayjs objects, which will always evaluate tofalse. This means zero-duration date overrides (where start equals end, typically used to mark a day as unavailable) will never be detected, and the function will incorrectly treat the slot as available within the override.
| slotStartTime.format("YYYY MM DD") | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { |
🤖 Grapple PR auto-fix • critical • confidence: 100%
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))
The
endvariable in the working-hours boundary check is computed fromslotStartTimeinstead ofslotEndTime, making the end-time guard completely ineffective. A slot whose end time exceedsworkingHour.endTimewill never be caught.
| } | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
🤖 Grapple PR auto-fix • critical • confidence: 100%
| } | ||
|
|
||
| if (dateOverrideExist) { | ||
| return true; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — architecture agent (Small fix (10 lines, 1 file))
The working hours check uses
slotStartTime.day()whereslotStartTimeistime.utc(). The.day()method returns the day-of-week in UTC. However,workingHours[].daysare defined relative to the organizer's local timezone. For organizers in timezones significantly offset from UTC, a slot at e.g. 23:00 UTC on Monday might be Tuesday in the organizer's timezone (UTC+2), causing the wrong day's working hours to be applied.
| return true; | |
| // Convert slot time to organizer's local timezone for day-of-week and | |
| // hour/minute extraction, since workingHours.days and startTime/endTime | |
| // are defined relative to the organizer's local timezone, not UTC. | |
| const slotInOrganizerTz = organizerTimeZone ? dayjs.tz(time, organizerTimeZone) : slotStartTime; | |
| if (workingHour.days.includes(slotInOrganizerTz.day())) { | |
| const start = slotInOrganizerTz.hour() * 60 + slotInOrganizerTz.minute(); | |
| const end = slotInOrganizerTz.hour() * 60 + slotInOrganizerTz.minute(); |
🤖 Grapple PR auto-fix • major • confidence: 100%
|
|
||
| if ( | ||
| dateOverrides.find((date) => { | ||
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — security agent (Small fix (7 lines, 1 file))
When
organizerTimeZoneisundefined,utcOffsetdefaults to0, causing the date comparison to use UTC dates. For organizers in non-UTC timezones without a configured timezone, overrides may be matched against the wrong calendar day (e.g., a UTC-5 organizer's override for Monday would be matched against Sunday UTC).
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | |
| // Use "UTC" as fallback so dayjs.tz() always operates with a real timezone. | |
| // Defaulting utcOffset to 0 without a timezone is only correct for UTC organizers; | |
| // for all others it would match overrides against the wrong calendar day. | |
| const resolvedOrganizerTimeZone = organizerTimeZone || "UTC"; | |
| const utcOffset = dayjs.tz(date.start, resolvedOrganizerTimeZone).utcOffset() * -1; |
🤖 Grapple PR auto-fix • minor • confidence: 100%
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — logic agent (Small fix (7 lines, 1 file))
When
organizerTimeZoneisundefined,utcOffsetdefaults to0, meaning date-matching comparisons use raw UTC dates. For organizers in non-UTC timezones, this could cause date overrides to be matched against the wrong calendar day.
| let dateOverrideExist = false; | |
| // When organizerTimeZone is provided, compute the UTC offset for that timezone. | |
| // When undefined, fall back to the offset embedded in the date itself so that | |
| // organizers in non-UTC timezones still get correct calendar-day matching. | |
| const utcOffset = organizerTimeZone | |
| ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 | |
| : dayjs(date.start).utcOffset() * -1; |
🤖 Grapple PR auto-fix • minor • confidence: 100%
| userId?: number | null; | ||
| start: Date; | ||
| end: Date; | ||
| timeZone?: string; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — architecture agent (Small fix (4 lines, 1 file))
The
timeZonefield is added as optional toTimeRange, but thedateOverridesparameter type incheckIfIsAvailable(viewer/slots.ts line 82-85) is defined inline as{ start: Date; end: Date; }[]without includingtimeZone. This means the type system does not enforce that timezone context is carried through tocheckIfIsAvailable, and the function cannot accesstimeZonefrom individual overrides even though it's available in the data.
| timeZone?: string; | |
| dateOverrides, // TimeRange already includes optional timeZone per override | |
| dateOverrides?: TimeRange[]; |
🤖 Grapple PR auto-fix • minor • confidence: 98%
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; |
There was a problem hiding this comment.
✅ [Grapple PR] Auto-fix — style agent (Small fix (4 lines, 1 file))
Object reference comparison used for Dayjs instances will always return false. The condition
dayjs(date.start).add(utcOffset, 'minutes') === dayjs(date.end).add(utcOffset, 'minutes')creates two separate Dayjs objects and compares them by reference, not by value. This zero-duration override detection will never work as intended.
| dateOverrideExist = true; | |
| if ( | |
| dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes")) | |
| ) { |
🤖 Grapple PR auto-fix • minor • confidence: 88%
| if ( | ||
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { | ||
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — security agent (Larger fix (13 lines, 1 file) — review recommended)
The working-hours check uses
slotStartTime.day()to get the day of week.slotStartTimeis a UTC Dayjs object (time.utc()). Calling.day()on a UTC object returns the UTC day of week, not the organizer's local day. For organizers in UTC+10, a slot at 23:00 UTC on Monday is actually Tuesday locally, but this code would check Monday's working hours.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -137,11 +137,19 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
- if (workingHour.days.includes(slotStartTime.day())) {
- const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ // Convert the UTC slot time to the organizer's local timezone so that
+ // day-of-week and time-of-day comparisons match how working hours were
+ // configured. Without this, a UTC+10 organizer's 23:00 UTC Monday slot
+ // would be checked against Monday's hours instead of Tuesday's.
+ const slotInOrganizerTz = organizerTimeZone
+ ? slotStartTime.tz(organizerTimeZone)
+ : slotStartTime;
+ if (workingHour.days.includes(slotInOrganizerTz.day())) {
+ const start = slotInOrganizerTz.hour() * 60 + slotInOrganizerTz.minute();
+ const end = slotInOrganizerTz.hour() * 60 + slotInOrganizerTz.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {🤖 Grapple PR auto-fix • major • Review this diff before applying
| dayjs(override.end).utc().add(offset, "minute").minute(), | ||
| }; | ||
| }); | ||
|
|
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — security agent (Larger fix (21 lines, 1 file) — review recommended)
The offset calculation in
getSlotsusesdayjs(override.start.toString()).tz(override.timeZone).utcOffset()— computing the UTC offset only at the override's start time. If a DST transition occurs within the override window (e.g., a 3-hour override spanning a clock change), the offset at the end of the window differs from the start, causing the endTime minute-of-day to be computed with the wrong offset.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -210,18 +210,22 @@ const getSlots = ({
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
- const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
- const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
- const offset = inviteeUtcOffset - organizerUtcOffset;
+ // Compute offsets independently at each boundary to handle DST transitions
+ // that may occur within the override window (e.g., a 3-hour override spanning
+ // a clock change where utcOffset() differs by 60 minutes at start vs. end).
+ const startOrganizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
+ const startInviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
+ const startOffset = startInviteeUtcOffset - startOrganizerUtcOffset;
+
+ const endOrganizerUtcOffset = dayjs(override.end.toString()).tz(override.timeZone).utcOffset();
+ const endInviteeUtcOffset = dayjs(override.end.toString()).tz(timeZone).utcOffset();
+ const endOffset = endInviteeUtcOffset - endOrganizerUtcOffset;
return {
userIds: override.userId ? [override.userId] : [],
startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
+ dayjs(override.start).utc().add(startOffset, "minute").hour() * 60 +
+ dayjs(override.start).utc().add(startOffset, "minute").minute(),
endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ dayjs(override.end).utc().add(endOffset, "minute").hour() * 60 +
+ dayjs(override.end).utc().add(endOffset, "minute").minute(),
};
});🤖 Grapple PR auto-fix • major • Review this diff before applying
| dayjs(override.end).utc().add(offset, "minute").minute(), | ||
| }; | ||
| }); | ||
|
|
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — security agent (Larger fix (26 lines, 1 file) — review recommended)
When the UTC offset delta causes an override's end time to cross midnight, the minute-of-day calculation using
.hour() * 60 + .minute()wraps to a small value (e.g., 00:30 → 30) instead of the expected value greater than 1440. This can make the override window appear inverted or empty, causing slots that should be included to be excluded and vice versa.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -208,22 +208,32 @@ const getSlots = ({
});
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;
+ const adjustedStart = dayjs(override.start).utc().add(offset, "minute");
+ const adjustedEnd = dayjs(override.end).utc().add(offset, "minute");
+
+ // If the UTC offset adjustment pushes the time past midnight, .hour() * 60 + .minute()
+ // wraps back to a small value (e.g., 01:00 → 60 instead of 1500). We detect day overflow
+ // by comparing the adjusted date's day-of-year to the original UTC date's day-of-year and
+ // add 1440 minutes per day crossed so that endTime > startTime always holds.
+ const startDayDelta =
+ adjustedStart.diff(dayjs(override.start).utc().startOf("day"), "day");
+ const endDayDelta =
+ adjustedEnd.diff(dayjs(override.end).utc().startOf("day"), "day");
+
return {
userIds: override.userId ? [override.userId] : [],
- startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
- endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ startTime:
+ adjustedStart.hour() * 60 +
+ adjustedStart.minute() +
+ startDayDelta * 1440,
+ endTime:
+ adjustedEnd.hour() * 60 +
+ adjustedEnd.minute() +
+ endDayDelta * 1440,
};
});🤖 Grapple PR auto-fix • major • Review this diff before applying
| // slot is not within the date override | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (11 lines, 1 file) — review recommended)
The working-hours day-of-week check uses
.day()on a UTC Dayjs object (slotStartTime), which returns the day-of-week in UTC. For organizers in timezones where the local calendar day differs from UTC (e.g., UTC+12 at 23:00 UTC on Monday = Tuesday local), this will check the wrong day's working hours.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -136,10 +136,14 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
- if (workingHour.days.includes(slotStartTime.day())) {
- const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ // Convert UTC slotStartTime to organizer's local time before checking day-of-week
+ // and time-of-day, since workingHours are defined in the organizer's timezone.
+ const localSlotStartTime = organizerTimeZone
+ ? slotStartTime.tz(organizerTimeZone)
+ : slotStartTime;
+ if (workingHour.days.includes(localSlotStartTime.day())) {
+ const start = localSlotStartTime.hour() * 60 + localSlotStartTime.minute();
+ const end = localSlotStartTime.hour() * 60 + localSlotStartTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}🤖 Grapple PR auto-fix • major • Review this diff before applying
| dayjs(override.start).utc().add(offset, "minute").hour() * 60 + | ||
| dayjs(override.start).utc().add(offset, "minute").minute(), | ||
| endTime: | ||
| dayjs(override.end).utc().add(offset, "minute").hour() * 60 + |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (22 lines, 1 file) — review recommended)
The offset calculation in
getSlotsuses onlyoverride.startto compute bothorganizerUtcOffsetandinviteeUtcOffset. If DST transitions occur between the override's start and end times (or between different override dates), the offset applied toendTimecould be incorrect, leading to slots being generated for the wrong time window.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -208,22 +208,27 @@ const getSlots = ({
});
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
- const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
- const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
- const offset = inviteeUtcOffset - organizerUtcOffset;
+ // Compute offsets independently at start and end instants to correctly handle
+ // DST transitions that may occur within the override window (e.g., a full-day
+ // override on a clock-change day). Using override.start for both offsets would
+ // apply the wrong UTC offset to endTime after a DST boundary is crossed.
+ const startOrganizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
+ const startInviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
+ const startOffset = startInviteeUtcOffset - startOrganizerUtcOffset;
+
+ const endOrganizerUtcOffset = dayjs(override.end.toString()).tz(override.timeZone).utcOffset();
+ const endInviteeUtcOffset = dayjs(override.end.toString()).tz(timeZone).utcOffset();
+ const endOffset = endInviteeUtcOffset - endOrganizerUtcOffset;
return {
userIds: override.userId ? [override.userId] : [],
startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
+ dayjs(override.start).utc().add(startOffset, "minute").hour() * 60 +
+ dayjs(override.start).utc().add(startOffset, "minute").minute(),
endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ dayjs(override.end).utc().add(endOffset, "minute").hour() * 60 +
+ dayjs(override.end).utc().add(endOffset, "minute").minute(),
};
});
// unset all working hours that relate to this user availability override
overrides.forEach((override) => {
let i = -1;🤖 Grapple PR auto-fix • major • Review this diff before applying
| dayjs(override.start).utc().add(offset, "minute").hour() * 60 + | ||
| dayjs(override.start).utc().add(offset, "minute").minute(), | ||
| endTime: | ||
| dayjs(override.end).utc().add(offset, "minute").hour() * 60 + |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (20 lines, 1 file) — review recommended)
When the offset delta shifts override start/end past midnight, the minute-of-day calculation (
hour * 60 + minute) wraps incorrectly. For example, if the override ends at 23:30 UTC and the offset is +60 minutes, the result is 00:30 the next day = 30 minutes, which is less thanstartTime, causing incorrect slot generation.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -208,22 +208,30 @@ const getSlots = ({
});
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;
+ const adjustedStart = dayjs(override.start).utc().add(offset, "minute");
+ const adjustedEnd = dayjs(override.end).utc().add(offset, "minute");
+
+ // Use startOfInviteeDay as the reference point so that times crossing
+ // midnight are represented as minutes since day-start (possibly > 1439
+ // for next-day overflow, or negative for previous-day underflow).
+ // This avoids the wrap-around bug where hour()*60+minute() gives a small
+ // value (e.g. 30) when the time has crossed past midnight, making
+ // endTime < startTime and producing zero slots.
+ const startTime = adjustedStart.diff(startOfInviteeDay, "minute");
+ const endTime = adjustedEnd.diff(startOfInviteeDay, "minute");
+
return {
userIds: override.userId ? [override.userId] : [],
- startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
- endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ startTime,
+ endTime,
};
});
// unset all working hours that relate to this user availability override
overrides.forEach((override) => {
let i = -1;🤖 Grapple PR auto-fix • major • Review this diff before applying
| dayjs(override.start).utc().add(offset, "minute").hour() * 60 + | ||
| dayjs(override.start).utc().add(offset, "minute").minute(), | ||
| endTime: | ||
| dayjs(override.end).utc().add(offset, "minute").hour() * 60 + |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (48 lines, 1 file) — review recommended)
The offset delta is applied to the override times and then
.hour() * 60 + .minute()is extracted, producing a minute-of-day value in [0, 1439]. If the offset pushes a time past midnight (e.g., an override ending at 23:00 UTC with a +120 minute offset becomes 01:00 the next day), the minute-of-day value wraps to a small number (60), which would be less thanstartTimeand produce an invalid/empty working hours range. There is no modular arithmetic or day-boundary handling.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -208,22 +208,36 @@ const getSlots = ({
});
if (!!activeOverrides.length) {
- const overrides = activeOverrides.flatMap((override) => {
- const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
- const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
- const offset = inviteeUtcOffset - organizerUtcOffset;
-
- return {
- userIds: override.userId ? [override.userId] : [],
- startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
- endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
- };
- });
-
+ const overrides = activeOverrides.flatMap((override) => {
+ // Convert the override's absolute UTC timestamps into the invitee's local
+ // timezone so that hour/minute extraction reflects the correct calendar day
+ // for the invitee. Using .tz(timeZone) instead of manual offset arithmetic
+ // correctly handles DST transitions and midnight-wrapping: if a 23:00 UTC
+ // override becomes 01:00 in the invitee's zone, the minute-of-day value
+ // (60) would be less than a startTime computed on the same day, producing
+ // an invalid range. We detect and discard such out-of-day overrides below.
+ const startInInviteeTz = dayjs(override.start).tz(timeZone);
+ const endInInviteeTz = dayjs(override.end).tz(timeZone);
+
+ const startTime = startInInviteeTz.hour() * 60 + startInInviteeTz.minute();
+ const endTime = endInInviteeTz.hour() * 60 + endInInviteeTz.minute();
+
+ // Guard: if the timezone conversion pushed the override entirely past
+ // midnight (end wraps to a smaller minute-of-day than start, or the
+ // converted date no longer falls on the invitee's requested day), the
+ // downstream slot generator would produce an empty or inverted range.
+ // Return an empty array so flatMap drops this override silently.
+ if (endTime <= startTime) {
+ return [];
+ }
+
+ return [
+ {
+ userIds: override.userId ? [override.userId] : [],
+ startTime,
+ endTime,
+ },
+ ];
+ });
+
// unset all working hours that relate to this user availability override
overrides.forEach((override) => {
let i = -1;🤖 Grapple PR auto-fix • major • Review this diff before applying
| const endCheckForAvailability = performance.now(); | ||
| checkForAvailabilityCount++; | ||
| checkForAvailabilityTime += endCheckForAvailability - startCheckForAvailability; | ||
| return isAvailable; |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (12 lines, 1 file) — review recommended)
At the call site on line 489,
checkIfIsAvailableis called with...schedulespread, but neitherdateOverridesnorworkingHoursappear to be properties ofschedule. The new parametersdateOverridesandworkingHoursdefault to empty arrays, meaning the date override and working hours validation logic will be entirely skipped at this call site.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -486,9 +486,11 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
const isAvailable = checkIfIsAvailable({
time: slot.time,
- ...schedule,
- ...availabilityCheckProps,
+ busy: schedule.busy,
+ dateOverrides: schedule.dateOverrides,
+ workingHours: schedule.workingHours,
+ ...availabilityCheckProps,
organizerTimeZone: schedule.timeZone,
});
const endCheckForAvailability = performance.now();
@@ -511,8 +513,10 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
return checkIfIsAvailable({
time: slot.time,
- ...userSchedule,
- ...availabilityCheckProps,
+ busy: userSchedule.busy,
+ dateOverrides: userSchedule.dateOverrides,
+ workingHours: userSchedule.workingHours,
+ ...availabilityCheckProps,
organizerTimeZone: userSchedule.timeZone,
});🤖 Grapple PR auto-fix • major • Review this diff before applying
| return true; | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — style agent (Larger fix (11 lines, 1 file) — review recommended)
The working-hours check uses
.day()on a UTC Dayjs object (slotStartTime.utc()), which returns the day-of-week in UTC. For users in timezones where the UTC calendar day differs from the local calendar day, this may validate against the wrong day's working hours.
--- a/packages/trpc/server/routers/viewer/slots.ts
+++ b/packages/trpc/server/routers/viewer/slots.ts
@@ -137,11 +137,13 @@ const checkIfIsAvailable = ({
//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
- if (workingHour.days.includes(slotStartTime.day())) {
- const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ // Convert to organizer's local time before comparing day-of-week and time-of-day,
+ // so that e.g. a UTC Monday slot that falls on Tuesday in UTC+8 is validated correctly.
+ const localSlotStartTime = organizerTimeZone
+ ? slotStartTime.tz(organizerTimeZone)
+ : slotStartTime;
+ if (workingHour.days.includes(localSlotStartTime.day())) {
+ const start = localSlotStartTime.hour() * 60 + localSlotStartTime.minute();
+ const end = localSlotStartTime.hour() * 60 + localSlotStartTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}🤖 Grapple PR auto-fix • minor • Review this diff before applying
| })); | ||
| const overrides = activeOverrides.flatMap((override) => { | ||
| const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset(); | ||
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — style agent (Larger fix (21 lines, 1 file) — review recommended)
The UTC offset is calculated only once from
override.start, but if a DST (Daylight Saving Time) transition occurs between the override's start and end times, the offset may differ. Using a single offset value could produce incorrect minute-of-day calculations for overrides that span a DST boundary.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -210,15 +210,20 @@ const getSlots = ({
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
- const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
- const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
- const offset = inviteeUtcOffset - organizerUtcOffset;
+ // Compute offsets independently for start and end to handle DST transitions:
+ // if a DST boundary falls within the override, the UTC offset may differ between
+ // the two endpoints, so we must not reuse the start offset for the end calculation.
+ const organizerStartOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
+ const inviteeStartOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
+ const startOffset = inviteeStartOffset - organizerStartOffset;
+
+ const organizerEndOffset = dayjs(override.end.toString()).tz(override.timeZone).utcOffset();
+ const inviteeEndOffset = dayjs(override.end.toString()).tz(timeZone).utcOffset();
+ const endOffset = inviteeEndOffset - organizerEndOffset;
return {
userIds: override.userId ? [override.userId] : [],
startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
+ dayjs(override.start).utc().add(startOffset, "minute").hour() * 60 +
+ dayjs(override.start).utc().add(startOffset, "minute").minute(),
endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ dayjs(override.end).utc().add(endOffset, "minute").hour() * 60 +
+ dayjs(override.end).utc().add(endOffset, "minute").minute(),
};
});🤖 Grapple PR auto-fix • minor • Review this diff before applying
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); | ||
| const offset = inviteeUtcOffset - organizerUtcOffset; | ||
|
|
||
| return { |
There was a problem hiding this comment.
🟡 [Grapple PR] Suggested fix — style agent (Larger fix (17 lines, 1 file) — review recommended)
Adding the offset delta and converting to minute-of-day does not use modular arithmetic. If the offset delta is large enough to shift a time past midnight, the minute-of-day calculation will wrap incorrectly (e.g., hour() will become negative or exceed 23). This is especially problematic for midnight-spanning overrides or large UTC offset differences.
--- a/packages/lib/slots.ts
+++ b/packages/lib/slots.ts
@@ -210,16 +210,22 @@ const getSlots = ({
if (!!activeOverrides.length) {
const overrides = activeOverrides.flatMap((override) => {
const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;
+ const adjustedStart = dayjs(override.start).utc().add(offset, "minute");
+ const adjustedEnd = dayjs(override.end).utc().add(offset, "minute");
+
+ // Use modular arithmetic to correctly handle times that cross midnight after
+ // the UTC offset is applied (e.g., 23:00 UTC + 120min offset = 01:00 next day).
+ // diff from startOf('day') gives true minute-of-day; % (24*60) keeps it in [0, 1440).
+ const startTime = Math.abs(adjustedStart.diff(adjustedStart.startOf("day"), "minute")) % (24 * 60);
+ const endTime = Math.abs(adjustedEnd.diff(adjustedEnd.startOf("day"), "minute")) % (24 * 60);
+
return {
userIds: override.userId ? [override.userId] : [],
- startTime:
- dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.start).utc().add(offset, "minute").minute(),
- endTime:
- dayjs(override.end).utc().add(offset, "minute").hour() * 60 +
- dayjs(override.end).utc().add(offset, "minute").minute(),
+ startTime,
+ endTime,
};
});🤖 Grapple PR auto-fix • minor • Review this diff before applying
2 participants
Benchmark test PR #7 - Replicating Greptile benchmark for Grapple PR evaluation.