Skip to content

fix(workflow): use local timezone for standup-report and weekly-digest boundaries #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
anshul-garg27 wants to merge 4 commits into from
Closed

fix(workflow): use local timezone for standup-report and weekly-digest boundaries #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b952502
fix(workflow): use local timezone for standup-report and weekly-diges…
anshul-garg27 Mar 13, 2026
ea4f202
refactor: extract local_today_boundaries() helper, use expect() over …
anshul-garg27 Mar 13, 2026
9434421
fix(test): use DST-safe range assertion for day span
anshul-garg27 Mar 13, 2026
e9c2c9f
chore: add changeset for workflow timezone fix
anshul-garg27 Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-workflow-timezone.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Use local timezone for standup-report and weekly-digest day boundaries
72 changes: 55 additions & 17 deletions src/helpers/workflows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,12 @@ async fn handle_standup_report(matches: &ArgMatches) -> Result<(), GwsError> {

let client = crate::client::build_client()?;

// Today's time range
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs();
let day_start = (now / 86400) * 86400;
let day_end = day_start + 86400;
let time_min = epoch_to_rfc3339(day_start);
let time_max = epoch_to_rfc3339(day_end);
// Today's time range using local timezone so boundaries align with the
// user's wall-clock day, not UTC midnight.
let (today_start_local, today_end_local) = local_today_boundaries();

let time_min = today_start_local.to_rfc3339();
let time_max = today_end_local.to_rfc3339();

// Fetch today's events
let events_json = get_json(
Expand Down Expand Up @@ -355,7 +352,7 @@ async fn handle_standup_report(matches: &ArgMatches) -> Result<(), GwsError> {
"meetingCount": meetings.len(),
"tasks": open_tasks,
"taskCount": open_tasks.len(),
"date": time_min.split('T').next().unwrap_or(""),
"date": today_start_local.format("%Y-%m-%d").to_string(),
});

format_and_print(&output, matches);
Expand Down Expand Up @@ -542,13 +539,11 @@ async fn handle_weekly_digest(matches: &ArgMatches) -> Result<(), GwsError> {

let client = crate::client::build_client()?;

let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs();
let week_end = now + 7 * 86400;
let time_min = epoch_to_rfc3339(now);
let time_max = epoch_to_rfc3339(week_end);
// Use local time so the period boundaries match the user's timezone.
let local_now = chrono::Local::now();
let week_end = local_now + chrono::Duration::days(7);
let time_min = local_now.to_rfc3339();
let time_max = week_end.to_rfc3339();

// Fetch this week's events
let events_json = get_json(
Expand Down Expand Up @@ -692,6 +687,25 @@ async fn handle_file_announce(matches: &ArgMatches) -> Result<(), GwsError> {
// Utilities
// ---------------------------------------------------------------------------

/// Returns (start_of_today, end_of_today) in the local timezone.
///
/// Uses `chrono::Local` to derive midnight boundaries from the user's
/// wall-clock time. Handles DST transitions via `.earliest()`.
fn local_today_boundaries() -> (chrono::DateTime<chrono::Local>, chrono::DateTime<chrono::Local>) {
use chrono::{Local, NaiveTime, TimeZone};

let local_now = Local::now();
let today_midnight = local_now
.date_naive()
.and_time(NaiveTime::from_hms_opt(0, 0, 0).unwrap());
let today_start = Local
.from_local_datetime(&today_midnight)
.earliest()
.expect("midnight should be representable in the local timezone");
let today_end = today_start + chrono::Duration::days(1);
(today_start, today_end)
}
Comment on lines +694 to +707
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The use of .expect() here can cause a panic. While rare, it's possible for midnight not to exist in a local timezone on a specific day due to events like switching timezones or extreme daylight saving time adjustments (e.g., a time jump from 23:59 to 01:00). If from_local_datetime results in a LocalResult::None, earliest() will return None, and expect() will panic, crashing the program.

To make this function more robust, it should return a Result and propagate the error to the caller. The suggested change refactors the function to do this.

This will require updating the call site in handle_standup_report to use ? (i.e., let (today_start_local, today_end_local) = local_today_boundaries()?;). Additionally, the new test test_local_today_boundaries will need to be updated to handle the Result as well (e.g., by using .unwrap() since a panic is acceptable in a test context).

fn local_today_boundaries() -> Result<(chrono::DateTime<chrono::Local>, chrono::DateTime<chrono::Local>), GwsError> {
    use chrono::{Local, NaiveTime, TimeZone};

    let local_now = Local::now();
    let today_midnight = local_now
        .date_naive()
        .and_time(NaiveTime::from_hms_opt(0, 0, 0).unwrap());
    let today_start = Local
        .from_local_datetime(&today_midnight)
        .earliest()
        .ok_or_else(|| {
            GwsError::Other(anyhow::anyhow!(
                "Could not determine start of today: midnight does not exist in local timezone for current date"
            ))
        })?;
    let today_end = today_start + chrono::Duration::days(1);
    Ok((today_start, today_end))
}

Copy link
Copy Markdown
Contributor Author

@anshul-garg27 anshul-garg27 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch — switched from .expect() to .unwrap_or() with local_now as fallback, same pattern as the calendar fix in #443

Comment on lines +694 to +707
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Using .expect() here can cause a panic if the program is run on a day in a timezone where midnight doesn't exist (e.g., due to a DST transition). While rare, this would crash the CLI.

To make this more robust, I suggest changing this function to return a Result and gracefully handle the case where earliest() returns None.

You will also need to update the call site in handle_standup_report (line 280) to handle the Result (e.g., with ?) and update the test test_local_today_boundaries (line 751) to unwrap() the result.

fn local_today_boundaries() -> Result<(chrono::DateTime<chrono::Local>, chrono::DateTime<chrono::Local>), GwsError> {
    use chrono::{Local, NaiveTime, TimeZone};

    let local_now = Local::now();
    let today_midnight = local_now
        .date_naive()
        .and_time(NaiveTime::from_hms_opt(0, 0, 0).unwrap());
    let today_start = Local
        .from_local_datetime(&today_midnight)
        .earliest()
        .ok_or_else(|| {
            GwsError::Other(anyhow::anyhow!(
                "Could not determine start of day in local timezone: midnight does not exist on this day"
            ))
        })?;
    let today_end = today_start + chrono::Duration::days(1);
    Ok((today_start, today_end))
}

Copy link
Copy Markdown
Contributor Author

@anshul-garg27 anshul-garg27 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, using .unwrap_or(local_now) now so it won't panic on weird DST days


fn epoch_to_rfc3339(epoch: u64) -> String {
use chrono::{TimeZone, Utc};
Utc.timestamp_opt(epoch as i64, 0).unwrap().to_rfc3339()
Expand Down Expand Up @@ -729,6 +743,30 @@ mod tests {
assert_eq!(epoch_to_rfc3339(1710000000), "2024-03-09T16:00:00+00:00");
}

#[test]
fn test_local_today_boundaries() {
// Verify that the shared helper produces boundaries that
// contain "now" and span ~24 hours.
let local_now = chrono::Local::now();
let (start, end) = local_today_boundaries();

// "now" must fall within [start, end)
assert!(local_now >= start);
assert!(local_now < end);

// The span is 24h (86400s) on most days, but 23h or 25h on DST
// transition days. Accept a range to avoid flaky tests.
let span = end.signed_duration_since(start).num_seconds();
assert!(
(82800..=90000).contains(&span),
"span {span}s is outside the expected 23h–25h range"
);

// The RFC 3339 output must include the local offset
let rfc = start.to_rfc3339();
assert!(rfc.contains('T'));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This assertion is correct, but it doesn't fully test the requirement from the comment on line 765: "The RFC 3339 output must include the local offset". The 'T' separator is always present in a DateTime's RFC 3339 representation and doesn't validate the offset.

A more robust test would be to ensure the generated RFC 3339 string can be parsed back into a DateTime object that represents the same instant in time. This implicitly validates that the timezone offset was serialized correctly.

        // Verify that the string can be parsed back to the same instant in time,
        // which implicitly checks that the timezone offset was correctly serialized.
        let parsed = chrono::DateTime::parse_from_rfc3339(&rfc).unwrap();
        assert_eq!(start, parsed);

Copy link
Copy Markdown
Contributor Author

@anshul-garg27 anshul-garg27 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, checking for 'T' doesn't prove much. updated to assert the local offset string appears in the output instead

}

#[test]
fn test_build_standup_report_cmd() {
let cmd = build_standup_report_cmd();
Expand Down
Loading