From 881e7f0be93f235d07eb75813c2c598786d06fcf Mon Sep 17 00:00:00 2001 From: EugeneTes Date: Sat, 26 Sep 2026 20:30:56 +0200 Subject: [PATCH] fix(google-calendar): address review findings - log an unknown stored zone once even when the tool description reads it first - serialise the flow's token write with /calendar_disconnect under the flow gate - a cancelled or expired pending flow is cleared, not re-sent or shown - chat keys anchor with \z so a trailing newline is hashed - a timeMax date includes that whole day (spec updated) - dispose the flow's CancellationTokenSource, drop ConnectFlow.Token, Begin is a plain write - GoogleCalendarOptions.IsConfigured replaces the host's copy of the rule; StoredToken is internal - test BotSetupService's menu through DI, tokens absent from tool results - Telegram.Tests csproj reference placed as in telegram-message-context; ChatHost passes calendar by name - docs: per-event timeZone, both config sources for /calendar_* --- docs/google-calendar.md | 8 +++- .../2026-09-26-google-calendar-design.md | 13 +++--- docs/telegram-bot.md | 2 +- .../CalendarCommands.cs | 20 ++++++--- .../CalendarTime.cs | 10 +++-- .../CalendarTool.cs | 6 +-- .../ChatKeys.cs | 2 +- .../ConnectFlow.cs | 14 ++++++- .../GoogleCalendarConnections.cs | 30 +++++++++---- .../GoogleCalendarExtension.cs | 4 +- .../GoogleCalendarOptions.cs | 3 ++ .../GoogleTokenStore.cs | 2 +- .../GoogleCalendarSetup.cs | 3 +- .../GoogleCalendar/CalendarCommandsTests.cs | 28 +++++++++++++ .../GoogleCalendar/CalendarTimeTests.cs | 8 ++++ .../GoogleCalendar/CalendarToolReadTests.cs | 16 +++++++ .../GoogleCalendar/ChatKeysTests.cs | 1 + .../GoogleCalendar/ConnectFlowTests.cs | 42 +++++++++++++++++++ .../GoogleCalendarExtensionTests.cs | 30 +++++++++++++ .../GoogleCalendarHostTests.cs | 32 ++++++++++++++ .../Support/ChatHost.cs | 2 +- ...Volcanic.AgenticCore.Telegram.Tests.csproj | 2 +- 22 files changed, 242 insertions(+), 36 deletions(-) diff --git a/docs/google-calendar.md b/docs/google-calendar.md index 184656c..aebe6e9 100644 --- a/docs/google-calendar.md +++ b/docs/google-calendar.md @@ -123,14 +123,18 @@ The refresh token never appears in logs, tool results or error messages. ## Time zones -- Times without an offset (`2026-09-30T10:00`) are in the calendar's own time zone, which the bot reads when the chat - connects. If that's unknown, they are in the chat's zone. +- `create` and `update` take an optional `timeZone` (an IANA id such as `Europe/Berlin`) for the event. Times without + an offset are then in that zone. +- Otherwise times without an offset (`2026-09-30T10:00`) are in the calendar's own time zone, which the bot reads when + the chat connects. If that's unknown, they are in the chat's zone. - A time with an offset (`2026-09-30T10:00:00+02:00`) keeps its offset. - Every tool result starts with the zone it uses, such as `Times in Europe/Berlin.` When the calendar's zone differs from the chat's, the header names both: `Times in America/New_York (the calendar's zone; this chat is in Europe/Berlin).` - A date alone (`2026-09-30`) is an all-day event. All-day ends are exclusive, as in Google: an event on the 30th ends on the 1st. +- In a `list` or `search` window, a `timeMax` date includes that day: `timeMin` and `timeMax` both `2026-09-30` cover + the whole 30th. - For recurring events, `list_instances` returns the ids of single occurrences. `update` and `delete` detect whether an id is one occurrence or a whole series, so `scope` (`instance` or `series`) only needs to be passed to be explicit. "This and following" edits aren't supported. diff --git a/docs/superpowers/specs/2026-09-26-google-calendar-design.md b/docs/superpowers/specs/2026-09-26-google-calendar-design.md index ca39fc5..1ac54c9 100644 --- a/docs/superpowers/specs/2026-09-26-google-calendar-design.md +++ b/docs/superpowers/specs/2026-09-26-google-calendar-design.md @@ -161,14 +161,15 @@ The handler (`ConnectCommand`) with `ctx.Ui`: does) and stop. 2. A flow is already pending: re-send its link and code (`Notify`) with the remaining minutes, and stop. It does not start a second flow: a new device code would invalidate the one the user may be typing. A flow pending in another - chat doesn't matter. + chat doesn't matter. A flow that is already cancelled or whose code has expired (still parked in its delay) doesn't + count: it is cancelled silently, cleared, and a new flow starts; `/calendar_status` doesn't show it either. 3. Otherwise `StartDeviceFlowAsync`, store it as the chat connection's `Pending`, and send **one** message: `ui.Notify("Connect Google Calendar:\n1. Open \n2. Enter the code \n3. Approve access. I'll confirm here when it's done (code valid for min).")`. Link and code in one message, for the ordering reason in the re-login spec. 4. Poll with `PollAsync` every `Interval` seconds (Google says 5), delays through `TimeProvider`: - `Pending`: keep going. `SlowDown`: add 5 s to the interval permanently (RFC 8628 ยง3.5). - `Success`: call `GET calendars/primary` with the new token to learn `account` (`id`) and `timeZone`, write the - store, `Invalidate()` the cache, then `Notify("โœ… Google Calendar connected as (time zone ).")`. If the + store (under the connection's flow gate, and only if the flow wasn't cancelled meanwhile), `Invalidate()` the cache, then `Notify("โœ… Google Calendar connected as (time zone ).")`. If the lookup fails, the token is still saved with those fields empty. - `Denied`: `Notify("Google Calendar access was denied.", Warning)`. - `Expired`, or `ExpiresAt` passed: `Notify("The Google code expired. Run /calendar_connect again.", Warning)`. @@ -189,8 +190,9 @@ success; the old one is not revoked automatically. - `/calendar_status`: `Connected as since , time zone .`, `Not connected. Run /calendar_connect.`, or `Waiting for approval: open and enter ( min left).` -- `/calendar_disconnect`: cancels a pending flow, revokes the refresh token (best effort), deletes the file, - invalidates the cache, then `Calendar disconnected.` (with ` Revoking at Google failed: ; remove the app at +- `/calendar_disconnect`: under the same flow gate a completing flow writes the store with, cancels a pending flow, + revokes the refresh token (best effort), deletes the file, invalidates the cache (so a flow that got its token just + before the disconnect can't write it afterwards), then `Calendar disconnected.` (with ` Revoking at Google failed: ; remove the app at https://myaccount.google.com/permissions.` appended on revoke failure). The replies go through `ctx.Ui.Notify`. Without a UI they're appended as a `CustomMessage` with `triggerTurn: false`. @@ -258,7 +260,8 @@ Behaviour follows the reference's recurring-events spec: - A missing timed `end` on create means start + 1 hour. `start` and `end` must be the same kind (both dates or both date-times). - `timeMin`/`timeMax` must be RFC 3339 with an offset for Google. A date or local date-time is converted in `Z` - (`TimeZoneInfo`) before sending. + (`TimeZoneInfo`) before sending. A `timeMin` date is that day's midnight; a `timeMax` date includes that day, so it + becomes the next day's midnight (`timeMin=timeMax=2026-09-30` covers the whole 30th). - Output times are rendered in the resolved zone with the zone in the header line of every result, e.g. `Times in Europe/Berlin.` When the calendar's zone differs from the chat's (the system prompt shows the chat's), the header says both: `Times in America/New_York (the calendar's zone; this chat is in Europe/Berlin).`, so the model diff --git a/docs/telegram-bot.md b/docs/telegram-bot.md index 051e82a..4755045 100644 --- a/docs/telegram-bot.md +++ b/docs/telegram-bot.md @@ -75,7 +75,7 @@ Any other `/command` is passed to the session, so extension commands such as `/c `/claude-code-login` work. When an extension asks something, the question appears with buttons (or as a reply prompt for free text). -The `/calendar_*` commands exist only when `GOOGLE_CLIENT_ID` and `GOOGLE_CLIENT_SECRET` are set. +The `/calendar_*` commands exist only when both the Google client id and secret are set (`GOOGLE_CLIENT_ID`/`GOOGLE_CLIENT_SECRET` or the `GoogleCalendar` section). If the Claude subscription session behind the bot expires, the **first id in `AllowedUserIds`** (the bot owner) gets the re-login link and code prompt in their own private chat, regardless of which chat's request triggered diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarCommands.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarCommands.cs index 7e86eda..60bac5e 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarCommands.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarCommands.cs @@ -68,11 +68,21 @@ internal static class DisconnectCommand public static CommandDefinition Create(IExtensionApi api, GoogleCalendarExtension extension) => new(Description, async (_, ctx, ct) => { var connection = extension.ConnectionFor(ctx.SessionManager.Scope); - connection.CancelPending(); - var token = connection.Store.Read(); - var revokeError = token is null ? null : await connection.OAuth.RevokeAsync(token.RefreshToken, ct).ConfigureAwait(false); - connection.Store.Delete(); - connection.Invalidate(); + string? revokeError; + // The gate a completing connect flow writes its token under: after CancelPending it can no longer write. + await connection.FlowGate.WaitAsync(ct).ConfigureAwait(false); + try + { + connection.CancelPending(); + var token = connection.Store.Read(); + revokeError = token is null ? null : await connection.OAuth.RevokeAsync(token.RefreshToken, ct).ConfigureAwait(false); + connection.Store.Delete(); + connection.Invalidate(); + } + finally + { + connection.FlowGate.Release(); + } var text = "Calendar disconnected."; if (revokeError is not null) text += $" Revoking at Google failed: {revokeError}; remove the app at {GoogleCalendarMessages.PermissionsUrl}."; diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTime.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTime.cs index c6d7ddc..54ce164 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTime.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTime.cs @@ -65,13 +65,17 @@ internal static class CalendarTime return (s is { } a ? ToEventTime(a, zoneId) : null, e is { } b ? ToEventTime(b, zoneId) : null); } - /// A query bound (timeMin/timeMax) as RFC 3339 with an offset; dates and local times are in . - public static string ToRfc3339(string text, TimeZoneInfo zone, string field) + /// + /// A query bound (timeMin/timeMax) as RFC 3339 with an offset; dates and local times are in + /// . A date is its midnight, or with (timeMax) + /// the next day's midnight, so timeMin=timeMax=2026-09-30 covers the whole 30th. + /// + public static string ToRfc3339(string text, TimeZoneInfo zone, string field, bool dateIsInclusiveEnd = false) { var value = Parse(text, field); return value.Kind switch { - CalendarTimeKind.Date => InZone(value.Date.ToDateTime(TimeOnly.MinValue), zone), + CalendarTimeKind.Date => InZone((dateIsInclusiveEnd ? value.Date.AddDays(1) : value.Date).ToDateTime(TimeOnly.MinValue), zone), CalendarTimeKind.Local => InZone(value.Local, zone), _ => value.Instant.ToString(OffsetFormat, Inv), }; diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTool.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTool.cs index 519dfd9..470e9fe 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTool.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/CalendarTool.cs @@ -12,7 +12,7 @@ public sealed record CalendarArgs( [property: Description("list, search, get, list_instances, create, update, delete or calendars")] string Action, [property: Description("Calendar id; default primary. action=calendars lists the others")] string? CalendarId = null, [property: Description("Window start (list, search, list_instances): RFC 3339, a local date-time or a date")] string? TimeMin = null, - [property: Description("Window end, same forms as timeMin")] string? TimeMax = null, + [property: Description("Window end, same forms as timeMin; a date includes that whole day")] string? TimeMax = null, [property: Description("Free-text search (search only, required there)")] string? Query = null, [property: Description("Maximum events to return; default 25, at most 250")] int? MaxResults = null, [property: Description("list only: occurrences (default; every occurrence of recurring events) or series (recurring events once, with their RRULE)")] string? ListMode = null, @@ -88,7 +88,7 @@ internal sealed partial class CalendarTool(GoogleCalendarSettings settings, Goog public string EventZoneName => ZoneIds.Name(EventZone); public int Max => Math.Clamp(Args.MaxResults ?? DefaultMax, 1, MaxCap); public string Bound(string? given, DateTimeOffset fallback, string field) => - given is { Length: > 0 } ? CalendarTime.ToRfc3339(given, EventZone, field) : CalendarTime.Utc(fallback); + given is { Length: > 0 } ? CalendarTime.ToRfc3339(given, EventZone, field, dateIsInclusiveEnd: field == "timeMax") : CalendarTime.Utc(fallback); } private async Task ListAsync(Run run) @@ -135,7 +135,7 @@ internal sealed partial class CalendarTool(GoogleCalendarSettings settings, Goog var id = Require(run.Args.EventId, "list_instances needs eventId."); var query = new List>(); if (run.Args.TimeMin is { Length: > 0 } min) query.Add(new("timeMin", CalendarTime.ToRfc3339(min, run.EventZone, "timeMin"))); - if (run.Args.TimeMax is { Length: > 0 } max) query.Add(new("timeMax", CalendarTime.ToRfc3339(max, run.EventZone, "timeMax"))); + if (run.Args.TimeMax is { Length: > 0 } max) query.Add(new("timeMax", CalendarTime.ToRfc3339(max, run.EventZone, "timeMax", dateIsInclusiveEnd: true))); var items = await PageAsync(run, GoogleCalendarApi.Event(run.CalendarId, id) + "/instances", query, new CalendarCall("list_instances", run.CalendarId, id)).ConfigureAwait(false); return Events("list_instances", run, items); } diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ChatKeys.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ChatKeys.cs index a5fc309..d43fb69 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ChatKeys.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ChatKeys.cs @@ -14,6 +14,6 @@ internal static partial class ChatKeys return "scope-" + Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(scope)))[..16]; } - [GeneratedRegex("^[A-Za-z0-9._-]{1,100}$")] + [GeneratedRegex(@"^[A-Za-z0-9._-]{1,100}\z")] // \z, not $: '$' also matches before a trailing newline private static partial Regex Safe(); } diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ConnectFlow.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ConnectFlow.cs index 072a6e2..5f86949 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ConnectFlow.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/ConnectFlow.cs @@ -24,7 +24,9 @@ internal sealed class ConnectFlow public DeviceAuthorization Device { get; } public TimeSpan Interval { get; private set; } - public CancellationToken Token => _cancellation.Token; + + /// False once the flow was cancelled or its code expired: such a flow is never re-sent or shown. + public bool IsLive(TimeProvider time) => !_cancellation.IsCancellationRequested && time.GetUtcNow() < Device.ExpiresAt; /// How many poll delays the flow has entered (tests drive the fake clock with it). internal int Parks => Volatile.Read(ref _parks); @@ -137,6 +139,7 @@ internal sealed class ConnectFlow finally { _connection.End(this); + _cancellation.Dispose(); // a late Cancel() swallows ObjectDisposedException } } @@ -152,16 +155,23 @@ internal sealed class ConnectFlow { logger.LogWarning("google-calendar: looking up the primary calendar failed: {Error}", ex.Message); } + // Under the gate /calendar_disconnect also takes: once it has cancelled the flow, the token is never written. + await _connection.FlowGate.WaitAsync(ct).ConfigureAwait(false); try { + ct.ThrowIfCancellationRequested(); _connection.Store.Write(new StoredToken(result.RefreshToken!, result.Scope, primary.Account, primary.TimeZone, _connection.Settings.Time.GetUtcNow())); + _connection.Invalidate(); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { ui.Notify($"Got a Google token but could not save it: {ex.Message}", NotifyKind.Error); return; } - _connection.Invalidate(); + finally + { + _connection.FlowGate.Release(); + } ui.Notify($"โœ… Google Calendar connected as {primary.Account ?? "an unknown account"} (time zone {primary.TimeZone ?? "unknown"})."); } } diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarConnections.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarConnections.cs index 4bd6675..0b064de 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarConnections.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarConnections.cs @@ -47,20 +47,33 @@ internal sealed class GoogleCalendarConnection public GoogleTokenStore Store { get; } public GoogleOAuthClient OAuth { get; } - /// Serialises "is a flow pending? else start one" so two /calendar_connect in one chat can't both start. + /// + /// Serialises "is a flow pending? else start one" so two /calendar_connect in one chat can't both start, and the + /// flow's final token write with /calendar_disconnect so a disconnected chat can't end up connected. + /// public SemaphoreSlim FlowGate { get; } = new(1, 1); - /// This chat's in-flight device flow, if any. - public ConnectFlow? Pending => Volatile.Read(ref _pending); + /// This chat's in-flight device flow, if any; a cancelled or expired one is cancelled, cleared and not returned. + public ConnectFlow? Pending + { + get + { + var flow = Volatile.Read(ref _pending); + if (flow is null || flow.IsLive(Settings.Time)) return flow; + flow.Cancel(); // an expired flow still parked in its delay ends silently instead of reporting later + End(flow); + return null; + } + } - /// Registers as pending; false when another flow already is. - public bool Begin(ConnectFlow flow) => Interlocked.CompareExchange(ref _pending, flow, null) is null; + /// Registers as pending. Callers hold . + public void Begin(ConnectFlow flow) => Volatile.Write(ref _pending, flow); /// Clears the pending flow if it is still . public void End(ConnectFlow flow) => Interlocked.CompareExchange(ref _pending, null, flow); /// Cancels the pending flow silently (used by /calendar_disconnect). - public void CancelPending() => Pending?.Cancel(); + public void CancelPending() => Volatile.Read(ref _pending)?.Cancel(); /// /// A valid access token: the cached one while it has more than 60 s left and belongs to the stored refresh @@ -95,8 +108,9 @@ internal sealed class GoogleCalendarConnection var id = Store.Read()?.TimeZone; if (string.IsNullOrWhiteSpace(id)) return chatZone; if (ZoneIds.TryFind(id, out var zone)) return zone; - if (Interlocked.Exchange(ref _unknownZoneLogged, 1) == 0) - logger?.LogWarning("google-calendar: the calendar's time zone {Zone} is unknown on this machine; using the chat's zone {ChatZone}", id, ZoneIds.Name(chatZone)); + // Claim the log-once flag only when there is a logger: a logger-less call (e.g. the tool description) must not use it up. + if (logger is not null && Interlocked.Exchange(ref _unknownZoneLogged, 1) == 0) + logger.LogWarning("google-calendar: the calendar's time zone {Zone} is unknown on this machine; using the chat's zone {ChatZone}", id, ZoneIds.Name(chatZone)); return chatZone; } diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarExtension.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarExtension.cs index d54936f..43ea62d 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarExtension.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarExtension.cs @@ -15,6 +15,7 @@ public sealed class GoogleCalendarExtension(GoogleCalendarOptions? options = nul private readonly GoogleCalendarOptions _options = options ?? new GoogleCalendarOptions(); private GoogleCalendarSettings? _settings; + private ILogger? _logger; private volatile string? _scope; private volatile TimeZoneInfo? _chatZone; @@ -41,6 +42,7 @@ public sealed class GoogleCalendarExtension(GoogleCalendarOptions? options = nul } _settings = _options.Resolve()!; + _logger = api.Logger; api.RegisterTool(new CalendarTool(_settings, Connections, CurrentZoneName)); api.RegisterCommand(ConnectCommand.Name, ConnectCommand.Create(api, this)); api.RegisterCommand(StatusCommand.Name, StatusCommand.Create(api, this)); @@ -53,7 +55,7 @@ public sealed class GoogleCalendarExtension(GoogleCalendarOptions? options = nul /// The zone the tool's description and guideline name: the calendar's, else the chat's. Null before the session starts. internal string? CurrentZoneName() => _scope is { } scope && _settings is not null - ? ZoneIds.Name(ConnectionFor(scope).ResolveTimeZone(_chatZone ?? TimeZoneInfo.Local)) + ? ZoneIds.Name(ConnectionFor(scope).ResolveTimeZone(_chatZone ?? TimeZoneInfo.Local, _logger)) : null; private ValueTask Track(IExtensionContext ctx) diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarOptions.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarOptions.cs index b2bb59f..786cbb7 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarOptions.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleCalendarOptions.cs @@ -26,6 +26,9 @@ public sealed record GoogleCalendarOptions internal string? EffectiveClientId => NonEmpty(ClientId) ?? NonEmpty(Env("GOOGLE_CLIENT_ID")); internal string? EffectiveClientSecret => NonEmpty(ClientSecret) ?? NonEmpty(Env("GOOGLE_CLIENT_SECRET")); + /// True when both the client id and the secret are set (here or in the environment): the extension registers its tool and commands. + public bool IsConfigured => EffectiveClientId is not null && EffectiveClientSecret is not null; + internal string EffectiveTokenDirectory => NonEmpty(TokenDirectory) ?? Path.Combine( NonEmpty(Env("VOLCANIC_AGENT_DIR")) ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".volcanic", "agent"), "google-calendar"); diff --git a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleTokenStore.cs b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleTokenStore.cs index f7b6922..77edc2b 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleTokenStore.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.GoogleCalendar/GoogleTokenStore.cs @@ -4,7 +4,7 @@ using Volcanic.AgenticCore.Abstractions.Json; namespace Volcanic.AgenticCore.Extensions.GoogleCalendar; /// What token.json holds for one chat. hides the refresh token. -public sealed record StoredToken(string RefreshToken, string? Scope = null, string? Account = null, string? TimeZone = null, DateTimeOffset? ConnectedAt = null) +internal sealed record StoredToken(string RefreshToken, string? Scope = null, string? Account = null, string? TimeZone = null, DateTimeOffset? ConnectedAt = null) { public override string ToString() => $"StoredToken {{ Account = {Account}, TimeZone = {TimeZone}, ConnectedAt = {ConnectedAt:O} }}"; } diff --git a/hosts/Volcanic.AgenticCore.Telegram/GoogleCalendarSetup.cs b/hosts/Volcanic.AgenticCore.Telegram/GoogleCalendarSetup.cs index b8308bb..acd12b9 100644 --- a/hosts/Volcanic.AgenticCore.Telegram/GoogleCalendarSetup.cs +++ b/hosts/Volcanic.AgenticCore.Telegram/GoogleCalendarSetup.cs @@ -34,8 +34,7 @@ public static class GoogleCalendarSetup } /// True when both the client id and the secret are set, i.e. the extension will register its tool and commands. - public static bool IsActive(GoogleCalendarOptions? options) => - options is not null && NonEmpty(options.ClientId) is not null && NonEmpty(options.ClientSecret) is not null; + public static bool IsActive(GoogleCalendarOptions? options) => options?.IsConfigured == true; /// The bot's command menu, with the calendar commands appended when the calendar is active. public static IReadOnlyList MenuFor(IReadOnlyList menu, GoogleCalendarOptions? options) => diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarCommandsTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarCommandsTests.cs index 8ad58a5..60d67b1 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarCommandsTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarCommandsTests.cs @@ -60,6 +60,34 @@ public class CalendarCommandsTests Assert.Empty(h.Http.To("POST /revoke")); // nothing stored, nothing to revoke } + [Fact] + public async Task Disconnect_while_the_flow_completes_leaves_the_chat_disconnected() + { + using var h = new ConnectHarness(); + h.Http.Json(ConnectHarness.TokenRoute, ConnectHarness.Approved()); + var primaryEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var primaryRelease = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + h.Http.On(ConnectHarness.PrimaryRoute, async _ => + { + primaryEntered.TrySetResult(); + await primaryRelease.Task; // ignores cancellation: the lookup returns after the disconnect + return GoogleFakeHttp.Response(HttpStatusCode.OK, """{"id":"me@example.com","timeZone":"Europe/Berlin"}"""); + }); + var ui = new CalendarRecordingUi(); + var connect = h.Run("calendar_connect", h.Context(ui)); + await FlowDriver.ParkedAsync(h.Connection()); + h.Time.Advance(TimeSpan.FromSeconds(5)); + await primaryEntered.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + await h.Run(Disconnect, h.Context(ui)); + primaryRelease.SetResult(); + await connect.WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.False(File.Exists(h.Connection().Store.Path)); + Assert.Equal(["Calendar disconnected."], ui.Notes.Skip(1).Select(n => n.Message).ToArray()); // no "connected" + Assert.Null(h.Connection().Pending); + } + [Fact] public async Task Disconnect_revokes_deletes_and_invalidates() { diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarTimeTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarTimeTests.cs index 73627ad..b39f5a3 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarTimeTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarTimeTests.cs @@ -95,6 +95,14 @@ public class CalendarTimeTests public void TimeMin_is_rfc3339_with_an_offset_in_the_zone(string text, string expected) => Assert.Equal(expected, CalendarTime.ToRfc3339(text, Berlin, "timeMin")); + [Theory] + [InlineData("2026-09-30", "2026-10-01T00:00:00+02:00")] // a date is inclusive: its whole day is in the window + [InlineData("2026-10-25", "2026-10-26T00:00:00+01:00")] // the next midnight's offset: Berlin leaves DST on the 25th + [InlineData("2026-09-30T18:00", "2026-09-30T18:00:00+02:00")] + [InlineData("2026-09-30T18:00:00Z", "2026-09-30T18:00:00+00:00")] + public void TimeMax_as_a_date_ends_at_the_next_midnight_in_the_zone(string text, string expected) => + Assert.Equal(expected, CalendarTime.ToRfc3339(text, Berlin, "timeMax", dateIsInclusiveEnd: true)); + [Fact] public void Utc_formats_with_Z() => Assert.Equal("2026-09-26T08:00:00Z", CalendarTime.Utc(new DateTimeOffset(2026, 9, 26, 10, 0, 0, TimeSpan.FromHours(2)))); diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarToolReadTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarToolReadTests.cs index cb8f0d4..42961a7 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarToolReadTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/CalendarToolReadTests.cs @@ -170,6 +170,22 @@ public class CalendarToolReadTests Assert.Equal("ev-1", ev.GetProperty("recurringEventId").GetString()); Assert.Equal("2026-09-30T10:00:00+02:00", ev.GetProperty("start").GetProperty("dateTime").GetString()); Assert.StartsWith("https://calendar.google.com/", ev.GetProperty("htmlLink").GetString()); + foreach (var secret in new[] { "rt-1", "at-1" }) // the refresh and access tokens never reach the model + { + Assert.DoesNotContain(secret, result.TextContent); + Assert.DoesNotContain(secret, details.GetRawText()); + } + } + + [Fact] + public async Task TimeMax_given_as_the_same_date_as_timeMin_covers_that_whole_day() + { + using var h = new CalendarToolHarness(); + h.Http.Json(CalendarToolHarness.EventsRoute, Ev.List()); + await h.RunAsync(new { action = "list", timeMin = "2026-09-30", timeMax = "2026-09-30" }); + var query = Assert.Single(h.CalendarRequests).Query; + Assert.Equal("2026-09-30T00:00:00+02:00", query["timeMin"]); + Assert.Equal("2026-10-01T00:00:00+02:00", query["timeMax"]); } [Fact] diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ChatKeysTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ChatKeysTests.cs index 381b727..9f9acb2 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ChatKeysTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ChatKeysTests.cs @@ -18,6 +18,7 @@ public class ChatKeysTests [InlineData("")] [InlineData("has space")] [InlineData("c:\\x")] + [InlineData("telegram-1\n")] // a trailing newline: '$' alone would accept it public void An_unsafe_scope_maps_to_a_hashed_key(string scope) { var expected = "scope-" + Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(scope)))[..16]; diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ConnectFlowTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ConnectFlowTests.cs index dab7b1c..3a43184 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ConnectFlowTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/ConnectFlowTests.cs @@ -166,6 +166,48 @@ public class ConnectFlowTests await first.WaitAsync(TimeSpan.FromSeconds(5)); } + [Fact] + public async Task A_connect_after_the_code_expired_starts_a_new_flow_instead_of_resending_the_dead_code() + { + using var h = new ConnectHarness(); + h.ScriptDevice("dev-1", "ABCD-EFGH", expiresIn: 10, interval: 30); // parked until +30, dead at +10 + h.Http.Json(ConnectHarness.TokenRoute, ConnectHarness.Pending); + var ui = new CalendarRecordingUi(); + var first = h.Run(Connect, h.Context(ui)); + var stale = await FlowDriver.ParkedAsync(h.Connection()); + h.Time.Advance(TimeSpan.FromSeconds(11)); + + var statusUi = new CalendarRecordingUi(); + await h.Run("calendar_status", h.Context(statusUi)); + Assert.Equal("Not connected. Run /calendar_connect.", Assert.Single(statusUi.Notes).Message); + await first.WaitAsync(TimeSpan.FromSeconds(5)); // the stale flow was cancelled: it ends silently + + h.ScriptDevice("dev-2", "WXYZ-1234"); + var second = h.Run(Connect, h.Context(ui)); + var fresh = await FlowDriver.ParkedAsync(h.Connection()); + + Assert.NotSame(stale, fresh); + Assert.Equal(2, h.Http.To(ConnectHarness.DeviceRoute).Count); + Assert.Equal(2, ui.Notes.Count); // two links, no "already waiting", no late expiry warning + Assert.Contains("WXYZ-1234", ui.Notes[1].Message); + h.Connection().CancelPending(); + await second.WaitAsync(TimeSpan.FromSeconds(5)); + } + + [Fact] + public async Task A_cancelled_flow_that_has_not_ended_yet_is_not_resent() + { + using var h = new ConnectHarness(); + h.Http.Json(ConnectHarness.TokenRoute, ConnectHarness.Pending); + var ui = new CalendarRecordingUi(); + var first = h.Run(Connect, h.Context(ui)); + var stale = await FlowDriver.ParkedAsync(h.Connection()); + stale.Cancel(); + + Assert.Null(h.Connection().Pending); + await first.WaitAsync(TimeSpan.FromSeconds(5)); + } + [Fact] public async Task Cancelling_a_pending_flow_is_silent_and_clears_it() { diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/GoogleCalendarExtensionTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/GoogleCalendarExtensionTests.cs index 8e11de7..74a0c7e 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/GoogleCalendarExtensionTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/GoogleCalendar/GoogleCalendarExtensionTests.cs @@ -57,4 +57,34 @@ public class GoogleCalendarExtensionTests h.Connection().Store.Write(new StoredToken("rt-1", TimeZone: "America/New_York")); Assert.Contains("without an offset are in America/New_York", Assert.Single(tool.PromptGuidelines)); } + + [Fact] + public async Task An_unknown_stored_zone_read_first_by_the_description_is_still_logged_exactly_once() + { + var logger = new ListLogger(); + using var h = new ConnectHarness(logger); + var tool = Assert.Single(h.Api.Tools); + await h.Api.RaiseAsync(new SessionStartEvent(SessionStartReason.Startup), h.Context(null)); + h.Connection().Store.Write(new StoredToken("rt-1", TimeZone: "Mars/Olympus_Mons")); + + Assert.Contains("without an offset are in Europe/Berlin", tool.Description); // read before any tool call + _ = tool.PromptGuidelines; + h.Connection().ResolveTimeZone(ConnectHarness.Berlin, logger); // what the tool's execution does + + var entry = Assert.Single(logger.Entries, e => e.Level == LogLevel.Warning); + Assert.Contains("Mars/Olympus_Mons", entry.Message); + } + + [Fact] + public void A_logger_less_zone_lookup_does_not_use_up_the_log_once_flag() + { + using var h = new ConnectHarness(); + var connection = h.Connection(); + connection.Store.Write(new StoredToken("rt-1", TimeZone: "Mars/Olympus_Mons")); + connection.ResolveTimeZone(ConnectHarness.Berlin); // no logger + var logger = new ListLogger(); + connection.ResolveTimeZone(ConnectHarness.Berlin, logger); + connection.ResolveTimeZone(ConnectHarness.Berlin, logger); + Assert.Single(logger.Entries); + } } diff --git a/tests/Volcanic.AgenticCore.Telegram.Tests/GoogleCalendarHostTests.cs b/tests/Volcanic.AgenticCore.Telegram.Tests/GoogleCalendarHostTests.cs index 008bd68..b136bec 100644 --- a/tests/Volcanic.AgenticCore.Telegram.Tests/GoogleCalendarHostTests.cs +++ b/tests/Volcanic.AgenticCore.Telegram.Tests/GoogleCalendarHostTests.cs @@ -1,6 +1,11 @@ using System.Net; using System.Text; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Time.Testing; +using Telegram.Bot; +using Telegram.Bot.Requests; using Volcanic.AgenticCore.Extensions.GoogleCalendar; using Volcanic.AgenticCore.Features.Processes; using Volcanic.AgenticCore.Telegram.Chats; @@ -80,6 +85,33 @@ public class GoogleCalendarHostTests : IDisposable Assert.Equal(["calendar_connect", "calendar_status", "calendar_disconnect"], menu.Skip(ChatCommands.Menu.Count).Select(c => c.Command).ToArray()); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Bot_setup_publishes_the_calendar_commands_only_when_the_calendar_is_configured(bool configured) + { + var bot = new FakeBotClient(); + var services = new ServiceCollection() + .AddSingleton(bot) + .AddSingleton(new TelegramOptions()) + .AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)) + .AddSingleton(); + if (configured) services.AddSingleton(Calendar()); // as Program.cs registers it + using var provider = services.BuildServiceProvider(); + var setup = provider.GetRequiredService(); + + await setup.StartAsync(CancellationToken.None); + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!bot.Requests.OfType().Any() && DateTime.UtcNow < deadline) await Task.Delay(10); + await setup.StopAsync(CancellationToken.None); + + var commands = Assert.Single(bot.Requests.OfType()).Commands.Select(c => c.Command).ToList(); + var calendarCommands = new[] { "calendar_connect", "calendar_status", "calendar_disconnect" }; + if (configured) Assert.Equal(calendarCommands, commands.Skip(ChatCommands.Menu.Count).ToArray()); + else Assert.DoesNotContain(commands, c => calendarCommands.Contains(c)); + Assert.Equal(ChatCommands.Menu.Select(c => c.Command), commands.Take(ChatCommands.Menu.Count)); + } + [Fact] public async Task Every_allowed_chat_lists_the_calendar_tool() { diff --git a/tests/Volcanic.AgenticCore.Telegram.Tests/Support/ChatHost.cs b/tests/Volcanic.AgenticCore.Telegram.Tests/Support/ChatHost.cs index 97ca8fb..862cd5d 100644 --- a/tests/Volcanic.AgenticCore.Telegram.Tests/Support/ChatHost.cs +++ b/tests/Volcanic.AgenticCore.Telegram.Tests/Support/ChatHost.cs @@ -55,7 +55,7 @@ public sealed class ChatHost : IAsyncDisposable _services = services.BuildServiceProvider(); ILoggerFactory loggers = NullLoggerFactory.Instance; var sender = new TelegramSender(Bot, NullLogger.Instance); - Chats = new ChatRegistry(_services.GetRequiredService(), sender, Options, loggers, calendar); + Chats = new ChatRegistry(_services.GetRequiredService(), sender, Options, loggers, calendar: calendar); Router = new UpdateRouter(Options, Chats, new ChatCommands(sender), sender, NullLogger.Instance); } diff --git a/tests/Volcanic.AgenticCore.Telegram.Tests/Volcanic.AgenticCore.Telegram.Tests.csproj b/tests/Volcanic.AgenticCore.Telegram.Tests/Volcanic.AgenticCore.Telegram.Tests.csproj index 33aa3cc..736b826 100644 --- a/tests/Volcanic.AgenticCore.Telegram.Tests/Volcanic.AgenticCore.Telegram.Tests.csproj +++ b/tests/Volcanic.AgenticCore.Telegram.Tests/Volcanic.AgenticCore.Telegram.Tests.csproj @@ -5,10 +5,10 @@ false - +