From ee67d520d3e7aeb7c7c8bb05cdf4dde6500f2473 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 00:13:12 -0400 Subject: [PATCH] Report a refused Google grant before a missing calendar on push A grant refused while no calendar was chosen (for example during calendar listing) made a later push return no_calendar instead of google_reconnect_required, sending the household to choose a calendar when only connecting again helps. grantedConnection now decides the precedence for every Calendar call that uses the grant (listing, choosing, pushing): not connected, then a refused grant, then anything else such as no calendar chosen. It holds the existing cleared-token check, moved out of accessToken. Fixes F16-T4-01 for Maestro task b8d8da5a-0450-4571-963e-b3dfead24283. Co-Authored-By: Claude Opus 5.5 --- internal/gcal/calendars.go | 31 ++++++++++++++------ internal/gcal/events.go | 11 +++++--- internal/gcal/gcaltest/gcaltest.go | 8 ++++++ internal/gcal/grant_test.go | 45 ++++++++++++++++++++++++++++++ internal/server/plans_test.go | 24 ++++++++++++++++ 5 files changed, 107 insertions(+), 12 deletions(-) diff --git a/internal/gcal/calendars.go b/internal/gcal/calendars.go index 23e21bc..b516ce0 100644 --- a/internal/gcal/calendars.go +++ b/internal/gcal/calendars.go @@ -160,7 +160,7 @@ func forgetEvents(ctx context.Context, q *db.Queries, now string) error { // calendars lists the writable calendars, each once, with the stored refresh // token. The caller holds s.account. func (s *Service) calendars(ctx context.Context) ([]Calendar, error) { - row, err := s.connection(ctx) + row, err := s.grantedConnection(ctx) if err != nil { return nil, err } @@ -207,14 +207,29 @@ func (s *Service) connection(ctx context.Context) (db.IntegrationsGoogle, error) return row, nil } -// accessToken trades row's refresh token for an access token, retrying a -// transient failure as send does. A grant Google reports as invalid_grant, -// or one already recorded as refused (no refresh token stored), is -// ErrReconnect. -func (s *Service) accessToken(ctx context.Context, row db.IntegrationsGoogle) (string, error) { - if row.RefreshTokenCt == nil { - return "", ErrReconnect +// grantedConnection reads the connected account for a Calendar call that +// uses the stored grant: listing, choosing and pushing. It decides the +// precedence of their preconditions: nothing connected is ErrNotConnected, +// then a grant already recorded as refused (no refresh token stored) is +// ErrReconnect, before anything else a call checks, such as whether a +// calendar is chosen. Only connecting again restores a refused grant, so +// it is reported whatever the calendar choice, and the refused token is +// never offered to Google. +func (s *Service) grantedConnection(ctx context.Context) (db.IntegrationsGoogle, error) { + row, err := s.connection(ctx) + if err != nil { + return db.IntegrationsGoogle{}, err } + if row.RefreshTokenCt == nil { + return db.IntegrationsGoogle{}, ErrReconnect + } + return row, nil +} + +// accessToken trades row's refresh token, from grantedConnection, for an +// access token, retrying a transient failure as send does. A grant Google +// reports as invalid_grant is ErrReconnect. +func (s *Service) accessToken(ctx context.Context, row db.IntegrationsGoogle) (string, error) { refresh, err := s.box.Decrypt(row.RefreshTokenCt) if err != nil { return "", fmt.Errorf("gcal: open refresh token: %w", err) diff --git a/internal/gcal/events.go b/internal/gcal/events.go index a60e0de..494805f 100644 --- a/internal/gcal/events.go +++ b/internal/gcal/events.go @@ -80,8 +80,9 @@ type eventTime struct { // includes an event deleted in Calendar) is updated in place instead, so // writing an event again never adds a second one. It stops at the first // failure; the events before it stay written, and writing them all again -// is safe. Nothing connected is ErrNotConnected, no calendar chosen is -// ErrNoCalendar, a chosen calendar that was deleted or is no longer +// is safe. Nothing connected is ErrNotConnected, a grant already recorded +// as refused is ErrReconnect whatever calendar is chosen, no calendar +// chosen is ErrNoCalendar, a chosen calendar that was deleted or is no longer // writable is ErrCalendarUnavailable, a grant Google no longer accepts is // ErrReconnect (recorded as reconnect required, see recordRefusal), and any // other Google failure is a *CalendarError, retryable when Google answered @@ -106,9 +107,11 @@ func (s *Service) PushEvents(ctx context.Context, events []Event, record func() } // pushEvents is PushEvents once the events are valid. The caller holds -// s.account. +// s.account. A refused grant is reported before no calendar chosen +// (grantedConnection): choosing a calendar cannot fix it, connecting again +// can. func (s *Service) pushEvents(ctx context.Context, events []Event, record func() error) error { - row, err := s.connection(ctx) + row, err := s.grantedConnection(ctx) if err != nil { return err } diff --git a/internal/gcal/gcaltest/gcaltest.go b/internal/gcal/gcaltest/gcaltest.go index 485f4bb..78d88af 100644 --- a/internal/gcal/gcaltest/gcaltest.go +++ b/internal/gcal/gcaltest/gcaltest.go @@ -282,6 +282,14 @@ func (g *Google) RejectAccessToken() { g.rejectAccess = true } +// AcceptAccessToken undoes RejectAccessToken, as Google does for the grant +// of a later connection. +func (g *Google) AcceptAccessToken() { + g.mu.Lock() + defer g.mu.Unlock() + g.rejectAccess = false +} + // CalendarCalls is how many requests reached the calendarList endpoint. func (g *Google) CalendarCalls() int { g.mu.Lock() diff --git a/internal/gcal/grant_test.go b/internal/gcal/grant_test.go index bc02de9..3993ee7 100644 --- a/internal/gcal/grant_test.go +++ b/internal/gcal/grant_test.go @@ -93,6 +93,51 @@ func TestRefusedGrantRequiresReconnect(t *testing.T) { } } +// TestRefusedGrantBeforeACalendarIsChosen refuses the grant while no +// calendar is chosen, on each call that can record it then (listing and +// choosing): every Calendar call, push included, is ErrReconnect rather than +// ErrNoCalendar, since only connecting again restores the grant. After +// connecting again and choosing a calendar, a push succeeds. +func TestRefusedGrantBeforeACalendarIsChosen(t *testing.T) { + t.Parallel() + for refusal, refuse := range refusals { + for _, call := range []string{"list calendars", "choose a calendar"} { + t.Run(refusal+"/"+call, func(t *testing.T) { + t.Parallel() + h := connected(t) + refuse(h.google) + if err := calls[call](h); !errors.Is(err, gcal.ErrReconnect) { + t.Fatalf("%s = %v, want ErrReconnect", call, err) + } + if row, _ := h.row(t); row.RefreshTokenCt != nil || row.CalendarID.Valid { + t.Fatalf("row = %+v, want the grant cleared and no calendar chosen", row) + } + tokens, cals, events := h.google.TokenCalls(), h.google.CalendarCalls(), h.google.EventRequests() + for again, do := range calls { + if err := do(h); !errors.Is(err, gcal.ErrReconnect) { + t.Errorf("%s after the refusal = %v, want ErrReconnect", again, err) + } + } + if h.google.TokenCalls() != tokens || h.google.CalendarCalls() != cals || h.google.EventRequests() != events { + t.Error("a call reached Google after the grant was refused") + } + + h.google.AcceptAccessToken() + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatalf("Complete: %v", err) + } + if _, err := h.svc.SelectCalendar(context.Background(), dinners.ID); err != nil { + t.Fatalf("SelectCalendar after connecting again: %v", err) + } + if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord); err != nil { + t.Errorf("PushEvents after connecting again: %v", err) + } + }) + } + } +} + // TestReconnectAfterRefusal connects again after Google refused the grant: // the new token is stored, the reason cleared and the calendar kept. func TestReconnectAfterRefusal(t *testing.T) { diff --git a/internal/server/plans_test.go b/internal/server/plans_test.go index 4895978..1b67833 100644 --- a/internal/server/plans_test.go +++ b/internal/server/plans_test.go @@ -473,6 +473,30 @@ func TestPushPlanToCalendarErrors(t *testing.T) { } } +// TestPushPlanAfterRefusalWithNoCalendar pushes after Google refused the +// grant while listing calendars, before any was chosen: the push is 409 +// google_reconnect_required, not no_calendar, since only connecting again +// restores it. After connecting again and choosing a calendar it succeeds. +func TestPushPlanAfterRefusalWithNoCalendar(t *testing.T) { + t.Parallel() + e := newPlansRouter(t) + e.connectCalendar(t, false) + e.google.RevokeGrant() + if _, err := e.gcal.Calendars(t.Context()); !errors.Is(err, gcal.ErrReconnect) { + t.Fatalf("Calendars = %v, want ErrReconnect", err) + } + + rec, body := e.push(t, "2026-10-12") + if rec.Code != http.StatusConflict || errorCode(body) != "google_reconnect_required" { + t.Errorf("push = %d %s, want 409 google_reconnect_required", rec.Code, rec.Body) + } + + e.connectCalendar(t, true) + if rec, _ := e.push(t, "2026-10-12"); rec.Code != http.StatusOK { + t.Errorf("push after connecting again = %d: %s", rec.Code, rec.Body) + } +} + func TestPushPlanChangedWhilePushing(t *testing.T) { t.Parallel() e := newPlansRouter(t)