From c29966aabf12b9b5b9929fe7785de28ee2da49de Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Wed, 24 Jun 2026 16:50:55 +0200 Subject: [PATCH 01/11] fix(console): reduce auth methods search limit from 200 to 100 --- console/src/views/settings/clients/store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/console/src/views/settings/clients/store.ts b/console/src/views/settings/clients/store.ts index beb5a1ad..12c5d2fb 100644 --- a/console/src/views/settings/clients/store.ts +++ b/console/src/views/settings/clients/store.ts @@ -19,7 +19,7 @@ import { // pagination if projects start exceeding this. export function useClients(projectId: UUID) { const [result, , reload, loading] = useResolver( - useCallback(() => api.authMethods.search(projectId, { limit: 200 }), [projectId]), + useCallback(() => api.authMethods.search(projectId, { limit: 100 }), [projectId]), ) const clients = (result?.results ?? []).map(authMethodToClient) return { clients, loading, reload } From 91fc9cd0ab0f0a0f0dbbc3390029dca1da821b12 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Thu, 25 Jun 2026 17:13:08 +0200 Subject: [PATCH 02/11] fix(scheduled): only report "Sending" for events that are due MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console renders a "Sending" badge for a scheduled message whenever `has_pending_events` is true. That field was computed as "any unfired event exists", but recurring schedules pre-generate the next occurrence's event the moment they advance (anchor + N*interval). As a result every recurring schedule perpetually had an unfired future event and was shown as "Sending" indefinitely, even while idle months before its next run. Scope `has_pending_events` to events that are actually due (`fired_at IS NULL AND fire_at <= NOW()`) in ListUserSchedules and ListOrganizationSchedules. This matches the existing scan-due predicate and the "up to 1 minute to process" tooltip: the badge now reflects an event being actively delivered, not one queued for a future occurrence. No frontend change needed — the badge keys solely off this field. --- internal/store/subjects/scheduled.go | 4 +- internal/store/subjects/scheduled_test.go | 57 +++++++++++++++++------ 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/internal/store/subjects/scheduled.go b/internal/store/subjects/scheduled.go index 46545df0..ed73ebd3 100644 --- a/internal/store/subjects/scheduled.go +++ b/internal/store/subjects/scheduled.go @@ -681,7 +681,7 @@ func (s *ScheduledStore) ListUserSchedules(ctx context.Context, projectID, userI us.occurrence, COALESCE(us.data, '{}'::jsonb) AS data, us.paused_at, us.created_at, us.updated_at, EXISTS ( SELECT 1 FROM user_scheduled_events use - WHERE use.user_schedule_id = us.id AND use.fired_at IS NULL + WHERE use.user_schedule_id = us.id AND use.fired_at IS NULL AND use.fire_at <= NOW() ) AS has_pending_events, COUNT(*) OVER () AS total_count FROM user_schedules us @@ -1288,7 +1288,7 @@ func (s *ScheduledStore) ListOrganizationSchedules(ctx context.Context, projectI os.occurrence, COALESCE(os.data, '{}'::jsonb) AS data, os.paused_at, os.created_at, os.updated_at, EXISTS ( SELECT 1 FROM organization_scheduled_events ose - WHERE ose.organization_schedule_id = os.id AND ose.fired_at IS NULL + WHERE ose.organization_schedule_id = os.id AND ose.fired_at IS NULL AND ose.fire_at <= NOW() ) AS has_pending_events, COUNT(*) OVER () AS total_count FROM organization_schedules os diff --git a/internal/store/subjects/scheduled_test.go b/internal/store/subjects/scheduled_test.go index e9658ddc..4b43abff 100644 --- a/internal/store/subjects/scheduled_test.go +++ b/internal/store/subjects/scheduled_test.go @@ -646,20 +646,33 @@ func TestListUserSchedulesHasPendingEvents(t *testing.T) { _, err = db.CreateUserSchedule(ctx, userID, scheduleID, &futureTime, nil, nil, json.RawMessage(`{}`)) require.NoError(t, err) - // While the generated event is unfired, the schedule reports pending events. + // An unfired event whose fire_at is still in the future is NOT "sending": + // has_pending_events only reflects events that are actually due. (Regression: + // recurring schedules pre-generate the next occurrence's event, which kept the + // "Sending" badge lit indefinitely.) items, _, err := db.ListUserSchedules(ctx, projectID, userID, store.Pagination{Limit: 10, Offset: 0}) require.NoError(t, err) require.Len(t, items, 1) - require.True(t, items[0].HasPendingEvents) + require.False(t, items[0].HasPendingEvents) - // Firing every event must clear the flag (regression: the badge previously - // relied on scheduled_at <= now and stayed lit forever). events, err := db.ListPendingScheduledEventsForUser(ctx, userID, scheduleID) require.NoError(t, err) - require.NotEmpty(t, events) - for _, e := range events { - require.NoError(t, db.MarkScheduledEventFired(ctx, e.ID)) - } + require.Len(t, events, 1) + + // Once the event is due (fire_at <= now), the schedule reports pending events. + pastTime := time.Now().Add(-1 * time.Hour).UTC().Truncate(time.Microsecond) + _, err = db.ScheduledStore.db.ExecContext(ctx, + `UPDATE user_scheduled_events SET fire_at = $1 WHERE id = $2`, + pastTime, events[0].ID) + require.NoError(t, err) + + items, _, err = db.ListUserSchedules(ctx, projectID, userID, store.Pagination{Limit: 10, Offset: 0}) + require.NoError(t, err) + require.Len(t, items, 1) + require.True(t, items[0].HasPendingEvents) + + // Firing the event clears the flag. + require.NoError(t, db.MarkScheduledEventFired(ctx, events[0].ID)) items, _, err = db.ListUserSchedules(ctx, projectID, userID, store.Pagination{Limit: 10, Offset: 0}) require.NoError(t, err) @@ -1185,19 +1198,33 @@ func TestListOrganizationSchedulesHasPendingEvents(t *testing.T) { _, err = db.CreateOrganizationSchedule(ctx, orgID, scheduleID, &futureTime, nil, nil, json.RawMessage(`{}`)) require.NoError(t, err) - // While the generated event is unfired, the schedule reports pending events. + // An unfired event whose fire_at is still in the future is NOT "sending": + // has_pending_events only reflects events that are actually due. (Regression: + // recurring schedules pre-generate the next occurrence's event, which kept the + // "Sending" badge lit indefinitely.) items, _, err := db.ListOrganizationSchedules(ctx, projectID, orgID, store.Pagination{Limit: 10, Offset: 0}) require.NoError(t, err) require.Len(t, items, 1) - require.True(t, items[0].HasPendingEvents) + require.False(t, items[0].HasPendingEvents) - // Firing every event must clear the flag. events, err := db.ListPendingOrgScheduledEventsForOrg(ctx, orgID, scheduleID) require.NoError(t, err) - require.NotEmpty(t, events) - for _, e := range events { - require.NoError(t, db.MarkOrgScheduledEventFired(ctx, e.ID)) - } + require.Len(t, events, 1) + + // Once the event is due (fire_at <= now), the schedule reports pending events. + pastTime := time.Now().Add(-1 * time.Hour).UTC().Truncate(time.Microsecond) + _, err = db.ScheduledStore.db.ExecContext(ctx, + `UPDATE organization_scheduled_events SET fire_at = $1 WHERE id = $2`, + pastTime, events[0].ID) + require.NoError(t, err) + + items, _, err = db.ListOrganizationSchedules(ctx, projectID, orgID, store.Pagination{Limit: 10, Offset: 0}) + require.NoError(t, err) + require.Len(t, items, 1) + require.True(t, items[0].HasPendingEvents) + + // Firing the event clears the flag. + require.NoError(t, db.MarkOrgScheduledEventFired(ctx, events[0].ID)) items, _, err = db.ListOrganizationSchedules(ctx, projectID, orgID, store.Pagination{Limit: 10, Offset: 0}) require.NoError(t, err) From dc1ce87e66027669f790ba3ecfd49d5bb584f22b Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Thu, 25 Jun 2026 17:31:32 +0200 Subject: [PATCH 03/11] fix(scheduled): compute next occurrence without a 10000-interval cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recurring schedules advanced to their next occurrence with a generate_series(1, 10000) scan from the anchor. That scan both capped how far a schedule could be advanced and was O(N) in the number of skipped occurrences: a schedule dormant for more than 10000 intervals (e.g. a 1-minute interval idle for ~7 days, or hourly for ~1 year) could never resume — the advance query returned no row and errored every cycle. Replace the scan with a next_schedule_occurrence SQL function that estimates the occurrence number in O(1) from the average interval length and then corrects it exactly with a couple of interval-arithmetic loops. The estimate only seeds the loops; the returned occurrence is exact for any strictly-increasing interval and there is no upper bound on the catch-up gap. Both the user and organization advance paths and computeNextOccurrence now call it, removing the duplicated calendar math. Tested: exact known cases (yearly, month-end clamping, boundary, ~470yr dormancy), a 640-case property grid asserting the defining properties across leap years / mixed calendar intervals / horizons to year 3025, and rejection of non-positive intervals. --- ...84500000_next_schedule_occurrence.down.sql | 1 + ...1784500000_next_schedule_occurrence.up.sql | 65 +++++++++ internal/store/subjects/scheduled.go | 43 ++---- internal/store/subjects/scheduled_test.go | 136 ++++++++++++++++++ 4 files changed, 215 insertions(+), 30 deletions(-) create mode 100644 internal/store/subjects/migrations/1784500000_next_schedule_occurrence.down.sql create mode 100644 internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql diff --git a/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.down.sql b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.down.sql new file mode 100644 index 00000000..a4069ae9 --- /dev/null +++ b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.down.sql @@ -0,0 +1 @@ +DROP FUNCTION IF EXISTS next_schedule_occurrence(TIMESTAMPTZ, INTERVAL, BIGINT, TIMESTAMPTZ); diff --git a/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql new file mode 100644 index 00000000..959a16dc --- /dev/null +++ b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql @@ -0,0 +1,65 @@ +-- next_schedule_occurrence returns the next occurrence of a recurring schedule: +-- the smallest N > current_occurrence for which (anchor + N * step) is strictly +-- after as_of, together with that N so it can be persisted. The timestamp is +-- always multiplied from the anchor (anchor + N * step) rather than chained +-- (prev + step), which avoids the drift that calendar intervals such as +-- '1 month' would otherwise accumulate via month-end clamping. +-- +-- It replaces the previous generate_series(1, 10000) scan, which both capped how +-- far ahead a schedule could be advanced (a schedule dormant for more than 10000 +-- intervals could never resume) and was O(N) in the number of skipped occurrences. +-- +-- Accuracy: the result is EXACT, not approximate. The epoch-based estimate only +-- chooses where the correction loops start -- it never enters the result. The two +-- loops below settle on N purely via exact interval arithmetic (anchor + n * step), +-- so the returned N satisfies, for any strictly-increasing (positive) interval: +-- (1) anchor + N * step > as_of (in the future) +-- (2) anchor + (N - 1) * step <= as_of (it is the FIRST such occurrence) +-- unless N = current_occurrence + 1 +-- (3) N >= current_occurrence + 1 (never moves backwards) +-- This is the same triple the old scan satisfied, so behaviour is identical where +-- the old scan was in range and well-defined beyond it. The estimate is within +-- ~1.4% of the true average interval length, so the loops run only a handful of +-- iterations regardless of how far out as_of is -- there is no cap on the gap. +CREATE OR REPLACE FUNCTION next_schedule_occurrence( + anchor TIMESTAMPTZ, + step INTERVAL, + current_occurrence BIGINT, + as_of TIMESTAMPTZ DEFAULT NOW() +) RETURNS TABLE(next_at TIMESTAMPTZ, occurrence BIGINT) +LANGUAGE plpgsql +STABLE +AS $$ +DECLARE + n BIGINT; + step_secs DOUBLE PRECISION := EXTRACT(EPOCH FROM step); +BEGIN + IF step_secs <= 0 THEN + RAISE EXCEPTION 'schedule interval must be positive, got %', step; + END IF; + + -- O(1) estimate from the average interval length. EXTRACT(EPOCH FROM step) + -- is within ~1.4% of the true average length even for calendar intervals, + -- so the estimate is off by at most a handful of occurrences. + n := GREATEST( + current_occurrence + 1, + FLOOR(EXTRACT(EPOCH FROM (as_of - anchor)) / step_secs)::BIGINT + ); + + -- Correct exactly using interval arithmetic only. First walk back while the + -- previous occurrence is still in the future (estimate overshot), enforcing + -- property (3) via the n > current_occurrence + 1 guard. Then walk forward + -- while the current occurrence is not yet past as_of (estimate undershot). + -- On exit, anchor + (n-1)*step <= as_of < anchor + n*step, i.e. properties + -- (1) and (2) hold. Both loops run only a few iterations. + WHILE n > current_occurrence + 1 AND anchor + (n - 1) * step > as_of LOOP + n := n - 1; + END LOOP; + + WHILE anchor + n * step <= as_of LOOP + n := n + 1; + END LOOP; + + RETURN QUERY SELECT anchor + n * step, n; +END; +$$; diff --git a/internal/store/subjects/scheduled.go b/internal/store/subjects/scheduled.go index ed73ebd3..9cd88aaf 100644 --- a/internal/store/subjects/scheduled.go +++ b/internal/store/subjects/scheduled.go @@ -820,14 +820,7 @@ type nextOccurrenceResult struct { // such that anchor + N * interval > NOW(), and returns both the timestamp and // the occurrence number N so it can be persisted to avoid drift. func (s *ScheduledStore) computeNextOccurrence(ctx context.Context, interval string, anchor time.Time) (nextOccurrenceResult, error) { - stmt := ` - SELECT - $1::timestamptz + n * $2::interval AS next_at, - n AS occurrence - FROM generate_series(1, 10000) AS n - WHERE $1::timestamptz + n * $2::interval > NOW() - ORDER BY n - LIMIT 1` + stmt := `SELECT next_at, occurrence FROM next_schedule_occurrence($1::timestamptz, $2::interval, 0)` var result nextOccurrenceResult err := s.db.GetContext(ctx, &result, stmt, anchor, interval) @@ -918,19 +911,14 @@ func (s *ScheduledStore) AdvanceAndGenerateUserScheduleEvents(ctx context.Contex return nil } - // Atomically increment occurrence and compute scheduled_at = anchor_at + occurrence * interval. - // The new occurrence is the smallest N > current occurrence where anchor_at + N * interval > NOW(). + // Atomically advance occurrence and scheduled_at to the next future occurrence + // (anchor_at + N * interval, smallest N > current occurrence). See the + // next_schedule_occurrence migration: it has no upper bound on the catch-up gap. advanceStmt := ` UPDATE user_schedules - SET occurrence = sub.n, - scheduled_at = sub.next_at - FROM ( - SELECT n, $2::timestamptz + n * $3::interval AS next_at - FROM generate_series($4::int + 1, $4::int + 10000) AS n - WHERE $2::timestamptz + n * $3::interval > NOW() - ORDER BY n - LIMIT 1 - ) sub + SET occurrence = f.occurrence, + scheduled_at = f.next_at + FROM next_schedule_occurrence($2::timestamptz, $3::interval, $4::bigint) f WHERE id = $1 RETURNING user_schedules.scheduled_at, user_schedules.occurrence` @@ -1570,19 +1558,14 @@ func (s *ScheduledStore) AdvanceAndGenerateOrgScheduleEvents(ctx context.Context return nil } - // Atomically increment occurrence and compute scheduled_at = anchor_at + occurrence * interval. - // The new occurrence is the smallest N > current occurrence where anchor_at + N * interval > NOW(). + // Atomically advance occurrence and scheduled_at to the next future occurrence + // (anchor_at + N * interval, smallest N > current occurrence). See the + // next_schedule_occurrence migration: it has no upper bound on the catch-up gap. advanceStmt := ` UPDATE organization_schedules - SET occurrence = sub.n, - scheduled_at = sub.next_at - FROM ( - SELECT n, $2::timestamptz + n * $3::interval AS next_at - FROM generate_series($4::int + 1, $4::int + 10000) AS n - WHERE $2::timestamptz + n * $3::interval > NOW() - ORDER BY n - LIMIT 1 - ) sub + SET occurrence = f.occurrence, + scheduled_at = f.next_at + FROM next_schedule_occurrence($2::timestamptz, $3::interval, $4::bigint) f WHERE id = $1 RETURNING organization_schedules.scheduled_at, organization_schedules.occurrence` diff --git a/internal/store/subjects/scheduled_test.go b/internal/store/subjects/scheduled_test.go index 4b43abff..ed3f6aae 100644 --- a/internal/store/subjects/scheduled_test.go +++ b/internal/store/subjects/scheduled_test.go @@ -1859,3 +1859,139 @@ func TestOrgScheduleDataPropagatedToEvents(t *testing.T) { require.Len(t, events, 1) require.JSONEq(t, `{"meeting":"standup"}`, string(events[0].Data)) } + +// nextOccurrence calls the next_schedule_occurrence SQL function with an explicit +// as_of so results are deterministic, and returns the occurrence number, its +// timestamp, and whether the defining properties (1)-(3) hold for that result. +type occurrenceProps struct { + Occurrence int64 `db:"occurrence"` + NextAt time.Time `db:"next_at"` + P1Future bool `db:"p1_future"` + P2First bool `db:"p2_first"` + P3Forward bool `db:"p3_forward"` +} + +func nextOccurrence(t *testing.T, db *State, anchor time.Time, interval string, cur int64, asOf time.Time) occurrenceProps { + t.Helper() + // Properties are evaluated with interval arithmetic in Postgres (the only place + // calendar math is exact). next_at > as_of (in the future); the previous + // occurrence is at or before as_of unless it is the first one (it is the + // earliest such occurrence); and the occurrence never moves backwards. + const q = ` + WITH r AS ( + SELECT next_at, occurrence + FROM next_schedule_occurrence($1::timestamptz, $2::interval, $3::bigint, $4::timestamptz) + ) + SELECT + r.occurrence, + r.next_at, + (r.next_at > $4) AS p1_future, + (r.occurrence = $3 + 1 OR $1::timestamptz + (r.occurrence - 1) * $2::interval <= $4) AS p2_first, + (r.occurrence >= $3 + 1) AS p3_forward + FROM r` + + var pr occurrenceProps + err := db.ScheduledStore.db.QueryRowxContext(context.Background(), q, anchor, interval, cur, asOf).StructScan(&pr) + require.NoError(t, err) + return pr +} + +func TestNextScheduleOccurrenceExactCases(t *testing.T) { + t.Parallel() + + db := NewContainerStore(t) + + ts := func(s string) time.Time { + v, err := time.Parse(time.RFC3339, s) + require.NoError(t, err) + return v.UTC() + } + + cases := []struct { + name string + anchor string + interval string + cur int64 + asOf string + wantOcc int64 + wantNext string + }{ + // The stuck-"Sending" anniversary: yearly, first send is anchor + 1 year. + {"yearly anniversary", "2026-03-28T15:45:52Z", "1 year", 0, "2026-06-25T00:00:00Z", 1, "2027-03-28T15:45:52Z"}, + // as_of exactly on an occurrence boundary must pick the next one (strict >). + {"daily on boundary picks next", "2026-01-01T00:00:00Z", "1 day", 0, "2026-01-10T00:00:00Z", 10, "2026-01-11T00:00:00Z"}, + // Month-end clamping: Jan 31 + 1 month = Feb 29 (leap), + 2 months = Mar 31. + {"month-end clamp from Jan 31", "2024-01-31T12:00:00Z", "1 month", 0, "2024-03-15T00:00:00Z", 2, "2024-03-31T12:00:00Z"}, + // Advancing from a non-zero current occurrence only moves forward. + {"advance from current occurrence", "2026-01-01T00:00:00Z", "1 day", 5, "2026-01-03T00:00:00Z", 6, "2026-01-07T00:00:00Z"}, + // ~470 years of dormancy on a 1-minute interval: far past the old 10000 cap. + {"far dormancy beyond old cap", "2026-01-01T00:00:00Z", "1 minute", 0, "2496-01-01T00:00:00Z", 247196161, "2496-01-01T00:01:00Z"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + pr := nextOccurrence(t, db, ts(tc.anchor), tc.interval, tc.cur, ts(tc.asOf)) + require.Equal(t, tc.wantOcc, pr.Occurrence) + require.Truef(t, pr.NextAt.Equal(ts(tc.wantNext)), "next_at = %s, want %s", pr.NextAt, tc.wantNext) + require.True(t, pr.P1Future && pr.P2First && pr.P3Forward, "defining properties must hold") + }) + } +} + +// TestNextScheduleOccurrenceProperties asserts the result is exact across a broad +// grid (leap years, month-end clamping, mixed calendar intervals, horizons out to +// year 3025) by checking the three defining properties for every combination. +func TestNextScheduleOccurrenceProperties(t *testing.T) { + t.Parallel() + + db := NewContainerStore(t) + + ts := func(s string) time.Time { + v, err := time.Parse(time.RFC3339, s) + require.NoError(t, err) + return v.UTC() + } + + anchors := []string{ + "2024-01-31T12:00:00Z", "2020-12-31T06:00:00Z", + "2026-03-28T15:45:52Z", "1999-11-15T09:00:00Z", + } + intervals := []string{ + "1 minute", "1 hour", "1 day", "1 week", "30 days", + "1 month", "3 months", "1 year", "5 years", "1 mon 15 days", + } + curs := []int64{0, 1, 7, 99} + asOfs := []string{ + "2026-06-25T14:00:00Z", "2030-02-28T00:00:00Z", + "2520-06-01T00:00:00Z", "3025-09-09T00:00:00Z", + } + + for _, a := range anchors { + for _, iv := range intervals { + for _, cur := range curs { + for _, asOf := range asOfs { + pr := nextOccurrence(t, db, ts(a), iv, cur, ts(asOf)) + require.Truef(t, pr.P1Future && pr.P2First && pr.P3Forward, + "anchor=%s interval=%s cur=%d asOf=%s -> occ=%d (p1=%v p2=%v p3=%v)", + a, iv, cur, asOf, pr.Occurrence, pr.P1Future, pr.P2First, pr.P3Forward) + } + } + } + } +} + +func TestNextScheduleOccurrenceRejectsNonPositiveInterval(t *testing.T) { + t.Parallel() + + db := NewContainerStore(t) + + const q = `SELECT next_at FROM next_schedule_occurrence($1::timestamptz, $2::interval, 0, $3::timestamptz)` + for _, interval := range []string{"0 seconds", "-1 day"} { + var nextAt time.Time + err := db.ScheduledStore.db.QueryRowxContext(context.Background(), q, + time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), interval, + time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC)).Scan(&nextAt) + require.Error(t, err, "interval %q should be rejected", interval) + require.Contains(t, err.Error(), "interval must be positive") + } +} From 26ac6750745286f6ea54ea48fc7c445e0c227679 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Thu, 25 Jun 2026 17:46:38 +0200 Subject: [PATCH 04/11] test(scheduled): cover leap-day yearly occurrences; trim estimate comments --- ...1784500000_next_schedule_occurrence.up.sql | 10 +++----- internal/store/subjects/scheduled_test.go | 25 +++++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql index 959a16dc..a6bd8f01 100644 --- a/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql +++ b/internal/store/subjects/migrations/1784500000_next_schedule_occurrence.up.sql @@ -18,9 +18,8 @@ -- unless N = current_occurrence + 1 -- (3) N >= current_occurrence + 1 (never moves backwards) -- This is the same triple the old scan satisfied, so behaviour is identical where --- the old scan was in range and well-defined beyond it. The estimate is within --- ~1.4% of the true average interval length, so the loops run only a handful of --- iterations regardless of how far out as_of is -- there is no cap on the gap. +-- the old scan was in range and well-defined beyond it. The epoch estimate lands +-- close to N, so the loops do little work, and there is no cap on the gap. CREATE OR REPLACE FUNCTION next_schedule_occurrence( anchor TIMESTAMPTZ, step INTERVAL, @@ -38,9 +37,8 @@ BEGIN RAISE EXCEPTION 'schedule interval must be positive, got %', step; END IF; - -- O(1) estimate from the average interval length. EXTRACT(EPOCH FROM step) - -- is within ~1.4% of the true average length even for calendar intervals, - -- so the estimate is off by at most a handful of occurrences. + -- O(1) starting estimate from the average interval length. This only seeds + -- the correction loops below; the exact N comes from them, not from here. n := GREATEST( current_occurrence + 1, FLOOR(EXTRACT(EPOCH FROM (as_of - anchor)) / step_secs)::BIGINT diff --git a/internal/store/subjects/scheduled_test.go b/internal/store/subjects/scheduled_test.go index ed3f6aae..cbc0dc01 100644 --- a/internal/store/subjects/scheduled_test.go +++ b/internal/store/subjects/scheduled_test.go @@ -1907,29 +1907,34 @@ func TestNextScheduleOccurrenceExactCases(t *testing.T) { return v.UTC() } - cases := []struct { - name string + type testCase struct { anchor string interval string cur int64 asOf string wantOcc int64 wantNext string - }{ + } + + cases := map[string]testCase{ // The stuck-"Sending" anniversary: yearly, first send is anchor + 1 year. - {"yearly anniversary", "2026-03-28T15:45:52Z", "1 year", 0, "2026-06-25T00:00:00Z", 1, "2027-03-28T15:45:52Z"}, + "yearly anniversary": {"2026-03-28T15:45:52Z", "1 year", 0, "2026-06-25T00:00:00Z", 1, "2027-03-28T15:45:52Z"}, + // Leap-day anchor + yearly clamps to Feb 28 in common years (occurrence 3 = 2027). + "leap-day yearly clamps to Feb 28": {"2024-02-29T12:00:00Z", "1 year", 0, "2027-01-01T00:00:00Z", 3, "2027-02-28T12:00:00Z"}, + // Leap-day anchor + yearly lands back on Feb 29 every 4th year (occurrence 4 = 2028). + "leap-day yearly lands on Feb 29": {"2024-02-29T12:00:00Z", "1 year", 0, "2027-06-01T00:00:00Z", 4, "2028-02-29T12:00:00Z"}, // as_of exactly on an occurrence boundary must pick the next one (strict >). - {"daily on boundary picks next", "2026-01-01T00:00:00Z", "1 day", 0, "2026-01-10T00:00:00Z", 10, "2026-01-11T00:00:00Z"}, + "daily on boundary picks next": {"2026-01-01T00:00:00Z", "1 day", 0, "2026-01-10T00:00:00Z", 10, "2026-01-11T00:00:00Z"}, // Month-end clamping: Jan 31 + 1 month = Feb 29 (leap), + 2 months = Mar 31. - {"month-end clamp from Jan 31", "2024-01-31T12:00:00Z", "1 month", 0, "2024-03-15T00:00:00Z", 2, "2024-03-31T12:00:00Z"}, + "month-end clamp from Jan 31": {"2024-01-31T12:00:00Z", "1 month", 0, "2024-03-15T00:00:00Z", 2, "2024-03-31T12:00:00Z"}, // Advancing from a non-zero current occurrence only moves forward. - {"advance from current occurrence", "2026-01-01T00:00:00Z", "1 day", 5, "2026-01-03T00:00:00Z", 6, "2026-01-07T00:00:00Z"}, + "advance from current occurrence": {"2026-01-01T00:00:00Z", "1 day", 5, "2026-01-03T00:00:00Z", 6, "2026-01-07T00:00:00Z"}, // ~470 years of dormancy on a 1-minute interval: far past the old 10000 cap. - {"far dormancy beyond old cap", "2026-01-01T00:00:00Z", "1 minute", 0, "2496-01-01T00:00:00Z", 247196161, "2496-01-01T00:01:00Z"}, + "far dormancy beyond old cap": {"2026-01-01T00:00:00Z", "1 minute", 0, "2496-01-01T00:00:00Z", 247196161, "2496-01-01T00:01:00Z"}, } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { + for name, tc := range cases { + t.Run(name, func(t *testing.T) { pr := nextOccurrence(t, db, ts(tc.anchor), tc.interval, tc.cur, ts(tc.asOf)) require.Equal(t, tc.wantOcc, pr.Occurrence) require.Truef(t, pr.NextAt.Equal(ts(tc.wantNext)), "next_at = %s, want %s", pr.NextAt, tc.wantNext) From 39a0d27cfa7fe864b46c5532076bdac47c74df47 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Fri, 26 Jun 2026 13:26:39 +0200 Subject: [PATCH 05/11] fix(console): re-enable campaign create after toggling transactional off The create button stayed disabled after enabling and then disabling the transactional toggle, even with a subscription selected. Toggling transactional on cleared subscription_id, and the auto-select effect restored a subscription via form.setValue without revalidating, leaving formState.isValid stale at false. Pass shouldValidate so the form revalidates when subscription_id is set programmatically. --- console/src/views/campaign/NewCampaign.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/console/src/views/campaign/NewCampaign.tsx b/console/src/views/campaign/NewCampaign.tsx index e9340a53..96e5c332 100644 --- a/console/src/views/campaign/NewCampaign.tsx +++ b/console/src/views/campaign/NewCampaign.tsx @@ -105,7 +105,9 @@ export default function NewCampaign() { ) if (!hasValidSelection) { - form.setValue("subscription_id", filteredSubscriptions[0].id) + form.setValue("subscription_id", filteredSubscriptions[0].id, { + shouldValidate: true, + }) } }, [isTransactional, subscriptionsLoading, filteredSubscriptions, subscriptionId, form]) @@ -323,7 +325,9 @@ export default function NewCampaign() { onCheckedChange={(checked) => { field.onChange(checked) if (checked) { - form.setValue("subscription_id", "") + form.setValue("subscription_id", "", { + shouldValidate: true, + }) } }} /> From 96e37b838a57ff73ba17b12a4cddac53d27e8f85 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Fri, 26 Jun 2026 13:49:31 +0200 Subject: [PATCH 06/11] fix(journey): auto-assign entrance data_key so input event variables are usable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate (and other downstream) variable picker only surfaces a step's event variables when that step has a `data_key` — the backend likewise only exposes step state under `journey..*`. Entrance steps never got a data_key assigned automatically (it was a manual, easily-missed field), so the triggering "input" event's variables never appeared in the picker; only the always-present "User" group showed. Give entrances a readable, unique default data_key ("entrance", "entrance_2", …): - at creation, in both the drag-drop and trigger-setup paths, and - as a non-destructive backfill on load for entrances that predate this, persisted on the next manual save. --- .../editor/JourneyEditor.utils.test.ts | 49 +++++++++++++++++++ .../journey/editor/JourneyEditor.utils.ts | 29 ++++++++++- .../journey/hooks/useJourneyEditorGraph.ts | 8 ++- .../journey/hooks/useJourneyFlowHandlers.ts | 16 +++++- 4 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 console/src/views/journey/editor/JourneyEditor.utils.test.ts diff --git a/console/src/views/journey/editor/JourneyEditor.utils.test.ts b/console/src/views/journey/editor/JourneyEditor.utils.test.ts new file mode 100644 index 00000000..f1daae6d --- /dev/null +++ b/console/src/views/journey/editor/JourneyEditor.utils.test.ts @@ -0,0 +1,49 @@ +// Unit tests for the entrance data_key defaulting: entrances need a data_key for +// their trigger ("input") event variables to be referenceable downstream as +// `journey..data.*`, so we assign one automatically and backfill any +// legacy entrances that lack one. + +import { describe, expect, it } from "vitest" + +import { defaultEntranceDataKey, stepsToNodes } from "./JourneyEditor.utils" +import type { JourneyStepMap } from "@/types" + +describe("defaultEntranceDataKey", () => { + it("uses 'entrance' when free", () => { + expect(defaultEntranceDataKey([])).toBe("entrance") + }) + + it("falls back to a numbered suffix when taken", () => { + expect(defaultEntranceDataKey(["entrance"])).toBe("entrance_2") + expect(defaultEntranceDataKey(["entrance", "entrance_2"])).toBe("entrance_3") + }) + + it("ignores gaps and unrelated keys", () => { + expect(defaultEntranceDataKey(["entrance", "purchase", "entrance_3"])).toBe("entrance_2") + }) +}) + +describe("stepsToNodes entrance data_key backfill", () => { + it("assigns a default data_key to entrances that lack one", () => { + const steps: JourneyStepMap = { + a: { type: "entrance", name: "Entry", x: 0, y: 0 }, + b: { type: "gate", name: "Gate", x: 0, y: 100 }, + } + const { nodes } = stepsToNodes(steps, {}) + const entrance = nodes.find((n) => n.id === "a") + const gate = nodes.find((n) => n.id === "b") + expect(entrance?.data.data_key).toBe("entrance") + // Non-entrance steps are left untouched. + expect(gate?.data.data_key).toBeUndefined() + }) + + it("preserves an existing data_key and de-duplicates against it", () => { + const steps: JourneyStepMap = { + a: { type: "entrance", name: "Entry 1", data_key: "entrance", x: 0, y: 0 }, + b: { type: "entrance", name: "Entry 2", x: 200, y: 0 }, + } + const { nodes } = stepsToNodes(steps, {}) + expect(nodes.find((n) => n.id === "a")?.data.data_key).toBe("entrance") + expect(nodes.find((n) => n.id === "b")?.data.data_key).toBe("entrance_2") + }) +}) diff --git a/console/src/views/journey/editor/JourneyEditor.utils.ts b/console/src/views/journey/editor/JourneyEditor.utils.ts index 9ff43a38..f11f89aa 100644 --- a/console/src/views/journey/editor/JourneyEditor.utils.ts +++ b/console/src/views/journey/editor/JourneyEditor.utils.ts @@ -100,6 +100,21 @@ export function createEdge({ data, sourceId, targetId, path }: CreateEdgeParams) } } +/** + * The trigger event payload captured by an entrance is only referenceable from + * downstream steps (gates, campaigns, …) as `journey..data.*`, and the + * backend only exposes a step's state when it has a non-empty `data_key`. Pick a + * readable, unique default ("entrance", "entrance_2", …) so the input event + * variables are available without the user having to discover the data_key field. + */ +export function defaultEntranceDataKey(existingKeys: Iterable): string { + const used = new Set(existingKeys) + if (!used.has("entrance")) return "entrance" + let i = 2 + while (used.has(`entrance_${i}`)) i++ + return `entrance_${i}` +} + export function stepsToNodes( stepMap: JourneyStepMap, actions: { @@ -113,6 +128,11 @@ export function stepsToNodes( const entries = Object.entries(stepMap) const nodeIds = new Set(entries.map(([id]) => id)) + // Backfill a default data_key for entrances that predate auto-assignment so + // their input event variables surface in downstream pickers. This only fills + // empty keys (non-destructive) and is persisted on the next manual save. + const usedDataKeys = new Set(entries.map(([, s]) => s.data_key).filter((k): k is string => !!k)) + for (const [ id, { x, y, type, data, name, data_key, children, stats, stats_at, id: stepId }, @@ -120,6 +140,13 @@ export function stepsToNodes( const { width, height, ...restData } = (data as Record) ?? {} const sizeStyle = typeof width === "number" && typeof height === "number" ? { width, height } : undefined + + let resolvedDataKey = data_key + if (!resolvedDataKey && type === "entrance") { + resolvedDataKey = defaultEntranceDataKey(usedDataKeys) + usedDataKeys.add(resolvedDataKey) + } + nodes.push({ id, position: { x, y }, @@ -128,7 +155,7 @@ export function stepsToNodes( data: { type, name, - data_key, + data_key: resolvedDataKey, data: restData, stats, stats_at, diff --git a/console/src/views/journey/hooks/useJourneyEditorGraph.ts b/console/src/views/journey/hooks/useJourneyEditorGraph.ts index 758e7288..5c0eefa3 100644 --- a/console/src/views/journey/hooks/useJourneyEditorGraph.ts +++ b/console/src/views/journey/hooks/useJourneyEditorGraph.ts @@ -8,7 +8,12 @@ import type { EntranceTrigger } from "../components/JourneyTriggerSetup" import type { JourneyEditorActionsValue } from "../editor/JourneyEditorActions" import type { JourneyHintsValue } from "../editor/JourneyHints" import type { JourneyEdge, JourneyNode } from "../editor/JourneyEditor.types" -import { cloneNodes, getStepType, isValidJourneyConnection } from "../editor/JourneyEditor.utils" +import { + cloneNodes, + defaultEntranceDataKey, + getStepType, + isValidJourneyConnection, +} from "../editor/JourneyEditor.utils" import { useJourneyFlowHandlers } from "./useJourneyFlowHandlers" import { useKeyboardShortcuts } from "./useKeyboardShortcuts" import { useStepEditing } from "./useStepEditing" @@ -347,6 +352,7 @@ export function useJourneyEditorGraph({ data: { type: "entrance", name: t("entrance"), + data_key: defaultEntranceDataKey([]), data: { ...defaultData, trigger }, editing: true, }, diff --git a/console/src/views/journey/hooks/useJourneyFlowHandlers.ts b/console/src/views/journey/hooks/useJourneyFlowHandlers.ts index 4b8ed080..8442a1e6 100644 --- a/console/src/views/journey/hooks/useJourneyFlowHandlers.ts +++ b/console/src/views/journey/hooks/useJourneyFlowHandlers.ts @@ -3,7 +3,11 @@ import { addEdge, type Connection, MarkerType, type ReactFlowInstance } from "@x import { useTranslation } from "react-i18next" import { createUuid } from "@/utils" import { DATA_FORMAT, STEP_STYLE } from "./JourneyEditor.constants" -import { getStepType, isValidJourneyConnection } from "../editor/JourneyEditor.utils" +import { + defaultEntranceDataKey, + getStepType, + isValidJourneyConnection, +} from "../editor/JourneyEditor.utils" import type { JourneyEdge, JourneyNode } from "../editor/JourneyEditor.types" export function useJourneyFlowHandlers( @@ -74,6 +78,15 @@ export function useJourneyFlowHandlers( if (entranceCount > 0) name = `${t("entrance")} ${entranceCount + 1}` } + // Give entrances a default data_key so their trigger event data is + // immediately referenceable downstream as `journey..data.*`. + const data_key = + payload.type === "entrance" + ? defaultEntranceDataKey( + nds.map((n) => n.data.data_key).filter((k): k is string => !!k), + ) + : undefined + const newNode: JourneyNode = { id: createUuid(), position: { x, y }, @@ -81,6 +94,7 @@ export function useJourneyFlowHandlers( data: { type: payload.type, name, + data_key, data, ...(isSticky ? { width: 275, height: 150 } : {}), }, From af486ff995e34d382f025290dc657e7228e4654a Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Fri, 26 Jun 2026 13:52:13 +0200 Subject: [PATCH 07/11] fix(journey): default exit to the first entrance on placement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An exit step requires an `entrance_uuid` (the entrance it terminates), but it was always empty on drop, forcing a manual pick and failing validation until set. Default it to the first entrance when placed, so single-entrance journeys — the common case — need no extra configuration. --- .../views/journey/hooks/useJourneyFlowHandlers.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/console/src/views/journey/hooks/useJourneyFlowHandlers.ts b/console/src/views/journey/hooks/useJourneyFlowHandlers.ts index 8442a1e6..2f21fba0 100644 --- a/console/src/views/journey/hooks/useJourneyFlowHandlers.ts +++ b/console/src/views/journey/hooks/useJourneyFlowHandlers.ts @@ -87,6 +87,17 @@ export function useJourneyFlowHandlers( ) : undefined + // An exit must target an entrance (required field). Default to the + // first entrance so single-entrance journeys need no manual pick. + let stepData = data + if ( + payload.type === "exit" && + !(stepData as { entrance_uuid?: string }).entrance_uuid + ) { + const firstEntrance = nds.find((n) => n.data.type === "entrance") + if (firstEntrance) stepData = { ...stepData, entrance_uuid: firstEntrance.id } + } + const newNode: JourneyNode = { id: createUuid(), position: { x, y }, @@ -95,7 +106,7 @@ export function useJourneyFlowHandlers( type: payload.type, name, data_key, - data, + data: stepData, ...(isSticky ? { width: 275, height: 150 } : {}), }, ...(isSticky ? { style: { width: 275, height: 150 } } : {}), From caa1173347837870103927fb986524ef3eb885fa Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Fri, 26 Jun 2026 13:40:34 +0200 Subject: [PATCH 08/11] fix(console): make email preview compact in Journey Send node The Journey editor's Send node renders the email preview inside a small 250x200 thumbnail (size="small"), but EmailFrame always laid out at full scale: 22px subject, large avatar, generous padding, and a full row of Gmail action icons. Crammed into the thumbnail this wrapped the subject, sender name, and timestamp onto multiple lines and overlapped the reply icon, leaving the preview looking broken. Thread the existing `size` prop through to EmailFrame and add a compact layout for "small": tighter padding, smaller subject/sender/avatar type, truncation instead of wrapping, and hidden action icons. Full-size previews are unchanged. --- console/src/components/preview.tsx | 2 +- console/src/components/preview/EmailFrame.tsx | 100 +++++++++++++----- 2 files changed, 73 insertions(+), 29 deletions(-) diff --git a/console/src/components/preview.tsx b/console/src/components/preview.tsx index 5d445a4e..7774d88f 100644 --- a/console/src/components/preview.tsx +++ b/console/src/components/preview.tsx @@ -83,7 +83,7 @@ function EmailPreviewContent({ }, [data?.code?.source]) return ( - +