amadan

oficina / meet Public

fix: nextExpiry reads auth_links too, so a plain sign-in schedules a wake

Re-review of 9e067e4: store.nextExpiry read only pending_joins, but a
plain member sign-in (no meeting-link cookie) writes no pending row at
all, only rastrillo's own auth_links row for the mailed magic link. If
that unclicked link were the only thing outstanding when the app
hibernated, NextDue returned zero and the platform never woke it to
sweep it.

nextExpiry now takes the earlier of pending_joins (via gorm) and a new
nextAuthLinkExpiry, which reads auth_links directly with
SELECT MIN(expires_at), parsing the RFC3339 text rastrillo/auth stores
there (auth/store.go:82-83). TestHandle gained NextDue() so a black-box
meettest test can check the wake time after a real request.

TestNextExpiryAlsoReadsAuthLinks (internal/meet/store_test.go) proves
the earlier-of-two-tables ordering in both directions.
TestNextDueWakesForAPlainMemberSignin (internal/meettest/signin_test.go)
drives a real plain POST /signin and checks NextDue is non-zero and
within the magic link's TTL window.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev c5981ca8fc345149f4913d6160245dad59185513 parent 9e067e4
4 files changed, +146 −6
  • internal/meet/app.go +6 −0
  • internal/meet/store.go +58 −6
  • internal/meet/store_test.go +54 −0
  • internal/meettest/signin_test.go +28 −0

CI passed — run details

