From fb7d8ddff812ab88939d30c03d65d96d0d3ddaa1 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 23:05:10 -0400 Subject: [PATCH] Forget pushed event ids when Google Calendar is disconnected (Feature 16) Disconnect deleted the connection but left every pushed plan synced with its stored event ids, though no calendar was chosen any more. It now deletes the connection and runs the same invalidation as choosing another calendar (forgetEvents: MarkCalendarPlansEdited, ClearCalendarEvents) in one transaction, under the account lock. A refused revoke still changes nothing. Tests push three weeks, disconnect, and check each is changed with no event id; then connect again, choose a calendar and push the whole week to it. Also: a refused revoke keeps every week synced, and a disconnect during a push waits for its record and then forgets it. Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 22 ++++--- internal/db/plans.sql.go | 6 +- internal/db/querier.go | 6 +- internal/gcal/calendars.go | 33 ++++++---- internal/gcal/connect.go | 16 +++-- internal/plan/push.go | 8 ++- internal/plan/push_test.go | 121 +++++++++++++++++++++++++++++++++++ migrations/queries/plans.sql | 6 +- 8 files changed, 183 insertions(+), 35 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index dcf3f12..59a4538 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -100,7 +100,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `suggestion_batches` | trigger, status, ai_run_id. | | `suggestions` | dinner_id, batch_id, name (the name the dish was suggested under, kept whatever its dinner is later renamed, so a Pass stays an immutable record every later batch dedupes against), reason, cited_regular_ids (JSON), status, decided_at. | | `week_plans` | week_start (unique), status (`draft` / `synced` / `changed`), ai_note, ai_run_id, calendar_synced_at, revision (raised by every swap and stored draft). | -| `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id (set while the chosen calendar shows the day as planned; cleared when its dinner changes or another calendar is chosen). | +| `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id (set while the chosen calendar shows the day as planned; cleared when its dinner changes, another calendar is chosen or Google Calendar is disconnected). | | `rotation_reviews` | dinner_id + plan_date (unique), status, decision (`keep` / `drop` / `not_made`), name (set only by a drop: the dinner's name when dropped, kept whatever it is later renamed, so a drop stays among the passed names), decided_at, updated_by. | | `shopping_lists` | week_start (unique), plan_id, title, status (`current` / `archived`), sent_to_chat_at, frozen_at. | | `shopping_groups` / `shopping_items` | One group per dinner/day; item name, qty, needed (bool), position. | @@ -640,8 +640,10 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. `connected_at` and redirects to `/settings/calendar`. A reconnect keeps the chosen calendar and clears `last_sync_error`. - `DELETE /api/v1/integrations/google` (CSRF-checked) revokes the token at - Google, then deletes the row; a token Google reports as `invalid_token` - counts as revoked, and any other revoke failure keeps the connection. + Google, then deletes the row and, in the same transaction, forgets every + pushed event (see Push to Calendar); a token Google reports as + `invalid_token` counts as revoked, and any other revoke failure keeps + the connection and its pushed events. - `GET /api/v1/integrations/google/calendars` trades the stored refresh token for an access token and reads every page of the Calendar API's `users/me/calendarList`, answering `[{id, summary, primary, @@ -686,11 +688,15 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. pushed, swapped, or redrafted to another dinner). When none is, it writes every day again, which puts back anything changed in Calendar. - A stored `calendar_event_id` is always an event on the chosen calendar. - Choosing a calendar other than the stored one clears every day's - `calendar_event_id` in the same transaction as the choice, and edits - each plan that had one (revision raised, `synced` → `changed`), so the - next push writes the whole week to the calendar now chosen and a push - that read such a plan before the choice is refused (409 `plan_changed`). + Choosing a calendar other than the stored one, and disconnecting (which + leaves none chosen; connecting again chooses none either), clear every + day's `calendar_event_id` in the same transaction as the choice or the + delete, and edit each plan that had one (revision raised, `synced` → + `changed`), so the next push writes the whole week to the calendar then + chosen and a push that read such a plan before is refused (409 + `plan_changed`). A reconnect while connected keeps the chosen calendar, + so it keeps the event ids. Nothing else clears the choice or the stored + token: a grant Google refuses leaves the connection as it is. - Only a push that wrote all its events is recorded, in one transaction: the plan becomes `synced` with `calendar_synced_at`, and each day written stores its event id. A push that failed part way records diff --git a/internal/db/plans.sql.go b/internal/db/plans.sql.go index 8adddf8..b429e30 100644 --- a/internal/db/plans.sql.go +++ b/internal/db/plans.sql.go @@ -273,9 +273,9 @@ WHERE EXISTS ( ` // Records that Calendar no longer shows the days with a calendar event id -// as planned, since another calendar is chosen: every plan with such a day -// is edited as MarkWeekPlanEdited does, so a synced plan becomes changed -// and a push that read it before is refused. +// as planned, since another calendar or none is chosen: every plan with +// such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes +// changed and a push that read it before is refused. func (q *Queries) MarkCalendarPlansEdited(ctx context.Context, updatedAt string) error { _, err := q.db.ExecContext(ctx, markCalendarPlansEdited, updatedAt) return err diff --git a/internal/db/querier.go b/internal/db/querier.go index e3bc615..bcfc102 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -173,9 +173,9 @@ type Querier interface { // first. ListWeekPlanDinnerIDs(ctx context.Context, weekStart string) ([]sql.NullString, error) // Records that Calendar no longer shows the days with a calendar event id - // as planned, since another calendar is chosen: every plan with such a day - // is edited as MarkWeekPlanEdited does, so a synced plan becomes changed - // and a push that read it before is refused. + // as planned, since another calendar or none is chosen: every plan with + // such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes + // changed and a push that read it before is refused. MarkCalendarPlansEdited(ctx context.Context, updatedAt string) error // Moves a dinner still in the Quest log to tried; a dinner with any other // status is left as it is (0 rows). diff --git a/internal/gcal/calendars.go b/internal/gcal/calendars.go index dce497a..9ce3436 100644 --- a/internal/gcal/calendars.go +++ b/internal/gcal/calendars.go @@ -96,10 +96,11 @@ func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, err // saveSelection stores sel as the chosen calendar. Choosing another calendar // than the one stored also forgets every plan day's calendar event id, in // the same transaction, since those events are on the calendar no longer -// chosen: each plan with one is edited, so a synced plan becomes changed and -// its next push writes every day to sel. The caller holds s.account, which a -// push also holds from its event writes through recording them, so a push -// never records events written to another calendar than the chosen one. +// chosen (forgetEvents): each plan with one is edited, so a synced plan +// becomes changed and its next push writes every day to sel. The caller +// holds s.account, which a push also holds from its event writes through +// recording them, so a push never records events written to another +// calendar than the chosen one. func (s *Service) saveSelection(ctx context.Context, sel Selection) error { return s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) @@ -123,16 +124,26 @@ func (s *Service) saveSelection(ctx context.Context, sel Selection) error { if row.CalendarID.Valid && row.CalendarID.String == sel.CalendarID { return nil } - if err := q.MarkCalendarPlansEdited(ctx, now); err != nil { - return fmt.Errorf("gcal: mark plans pushed to the earlier calendar edited: %w", err) - } - if err := q.ClearCalendarEvents(ctx, now); err != nil { - return fmt.Errorf("gcal: forget events on the earlier calendar: %w", err) - } - return nil + return forgetEvents(ctx, q, now) }) } +// forgetEvents records, in the caller's transaction q, that no calendar +// event id stored is on the chosen calendar any more: every plan with one is +// edited (a synced plan becomes changed, and a push that read it before is +// refused), then every id is cleared. Every change that ends the chosen +// calendar — choosing another (saveSelection) or disconnecting +// (Disconnect) — runs it in the transaction that makes the change. +func forgetEvents(ctx context.Context, q *db.Queries, now string) error { + if err := q.MarkCalendarPlansEdited(ctx, now); err != nil { + return fmt.Errorf("gcal: mark plans pushed to the earlier calendar edited: %w", err) + } + if err := q.ClearCalendarEvents(ctx, now); err != nil { + return fmt.Errorf("gcal: forget events on the earlier calendar: %w", err) + } + return nil +} + // 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) { diff --git a/internal/gcal/connect.go b/internal/gcal/connect.go index 2e69046..fc37080 100644 --- a/internal/gcal/connect.go +++ b/internal/gcal/connect.go @@ -342,6 +342,11 @@ func (s *Service) store(ctx context.Context, sealed []byte) error { // connection. A token Google no longer knows (the household already removed // MealQuest's access in their Google account) counts as revoked. Any other // revoke failure keeps the connection so the household can retry. +// +// No calendar is chosen once disconnected, so the same transaction that +// deletes the connection forgets every pushed event (forgetEvents): every +// synced plan becomes changed, and the next push after connecting again and +// choosing a calendar writes the whole week. func (s *Service) Disconnect(ctx context.Context) error { s.account.Lock() defer s.account.Unlock() @@ -359,10 +364,13 @@ func (s *Service) Disconnect(ctx context.Context) error { if err := s.revoke(ctx, string(token)); err != nil { return err } - if err := db.New(s.st.Writer).DeleteIntegrationsGoogle(ctx); err != nil { - return fmt.Errorf("gcal: delete Google integration: %w", err) - } - return nil + return s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + if err := q.DeleteIntegrationsGoogle(ctx); err != nil { + return fmt.Errorf("gcal: delete Google integration: %w", err) + } + return forgetEvents(ctx, q, store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String()) + }) } // RevokeError is a revoke Google refused or that failed in transit. Its text diff --git a/internal/plan/push.go b/internal/plan/push.go index 770050b..e002397 100644 --- a/internal/plan/push.go +++ b/internal/plan/push.go @@ -64,8 +64,9 @@ type pushDay struct { // calendar_event_id is empty. When every day has one, Calendar shows the // whole plan and every day is written again, which puts back anything // changed in Calendar since: the app is the source of truth. A stored event -// id is always on the chosen calendar: choosing another clears them all -// (gcal.Service.SelectCalendar), so the next push writes the whole week. +// id is always on the chosen calendar: choosing another +// (gcal.Service.SelectCalendar) or disconnecting (gcal.Service.Disconnect) +// clears them all, so the next push writes the whole week. // // Only a push that wrote every one of those events is recorded: the plan // becomes synced with calendar_synced_at now, and each day written stores @@ -74,7 +75,8 @@ type pushDay struct { // updates the events already written rather than adding them twice. The // push is recorded while the calendar written to is still the chosen one. // A plan edited while it was pushed, or whose events were forgotten by -// choosing another calendar, fails with ErrPlanChanged, unrecorded. +// choosing another calendar or disconnecting, fails with ErrPlanChanged, +// unrecorded. // // The week must have a plan (else ErrNotFound). The calendar's errors are // returned as they are: gcal.ErrNotConnected, gcal.ErrNoCalendar, diff --git a/internal/plan/push_test.go b/internal/plan/push_test.go index dfad043..5f4d44d 100644 --- a/internal/plan/push_test.go +++ b/internal/plan/push_test.go @@ -631,3 +631,124 @@ func TestChoosingAnotherCalendarWaitsForAPush(t *testing.T) { } e.assertWeekOn(t, kitchenCal.ID, week, dates, fullWeek...) } + +// pushedWeeks are the weeks the disconnect tests push: the calendar and the +// connection are the household's, so a disconnect concerns every one. +var pushedWeeks = []store.LocalDate{week, mustDate("2026-10-19"), mustDate("2026-10-26")} + +// assertForgotten fails unless the plan of weekStart is changed and no day +// keeps an event id. +func (e *env) assertForgotten(t *testing.T, weekStart store.LocalDate, why string) { + t.Helper() + p, err := e.svc.Get(t.Context(), weekStart) + if err != nil { + t.Fatal(err) + } + if p.Status != plan.StatusChanged { + t.Errorf("plan of %s %s = %s, want changed", weekStart, why, p.Status) + } + for date, id := range e.eventIDs(t, weekStart) { + if id != "" { + t.Errorf("%s kept event id %q %s", date, id, why) + } + } +} + +func TestDisconnectForgetsEveryPushedWeek(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := map[store.LocalDate][]string{} + for _, w := range pushedWeeks { + dates[w] = e.storePlan(t, w, fullWeek...) + if _, err := e.svc.Push(t.Context(), w); err != nil { + t.Fatalf("Push %s: %v", w, err) + } + } + draftWeek := mustDate("2026-11-02") + e.storePlan(t, draftWeek, fullWeek...) + + if err := e.gcal.Disconnect(t.Context()); err != nil { + t.Fatalf("Disconnect: %v", err) + } + for _, w := range pushedWeeks { + e.assertForgotten(t, w, "after disconnecting") + } + e.assertUnsynced(t, draftWeek, plan.StatusDraft) + + // Connecting again chooses no calendar, so nothing is pushed until one + // is chosen; then a push writes the whole week to it. + e.google.Connect(t, e.gcal) + if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) { + t.Fatalf("Push after connecting again = %v, want ErrNoCalendar", err) + } + e.google.SetCalendars(0, dinnersCal, kitchenCal) + if _, err := e.gcal.SelectCalendar(t.Context(), kitchenCal.ID); err != nil { + t.Fatal(err) + } + for _, w := range pushedWeeks { + e.assertForgotten(t, w, "after connecting again") + p, err := e.svc.Push(t.Context(), w) + if err != nil || p.Status != plan.StatusSynced { + t.Fatalf("Push %s = %s, %v; want synced", w, p.Status, err) + } + e.assertWeekOn(t, kitchenCal.ID, w, dates[w], fullWeek...) + } +} + +// TestDisconnectRefusedKeepsPushedWeeks: a revoke Google refuses keeps the +// connection and its calendar, so every pushed week stays synced to it. +func TestDisconnectRefusedKeepsPushedWeeks(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := map[store.LocalDate][]string{} + for _, w := range pushedWeeks { + dates[w] = e.storePlan(t, w, fullWeek...) + if _, err := e.svc.Push(t.Context(), w); err != nil { + t.Fatalf("Push %s: %v", w, err) + } + } + e.google.AnswerRevoke(http.StatusServiceUnavailable, "") + var revokeErr *gcal.RevokeError + if err := e.gcal.Disconnect(t.Context()); !errors.As(err, &revokeErr) { + t.Fatalf("Disconnect = %v, want a *RevokeError", err) + } + for _, w := range pushedWeeks { + if p, err := e.svc.Get(t.Context(), w); err != nil || p.Status != plan.StatusSynced { + t.Errorf("plan of %s after a refused disconnect = %s, %v; want synced", w, p.Status, err) + } + e.assertWeekOn(t, dinnersCal.ID, w, dates[w], fullWeek...) + } +} + +// TestDisconnectWaitsForAPush disconnects while a push is at Google: the +// disconnect waits until the push is recorded, then forgets its events. +func TestDisconnectWaitsForAPush(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + e.storePlan(t, week, fullWeek...) + arrived, release := e.google.HoldEvents() + defer release() + pushed := make(chan error, 1) + go func() { + _, err := e.svc.Push(t.Context(), week) + pushed <- err + }() + <-arrived + disconnected := make(chan error, 1) + go func() { + disconnected <- e.gcal.Disconnect(t.Context()) + }() + select { + case err := <-disconnected: + t.Fatalf("Disconnect finished (%v) while a push was at Google", err) + case <-time.After(200 * time.Millisecond): + } + release() + if err := <-pushed; err != nil { + t.Fatalf("Push: %v", err) + } + if err := <-disconnected; err != nil { + t.Fatalf("Disconnect: %v", err) + } + e.assertForgotten(t, week, "after disconnecting") +} diff --git a/migrations/queries/plans.sql b/migrations/queries/plans.sql index adece04..369ec03 100644 --- a/migrations/queries/plans.sql +++ b/migrations/queries/plans.sql @@ -60,9 +60,9 @@ WHERE id = ? AND revision = ?; -- name: MarkCalendarPlansEdited :exec -- Records that Calendar no longer shows the days with a calendar event id --- as planned, since another calendar is chosen: every plan with such a day --- is edited as MarkWeekPlanEdited does, so a synced plan becomes changed --- and a push that read it before is refused. +-- as planned, since another calendar or none is chosen: every plan with +-- such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes +-- changed and a push that read it before is refused. UPDATE week_plans SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, revision = revision + 1,