Repository navigation
Add minute-aware scheduling durations for Event and Meeting flows - #264
Conversation
Implement pm-zoe4 by accepting `30min` and ISO-8601 `PT...` duration forms in event duration parsing for create/update and meet/event shortcuts, while preserving legacy bare `m` month semantics for backward compatibility. Sync help/contracts/docs and expand regression coverage so the project remains at 100/100/100/100.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
More reviews will be available in 14 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds sub-hour duration parsing ( ChangesSub-hour event duration parsing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Update the contracts fixture after adding reminder/event parameter metadata so contracts:check stays green and the PR remains CI-ready.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for sub-hour duration granularities, such as explicit minute forms (e.g., '30min') and ISO-8601 durations (e.g., 'PT30M'), in scheduling commands and event options. It ensures backward compatibility by keeping bare 'm' as months. The implementation updates the duration resolution logic, CLI help content, and parameter contracts, and is supported by comprehensive unit and integration tests. As there are no review comments, I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces sub-hour duration granularity for scheduling by supporting explicit minute forms (such as '30min' or 'minutes') and ISO-8601 durations (such as 'PT30M') for event durations, while preserving legacy behavior where bare 'm' represents months. It also updates CLI help documentation, SDK option contracts, and adds extensive unit tests. The review feedback correctly points out that using a fixed millisecond value for days in ISO-8601 durations can fail during Daylight Saving Time (DST) transitions, and suggests using calendar-based date arithmetic to resolve this issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const MILLIS_PER_SECOND = 1000; | ||
| const MILLIS_PER_MINUTE = 60 * MILLIS_PER_SECOND; | ||
| const MILLIS_PER_HOUR = 60 * MILLIS_PER_MINUTE; | ||
| const MILLIS_PER_DAY = 24 * MILLIS_PER_HOUR; |
There was a problem hiding this comment.
This constant is no longer needed if we use calendar-based date arithmetic for the days component.
| const days = Number.parseInt(daysToken ?? "0", 10); | ||
| const hours = Number.parseInt(hoursToken ?? "0", 10); | ||
| const minutes = Number.parseInt(minutesToken ?? "0", 10); | ||
| const seconds = Number.parseInt(secondsToken ?? "0", 10); | ||
| const totalMillis = | ||
| days * MILLIS_PER_DAY + hours * MILLIS_PER_HOUR + minutes * MILLIS_PER_MINUTE + seconds * MILLIS_PER_SECOND; | ||
| return new Date(start.getTime() + sign * totalMillis).toISOString(); |
There was a problem hiding this comment.
When parsing ISO-8601 durations that include days (D), calculating the end date by multiplying the number of days by a fixed millisecond value (MILLIS_PER_DAY) can lead to incorrect results during Daylight Saving Time (DST) transitions (where a day may be 23 or 25 hours instead of 24).
Using calendar-based date arithmetic for the days component, and then adding the remaining time components (hours, minutes, seconds) as milliseconds, ensures correctness across DST boundaries.
const days = Number.parseInt(daysToken ?? "0", 10);
const hours = Number.parseInt(hoursToken ?? "0", 10);
const minutes = Number.parseInt(minutesToken ?? "0", 10);
const seconds = Number.parseInt(secondsToken ?? "0", 10);
const end = new Date(start.getTime());
end.setDate(end.getDate() + sign * days);
const totalMillis =
hours * MILLIS_PER_HOUR + minutes * MILLIS_PER_MINUTE + seconds * MILLIS_PER_SECOND;
return new Date(end.getTime() + sign * totalMillis).toISOString();Address review feedback by constraining ISO duration parsing for event.duration to PT time components (H/M/S only), keeping day semantics and DST-sensitive expectations out of this feature scope. Add regression coverage that P1D is rejected and keep full coverage at 100/100/100/100.
|
Follow-up push applied (ISO PT duration scope refinement per review feedback). @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements sub-hour duration granularity for scheduling by adding support for minutes in event durations (e.g., 30min, PT30M) while preserving backward compatibility where a bare m remains interpreted as months. The changes update the duration parsing logic, command-line help content, SDK option descriptions, and parameter tables, and include comprehensive unit test coverage. There are no review comments to address, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Add rmSync retry options in contracts-snapshot cleanup so transient ENOTEMPTY temp-directory races on GitHub runners do not fail the static gate.
|
Follow-up push applied for CI static-gate reliability fix. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces sub-hour duration granularity for scheduling by supporting explicit minute forms (e.g., '30min', 'PT30M') in event durations, while preserving legacy behavior where bare 'm' represents months. It also updates documentation, help content, and CLI contracts, and adds robust test coverage. Feedback on the changes highlights a potential crash when parsing extremely large duration values, where calling .toISOString() on an invalid date throws a RangeError; validating the date object before serialization is recommended to prevent unhandled exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function resolveDurationAgainstStart(startAt: string, durationRaw: string): string { | ||
| const start = new Date(startAt); | ||
| const trimmedDuration = durationRaw.trim(); | ||
|
|
||
| const minuteDuration = MINUTE_DURATION.exec(trimmedDuration); | ||
| if (minuteDuration) { | ||
| const sign = minuteDuration[1] === "-" ? -1 : 1; | ||
| const amount = Number.parseInt(minuteDuration[2], 10) * sign; | ||
| return new Date(start.getTime() + amount * MILLIS_PER_MINUTE).toISOString(); | ||
| } | ||
|
|
||
| const isoDuration = ISO_8601_TIME_DURATION.exec(trimmedDuration); | ||
| if (isoDuration) { | ||
| const [, signToken, hoursToken, minutesToken, secondsToken] = isoDuration; | ||
| const hasComponent = hoursToken || minutesToken || secondsToken; | ||
| if (hasComponent) { | ||
| const sign = signToken === "-" ? -1 : 1; | ||
| const hours = Number.parseInt(hoursToken ?? "0", 10); | ||
| const minutes = Number.parseInt(minutesToken ?? "0", 10); | ||
| const seconds = Number.parseInt(secondsToken ?? "0", 10); | ||
| const totalMillis = hours * MILLIS_PER_HOUR + minutes * MILLIS_PER_MINUTE + seconds * MILLIS_PER_SECOND; | ||
| return new Date(start.getTime() + sign * totalMillis).toISOString(); | ||
| } | ||
| } | ||
|
|
||
| const normalizedDuration = | ||
| trimmedDuration.startsWith("+") || trimmedDuration.startsWith("-") ? trimmedDuration : `+${trimmedDuration}`; | ||
| return resolveIsoOrRelative(normalizedDuration, start, "event.duration"); | ||
| } |
There was a problem hiding this comment.
If the user inputs an extremely large duration value (e.g., '99999999999999999999min' or 'PT99999999999999999999H'), parsing it can result in an invalid date. Calling '.toISOString()' on an invalid date throws a 'RangeError: Invalid time value', which causes an unhandled exception and crashes the CLI.
We should validate that the resulting date is valid before calling '.toISOString()', and throw a user-friendly 'PmCliError' instead.
function resolveDurationAgainstStart(startAt: string, durationRaw: string): string {
const start = new Date(startAt);
const trimmedDuration = durationRaw.trim();
let end: Date | undefined;
const minuteDuration = MINUTE_DURATION.exec(trimmedDuration);
if (minuteDuration) {
const sign = minuteDuration[1] === "-" ? -1 : 1;
const amount = Number.parseInt(minuteDuration[2], 10) * sign;
end = new Date(start.getTime() + amount * MILLIS_PER_MINUTE);
} else {
const isoDuration = ISO_8601_TIME_DURATION.exec(trimmedDuration);
if (isoDuration) {
const [, signToken, hoursToken, minutesToken, secondsToken] = isoDuration;
const hasComponent = hoursToken || minutesToken || secondsToken;
if (hasComponent) {
const sign = signToken === "-" ? -1 : 1;
const hours = Number.parseInt(hoursToken ?? "0", 10);
const minutes = Number.parseInt(minutesToken ?? "0", 10);
const seconds = Number.parseInt(secondsToken ?? "0", 10);
const totalMillis = hours * MILLIS_PER_HOUR + minutes * MILLIS_PER_MINUTE + seconds * MILLIS_PER_SECOND;
end = new Date(start.getTime() + sign * totalMillis);
}
}
}
if (end) {
if (Number.isNaN(end.getTime())) {
throw new PmCliError("Invalid event.duration value", EXIT_CODE.USAGE);
}
return end.toISOString();
}
const normalizedDuration =
trimmedDuration.startsWith("+") || trimmedDuration.startsWith("-") ? trimmedDuration : "+" + trimmedDuration;
return resolveIsoOrRelative(normalizedDuration, start, "event.duration");
}Add a safe ISO conversion guard for duration-derived timestamps so out-of-range minute/PT durations fail with a usage error instead of throwing RangeError, and cover the behavior with regression tests.
|
Follow-up push applied for duration overflow guard and regression tests. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces sub-hour duration granularity for scheduling by supporting minute-based forms (such as 30min and ISO-8601 PT30M) in --duration and event duration= fields, while preserving backward compatibility where bare m remains interpreted as months. The changes update validation logic, CLI help content, SDK contracts, and add extensive unit tests to cover the new parsing behavior, overflow limits, and edge cases. Additionally, the cleanup process in scripts/contracts-snapshot.mjs is hardened with retries to prevent ENOTEMPTY CI flakes. Feedback is provided to rename toIsoDurationOrThrow to toIsoTimestampOrThrow to better reflect that it returns an ISO timestamp rather than a duration string.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function toIsoDurationOrThrow(timestampMs: number, durationRaw: string): string { | ||
| const resolved = new Date(timestampMs); | ||
| if (!Number.isFinite(resolved.getTime())) { | ||
| throw new PmCliError( | ||
| `Invalid event.duration value "${durationRaw}". Duration is out of supported timestamp range.`, | ||
| EXIT_CODE.USAGE, | ||
| ); | ||
| } | ||
| return resolved.toISOString(); | ||
| } |
There was a problem hiding this comment.
The function toIsoDurationOrThrow actually returns an ISO 8601 timestamp (date string) representing the resolved end date, not an ISO 8601 duration string (like PT30M). To prevent confusion and improve maintainability, consider renaming it to toIsoTimestampOrThrow or toIsoStringOrThrow. Remember to also update its callers on lines 36 and 49.
| function toIsoDurationOrThrow(timestampMs: number, durationRaw: string): string { | |
| const resolved = new Date(timestampMs); | |
| if (!Number.isFinite(resolved.getTime())) { | |
| throw new PmCliError( | |
| `Invalid event.duration value "${durationRaw}". Duration is out of supported timestamp range.`, | |
| EXIT_CODE.USAGE, | |
| ); | |
| } | |
| return resolved.toISOString(); | |
| } | |
| function toIsoTimestampOrThrow(timestampMs: number, durationRaw: string): string { | |
| const resolved = new Date(timestampMs); | |
| if (!Number.isFinite(resolved.getTime())) { | |
| throw new PmCliError( | |
| "Invalid event.duration value \"" + durationRaw + "\". Duration is out of supported timestamp range.", | |
| EXIT_CODE.USAGE | |
| ); | |
| } | |
| return resolved.toISOString(); | |
| } |
Rename toIsoDurationOrThrow to toIsoTimestampOrThrow so the helper name matches its output, while preserving behavior and tests.
|
Follow-up push applied for naming clarity polish. @codex review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements sub-hour duration granularity for scheduling by adding support for minutes in event durations (such as '30min' and 'PT30M') while preserving backward compatibility where a bare 'm' represents months. The changes span CLI commands, help content, SDK contracts, and validation logic, backed by comprehensive unit tests. Additionally, the CI cleanup process in contracts-snapshot.mjs was hardened with retries to prevent ENOTEMPTY flakes. There are no review comments, and I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Record merged evidence by closing pm-zoe4 with the final outcome and releasing assignment ownership after PR #264 landed on main.
Summary
pm-zoe4by extending event duration parsing to accept explicit minute forms (30min|mins|minute|minutes) and ISO-8601 durations (PT30M,PT1H, etc.) in create/update + scheduling shortcutsmremains months for duration tokens (45mstill means 45 months)Test plan
pnpm buildnode scripts/run-tests.mjs test -- tests/unit/core/item/parse-utils.spec.ts tests/unit/commands/create-command.spec.ts tests/unit/commands/update-command.spec.ts tests/unit/commands/scheduling-shortcuts.spec.ts tests/unit/commands/register-commands.spec.tsnode scripts/run-tests.mjs coverage(100/100/100/100)node dist/cli.js --path <temp>/.agents/pm:meet --duration 30min-> +30 minutesevent --duration PT30M-> +30 minutesevent --duration 45m-> month-compat behavior retainedupdate --event ...duration=30minparity checkvalidate --check-resolution --check-history-driftandhealth --check-only