diff --git a/internal/meet/app.go b/internal/meet/app.go
index 974002f..90fb8cd 100644
--- a/internal/meet/app.go
+++ b/internal/meet/app.go
@@ -49,6 +49,12 @@ func (h *TestHandle) SweepForTest(now time.Time) {
h.a.sweepOnce(ctx, now)
}
+// NextDue answers what Runtime.NextDue would answer right now — the
+// platform wake-up time App wires to Options.NextDue. Exposed so a
+// black-box meettest test can check it after driving a real request,
+// without reaching into the store directly.
+func (h *TestHandle) NextDue() time.Time { return h.a.nextDue() }
+
// gormWriter is GORM's logger.Writer over the app's slog logger, so
// GORM's warnings land in one log stream rather than a second stdout
// format. rastrillo/db has the same adapter, unexported.
diff --git a/internal/meet/store.go b/internal/meet/store.go
index 54806b5..b1b27e6 100644
--- a/internal/meet/store.go
+++ b/internal/meet/store.go
@@ -2,6 +2,7 @@ package meet
import (
"context"
+ "database/sql"
"errors"
"fmt"
"log/slog"
@@ -391,17 +392,68 @@ func (s *store) sweep(ctx context.Context, now time.Time) (int64, error) {
return res.RowsAffected, res.Error
}
-// nextExpiry answers the earliest expires_at among pending_joins, or the
-// zero time when there is no pending row at all. It is the platform
-// wake-up input for cmd/meet/main.go's Options.NextDue (review C2-2).
+// nextExpiry answers the earliest expires_at across pending_joins AND
+// rastrillo's own auth_links, or the zero time when both are empty. It
+// is the platform wake-up input for cmd/meet/main.go's Options.NextDue
+// (review C2-2).
+//
+// auth_links matters here too: a plain member sign-in writes no pending
+// row at all (that table exists for the outsider/pending-join path,
+// spec §3.2), only rastrillo's own magic-link row. If nextExpiry read
+// pending_joins alone, an app that hibernated with nothing outstanding
+// but one unclicked member link would never be woken to sweep it — the
+// platform sees a zero NextDue and assumes there is nothing to do
+// (round 2 follow-up, caught on re-review of 9e067e4).
func (s *store) nextExpiry(ctx context.Context) (time.Time, error) {
var p PendingJoin
err := s.g.WithContext(ctx).Order("expires_at ASC").Take(&p).Error
- if errors.Is(err, gorm.ErrRecordNotFound) {
- return time.Time{}, nil
+ var pending time.Time
+ switch {
+ case errors.Is(err, gorm.ErrRecordNotFound):
+ // pending stays zero.
+ case err != nil:
+ return time.Time{}, err
+ default:
+ pending = p.ExpiresAt
}
+
+ authLink, err := s.nextAuthLinkExpiry(ctx)
if err != nil {
return time.Time{}, err
}
- return p.ExpiresAt, nil
+
+ switch {
+ case pending.IsZero():
+ return authLink, nil
+ case authLink.IsZero():
+ return pending, nil
+ case authLink.Before(pending):
+ return authLink, nil
+ default:
+ return pending, nil
+ }
+}
+
+// nextAuthLinkExpiry reads rastrillo's own auth_links table directly —
+// it is rastrillo/auth's table, not a store model, and rastrillo writes
+// to it over database/sql rather than gorm, storing expires_at as
+// RFC3339 text via now.UTC().Format(time.RFC3339)
+// (auth/store.go:83, auth/migrations/0001_init.sql). MIN() over that
+// text column sorts correctly because every row is formatted the same
+// fixed-width, UTC way, so lexicographic and chronological order agree.
+// The zero time means no row; an empty table's MIN() is a SQL NULL, not
+// an empty string, hence sql.NullString rather than a bare string.
+func (s *store) nextAuthLinkExpiry(ctx context.Context) (time.Time, error) {
+ var raw sql.NullString
+ if err := s.g.WithContext(ctx).Raw("SELECT MIN(expires_at) FROM auth_links").Row().Scan(&raw); err != nil {
+ return time.Time{}, err
+ }
+ if !raw.Valid || raw.String == "" {
+ return time.Time{}, nil
+ }
+ t, err := time.Parse(time.RFC3339, raw.String)
+ if err != nil {
+ return time.Time{}, fmt.Errorf("meet: parse auth_links.expires_at: %w", err)
+ }
+ return t, nil
}
diff --git a/internal/meet/store_test.go b/internal/meet/store_test.go
index b25afa0..776517c 100644
--- a/internal/meet/store_test.go
+++ b/internal/meet/store_test.go
@@ -326,6 +326,60 @@ func TestNextExpiryIsTheEarliestExpiryNotTheLatestRow(t *testing.T) {
}
}
+// Re-review of 9e067e4: a plain member sign-in (no meeting-link cookie
+// in play) writes no pending_joins row at all — that table exists for
+// the outsider/pending-join path, spec §3.2 — only rastrillo's own
+// auth_links row for the magic link it mailed. nextExpiry has to read
+// that table too, or an app that hibernates with nothing outstanding
+// but one unclicked member link is never woken to sweep it. rastrillo
+// writes auth_links.expires_at as RFC3339 text, UTC
+// (auth/store.go:83); this test writes a row the same way, bypassing
+// gorm entirely since auth_links is rastrillo's table, not a model
+// here.
+func TestNextExpiryAlsoReadsAuthLinks(t *testing.T) {
+ s, now := testStore(t)
+ ctx := context.Background()
+
+ if got, err := s.nextExpiry(ctx); err != nil || !got.IsZero() {
+ t.Fatalf("nextExpiry with nothing outstanding: %v (err %v), want the zero time", got, err)
+ }
+
+ authExpiry := now.Add(20 * time.Minute).UTC()
+ if err := s.g.WithContext(ctx).Exec(
+ `INSERT INTO auth_links (hash, address, purpose, expires_at) VALUES (?, ?, ?, ?)`,
+ "h1", "a@b.test", "signin", authExpiry.Format(time.RFC3339),
+ ).Error; err != nil {
+ t.Fatal(err)
+ }
+ if got, err := s.nextExpiry(ctx); err != nil || !got.Equal(authExpiry) {
+ t.Fatalf("nextExpiry with only an auth_links row: %v (err %v), want %v", got, err, authExpiry)
+ }
+
+ // A pending row that expires sooner (15 minutes out) than the
+ // auth_links row (20 minutes out) must win.
+ _, tok, _ := s.createMeeting(ctx, "x", nil, "person:1")
+ l, _, _, _ := s.liveLink(ctx, tok)
+ if err := s.writePending(ctx, "guest@example.com", l); err != nil {
+ t.Fatal(err)
+ }
+ pendingExpiry := now.Add(pendingLife).UTC()
+ if got, err := s.nextExpiry(ctx); err != nil || !got.Equal(pendingExpiry) {
+ t.Fatalf("nextExpiry with a sooner pending row: %v (err %v), want %v (the pending row)", got, err, pendingExpiry)
+ }
+
+ // Move the auth_links row to be the sooner one instead; it must win
+ // back.
+ soonerAuth := now.Add(time.Minute).UTC()
+ if err := s.g.WithContext(ctx).Exec(
+ `UPDATE auth_links SET expires_at = ? WHERE hash = ?`, soonerAuth.Format(time.RFC3339), "h1",
+ ).Error; err != nil {
+ t.Fatal(err)
+ }
+ if got, err := s.nextExpiry(ctx); err != nil || !got.Equal(soonerAuth) {
+ t.Fatalf("nextExpiry with a sooner auth_links row: %v (err %v), want %v", got, err, soonerAuth)
+ }
+}
+
func TestPendingIsKeyedFingerprintAndFollowsItsLink(t *testing.T) {
s, now := testStore(t)
ctx := context.Background()
diff --git a/internal/meettest/signin_test.go b/internal/meettest/signin_test.go
index 270c2cd..6a35cc9 100644
--- a/internal/meettest/signin_test.go
+++ b/internal/meettest/signin_test.go
@@ -11,6 +11,7 @@ import (
"path/filepath"
"strings"
"testing"
+ "time"
"amadan.net/rastrillo/rastrillo/db"
@@ -115,3 +116,30 @@ var errFakeSMTPFailure = errors.New("550 mailbox unavailable")
type failingMailer struct{ err error }
func (f failingMailer) Send(ctx context.Context, to, subject, body string) error { return f.err }
+
+// Re-review of 9e067e4: a plain member sign-in — no meeting-link cookie,
+// so no pending_joins row at all — still leaves rastrillo's own
+// auth_links row for the magic link it mailed. nextExpiry (and so
+// NextDue) has to see that row, or an app that hibernates right after
+// would never be woken to sweep it.
+func TestNextDueWakesForAPlainMemberSignin(t *testing.T) {
+ x := newHarness(t, "paul@example.com")
+ if rec := x.browser(t).post("/signin", url.Values{"address": {"paul@example.com"}}); rec.Header().Get("Location") != "/signin?sent=1" {
+ t.Fatalf("sign-in request: %s", rec.Header().Get("Location"))
+ }
+ if n := x.authLinks(t); n != 1 {
+ t.Fatalf("auth_links rows after a plain sign-in: %d, want 1", n)
+ }
+
+ next := x.ctl.NextDue()
+ if next.IsZero() {
+ t.Fatal("NextDue is zero after a plain sign-in left an unclicked magic link outstanding")
+ }
+ // signin.Flow.LinkTTL defaults to 15 minutes
+ // (keymaildev/signin@v0.1.1/flow.go:57,186); the wide window is
+ // about not coupling this test to that constant, not about
+ // precision.
+ if until := next.Sub(x.now); until <= 0 || until > 20*time.Minute {
+ t.Fatalf("NextDue is %v away, want a positive duration within the magic link's TTL window", until)
+ }
+}