diff --git a/server/internal/app/openapi/openapi.yaml b/server/internal/app/openapi/openapi.yaml index 6504b912c..04af08f44 100644 --- a/server/internal/app/openapi/openapi.yaml +++ b/server/internal/app/openapi/openapi.yaml @@ -2730,9 +2730,24 @@ paths: - $ref: "#/components/parameters/CheckUidPath" - name: cursor in: query - description: Cursor for pagination + description: >- + Opaque keyset cursor; pass the previous response's `pagination.cursor` back + verbatim. A cursor that cannot be parsed starts from the first page. With + `order=asc` it means "strictly after this row" and the response always carries + a cursor for its last row, even on the last page (an empty page returns the + cursor it was given), so a consumer stores it and resumes from it later. + schema: + type: string + - name: order + in: query + description: >- + Sort order on `(created_at, uid)`. `desc` (default) is newest first. `asc` is oldest + first and only returns events older than 5 seconds (settle window), so a + consumer sees a row at most 5 s late and never skips one. schema: type: string + enum: [desc, asc] + default: desc - name: size in: query description: Results per page (default 20, max 100) @@ -3535,9 +3550,25 @@ paths: example: "degraded" - name: cursor in: query - description: Cursor for pagination + description: >- + Opaque keyset cursor; pass the previous response's `pagination.cursor` back + verbatim. A cursor that cannot be parsed starts from the first page. With + `order=asc` it means "strictly after this row" and the response always carries + a cursor for its last row, even on the last page (an empty page returns the + cursor it was given), so a consumer stores it and resumes from it later. + schema: + type: string + - name: order + in: query + description: >- + Sort order on `(started_at, uid)`. `desc` (default) is newest first. `asc` is oldest + first and only returns incidents older than 5 seconds (settle window), so a + consumer sees a row at most 5 s late and never skips one. + `asc` lists new incidents only: changes to existing incidents (acked, resolved, reopened) are events, follow `GET /events?type=incident&order=asc`. schema: type: string + enum: [desc, asc] + default: desc - name: size in: query description: Results per page (default 20, max 100) @@ -3609,9 +3640,24 @@ paths: - $ref: "#/components/parameters/IncidentUidPath" - name: cursor in: query - description: Cursor for pagination + description: >- + Opaque keyset cursor; pass the previous response's `pagination.cursor` back + verbatim. A cursor that cannot be parsed starts from the first page. With + `order=asc` it means "strictly after this row" and the response always carries + a cursor for its last row, even on the last page (an empty page returns the + cursor it was given), so a consumer stores it and resumes from it later. + schema: + type: string + - name: order + in: query + description: >- + Sort order on `(created_at, uid)`. `desc` (default) is newest first. `asc` is oldest + first and only returns events older than 5 seconds (settle window), so a + consumer sees a row at most 5 s late and never skips one. schema: type: string + enum: [desc, asc] + default: desc - name: size in: query description: Results per page (default 20, max 100) @@ -4771,9 +4817,24 @@ paths: format: date-time - name: cursor in: query - description: Cursor for pagination + description: >- + Opaque keyset cursor; pass the previous response's `pagination.cursor` back + verbatim. A cursor that cannot be parsed starts from the first page. With + `order=asc` it means "strictly after this row" and the response always carries + a cursor for its last row, even on the last page (an empty page returns the + cursor it was given), so a consumer stores it and resumes from it later. + schema: + type: string + - name: order + in: query + description: >- + Sort order on `(created_at, uid)`. `desc` (default) is newest first. `asc` is oldest + first and only returns events older than 5 seconds (settle window), so a + consumer sees a row at most 5 s late and never skips one. schema: type: string + enum: [desc, asc] + default: desc - name: size in: query description: Results per page (default 20, max 100) diff --git a/server/internal/db/models/event.go b/server/internal/db/models/event.go index d7a1665c1..4effead9b 100644 --- a/server/internal/db/models/event.go +++ b/server/internal/db/models/event.go @@ -459,6 +459,13 @@ type ListEventsFilter struct { CursorTimestamp *time.Time // Optional: events with created_at < this timestamp CursorUID *string // Optional: for same timestamp, events with UID < this + // Ascending lists oldest first on (created_at, uid); the cursor then means + // "strictly after" and the comparisons flip. + Ascending bool + // SettledBefore only admits events created before this instant (forward + // listings hold back rows that may still be committed out of order). + SettledBefore *time.Time + Limit int // Optional: pagination limit } diff --git a/server/internal/db/models/incident.go b/server/internal/db/models/incident.go index 6765a956f..a08664330 100644 --- a/server/internal/db/models/incident.go +++ b/server/internal/db/models/incident.go @@ -232,6 +232,12 @@ type ListIncidentsFilter struct { CursorTimestamp *time.Time // Optional: incidents with started_at < this timestamp CursorUID *string // Optional: for same timestamp, incidents with UID < this + // Ascending lists oldest first on (started_at, uid); the cursor then means + // "strictly after" and the comparisons flip. + Ascending bool + // SettledBefore only admits incidents started before this instant. + SettledBefore *time.Time + Limit int // Optional: pagination limit } diff --git a/server/internal/db/postgres/postgres.go b/server/internal/db/postgres/postgres.go index ac230fd76..d3c7d5ad0 100644 --- a/server/internal/db/postgres/postgres.go +++ b/server/internal/db/postgres/postgres.go @@ -4150,8 +4150,16 @@ func (s *Service) ListIncidents( ) ([]*models.Incident, int64, error) { var incidents []*models.Incident + order := []string{"started_at DESC", "uid DESC"} + cmp := "<" + + if filter.Ascending { + order = []string{"started_at ASC", "uid ASC"} + cmp = ">" + } + query := applyIncidentsFilter( - s.db.NewSelect().Model(&incidents).Order("started_at DESC"), + s.db.NewSelect().Model(&incidents).Order(order...), filter, ) @@ -4159,13 +4167,17 @@ func (s *Service) ListIncidents( if filter.CursorTimestamp != nil { if filter.CursorUID != nil { - query = query.Where("(started_at < ? OR (started_at = ? AND uid < ?))", + query = query.Where("(started_at "+cmp+" ? OR (started_at = ? AND uid "+cmp+" ?))", *filter.CursorTimestamp, *filter.CursorTimestamp, *filter.CursorUID) } else { - query = query.Where("started_at < ?", *filter.CursorTimestamp) + query = query.Where("started_at "+cmp+" ?", *filter.CursorTimestamp) } } + if filter.SettledBefore != nil { + query = query.Where("started_at < ?", *filter.SettledBefore) + } + if filter.Limit > 0 { query = query.Limit(filter.Limit) } @@ -4328,25 +4340,39 @@ func (s *Service) ListEvents(ctx context.Context, filter *models.ListEventsFilte query := s.db.NewSelect(). Model(&events). - Where("organization_uid = ?", filter.OrganizationUID). - // uid is part of the sort key, not decoration: the keyset cursor's - // predicate is (created_at < ? OR (created_at = ? AND uid < ?)), which - // only works if rows sharing a created_at have a DEFINED order. Without - // this tie-break an audit trail — which bulk-inserts within the same - // microsecond constantly — could skip events at a page boundary. - Order("created_at DESC", "uid DESC") + Where("organization_uid = ?", filter.OrganizationUID) + + // uid is part of the sort key, not decoration: the keyset cursor's + // predicate is (created_at < ? OR (created_at = ? AND uid < ?)), which + // only works if rows sharing a created_at have a DEFINED order. Without + // this tie-break an audit trail — which bulk-inserts within the same + // microsecond constantly — could skip events at a page boundary. + if filter.Ascending { + query = query.Order("created_at ASC", "uid ASC") + } else { + query = query.Order("created_at DESC", "uid DESC") + } query = applyEventFilters(query, filter) if filter.CursorTimestamp != nil { + cmp := "<" + if filter.Ascending { + cmp = ">" + } + if filter.CursorUID != nil { - query = query.Where("(created_at < ? OR (created_at = ? AND uid < ?))", + query = query.Where("(created_at "+cmp+" ? OR (created_at = ? AND uid "+cmp+" ?))", *filter.CursorTimestamp, *filter.CursorTimestamp, *filter.CursorUID) } else { - query = query.Where("created_at < ?", *filter.CursorTimestamp) + query = query.Where("created_at "+cmp+" ?", *filter.CursorTimestamp) } } + if filter.SettledBefore != nil { + query = query.Where("created_at < ?", *filter.SettledBefore) + } + if filter.Limit > 0 { query = query.Limit(filter.Limit) } diff --git a/server/internal/db/service_test.go b/server/internal/db/service_test.go index 9374ae1bb..5f3068163 100644 --- a/server/internal/db/service_test.go +++ b/server/internal/db/service_test.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" "os" + "sort" "strconv" "sync" "testing" @@ -73,6 +74,7 @@ func testService(t *testing.T, svc db.Service) { t.Run("EventsKeysetPaginationTieBreak", func(t *testing.T) { testEventsKeysetPaginationTieBreak(ctx, t, svc) + testAscendingKeysetPagination(ctx, t, svc) }) t.Run("EventsTargetPayloadFilters", func(t *testing.T) { @@ -2904,3 +2906,98 @@ func strPtrTest(value string) *string { return &value } // tokenActivityTimePtr is a local pointer helper for the TokenActivityByUsers // test cases above. func tokenActivityTimePtr(value time.Time) *time.Time { return &value } + +// testAscendingKeysetPagination walks events and incidents oldest first with +// a page size smaller than the group of rows sharing one timestamp, and checks +// the settle bound. Runs on both engines: the flipped comparison and the +// ASC tie-break are SQL, not Go. +func testAscendingKeysetPagination(ctx context.Context, t *testing.T, svc db.Service) { + t.Helper() + + r := require.New(t) + + org := models.NewOrganization("asc-"+uuid.New().String()[:8], "Acme") + r.NoError(svc.CreateOrganization(ctx, org)) + + check := models.NewCheck(org.UID, "asc-check", "http") + r.NoError(svc.CreateCheck(ctx, check)) + + base := time.Date(2026, time.October, 1, 9, 0, 0, 0, time.UTC) + // Ten rows on one instant, then one later row (the settle test excludes it). + for i := 0; i < 11; i++ { + at := base + if i == 10 { + at = base.Add(time.Hour) + } + + event := models.NewEvent(org.UID, models.EventTypeCheckCreated, models.ActorTypeSystem) + event.CreatedAt = at + r.NoError(svc.CreateEvent(ctx, event)) + + inc := models.NewIncident(org.UID, check.UID, at, "asc incident") + r.NoError(svc.CreateIncident(ctx, inc)) + } + + settled := base.Add(30 * time.Minute) + + var ( + seenEvents []string + seenIncidents []string + evTS, incTS *time.Time + evUID, incUID *string + ) + + for page := 0; page < 10; page++ { + rows, err := svc.ListEvents(ctx, &models.ListEventsFilter{ + OrganizationUID: org.UID, Limit: 3, Ascending: true, SettledBefore: &settled, + CursorTimestamp: evTS, CursorUID: evUID, + }) + r.NoError(err) + + if len(rows) == 0 { + break + } + + for _, row := range rows { + seenEvents = append(seenEvents, row.UID) + } + + ts, uid := rows[len(rows)-1].CreatedAt, rows[len(rows)-1].UID + evTS, evUID = &ts, &uid + } + + for page := 0; page < 10; page++ { + rows, _, err := svc.ListIncidents(ctx, &models.ListIncidentsFilter{ + OrganizationUID: org.UID, Limit: 3, Ascending: true, SettledBefore: &settled, + CursorTimestamp: incTS, CursorUID: incUID, + }) + r.NoError(err) + + if len(rows) == 0 { + break + } + + for _, row := range rows { + seenIncidents = append(seenIncidents, row.UID) + } + + ts, uid := rows[len(rows)-1].StartedAt, rows[len(rows)-1].UID + incTS, incUID = &ts, &uid + } + + r.Len(seenEvents, 10, "ten tied events once each, the unsettled one held back") + r.Len(seenIncidents, 10, "ten tied incidents once each, the unsettled one held back") + r.Len(uniqueStrings(seenEvents), 10) + r.Len(uniqueStrings(seenIncidents), 10) + r.True(sort.StringsAreSorted(seenEvents), "ties come back in uid order") + r.True(sort.StringsAreSorted(seenIncidents), "ties come back in uid order") +} + +func uniqueStrings(in []string) map[string]struct{} { + out := make(map[string]struct{}, len(in)) + for _, v := range in { + out[v] = struct{}{} + } + + return out +} diff --git a/server/internal/db/sqlite/sqlite.go b/server/internal/db/sqlite/sqlite.go index ce3bdc13f..353c14837 100644 --- a/server/internal/db/sqlite/sqlite.go +++ b/server/internal/db/sqlite/sqlite.go @@ -4110,8 +4110,16 @@ func (s *Service) ListIncidents( ) ([]*models.Incident, int64, error) { var incidents []*models.Incident + order := []string{"started_at DESC", "uid DESC"} + cmp := "<" + + if filter.Ascending { + order = []string{"started_at ASC", "uid ASC"} + cmp = ">" + } + query := applyIncidentsFilter( - s.db.NewSelect().Model(&incidents).Order("started_at DESC"), + s.db.NewSelect().Model(&incidents).Order(order...), filter, ) @@ -4119,13 +4127,17 @@ func (s *Service) ListIncidents( if filter.CursorTimestamp != nil { if filter.CursorUID != nil { - query = query.Where("(started_at < ? OR (started_at = ? AND uid < ?))", + query = query.Where("(started_at "+cmp+" ? OR (started_at = ? AND uid "+cmp+" ?))", *filter.CursorTimestamp, *filter.CursorTimestamp, *filter.CursorUID) } else { - query = query.Where("started_at < ?", *filter.CursorTimestamp) + query = query.Where("started_at "+cmp+" ?", *filter.CursorTimestamp) } } + if filter.SettledBefore != nil { + query = query.Where("started_at < ?", *filter.SettledBefore) + } + if filter.Limit > 0 { query = query.Limit(filter.Limit) } @@ -4291,25 +4303,39 @@ func (s *Service) ListEvents(ctx context.Context, filter *models.ListEventsFilte query := s.db.NewSelect(). Model(&events). - Where("organization_uid = ?", filter.OrganizationUID). - // uid is part of the sort key, not decoration: the keyset cursor's - // predicate is (created_at < ? OR (created_at = ? AND uid < ?)), which - // only works if rows sharing a created_at have a DEFINED order. Without - // this tie-break an audit trail — which bulk-inserts within the same - // microsecond constantly — could skip events at a page boundary. - Order("created_at DESC", "uid DESC") + Where("organization_uid = ?", filter.OrganizationUID) + + // uid is part of the sort key, not decoration: the keyset cursor's + // predicate is (created_at < ? OR (created_at = ? AND uid < ?)), which + // only works if rows sharing a created_at have a DEFINED order. Without + // this tie-break an audit trail — which bulk-inserts within the same + // microsecond constantly — could skip events at a page boundary. + if filter.Ascending { + query = query.Order("created_at ASC", "uid ASC") + } else { + query = query.Order("created_at DESC", "uid DESC") + } query = applyEventFilters(query, filter) if filter.CursorTimestamp != nil { + cmp := "<" + if filter.Ascending { + cmp = ">" + } + if filter.CursorUID != nil { - query = query.Where("(created_at < ? OR (created_at = ? AND uid < ?))", + query = query.Where("(created_at "+cmp+" ? OR (created_at = ? AND uid "+cmp+" ?))", *filter.CursorTimestamp, *filter.CursorTimestamp, *filter.CursorUID) } else { - query = query.Where("created_at < ?", *filter.CursorTimestamp) + query = query.Where("created_at "+cmp+" ?", *filter.CursorTimestamp) } } + if filter.SettledBefore != nil { + query = query.Where("created_at < ?", *filter.SettledBefore) + } + if filter.Limit > 0 { query = query.Limit(filter.Limit) } diff --git a/server/internal/handlers/base/cursor.go b/server/internal/handlers/base/cursor.go new file mode 100644 index 000000000..056b37fe0 --- /dev/null +++ b/server/internal/handlers/base/cursor.go @@ -0,0 +1,75 @@ +package base + +import ( + "encoding/base64" + "errors" + "fmt" + "net/url" + "strings" + "time" +) + +// SettleWindow is how long a row must have existed before a forward ("asc") +// listing returns it. Two transactions can commit out of created_at order, so +// a row stamped earlier can become visible after a later one that a consumer +// already read past. Holding rows back this long means a consumer sees a row +// at most this late, never skips one. It is a constant, not a setting. +const SettleWindow = 5 * time.Second + +// ErrInvalidOrder is returned when the order query parameter is neither asc nor desc. +var ErrInvalidOrder = errors.New("invalid order parameter: must be asc or desc") + +// ParseOrder reads the order query parameter. It reports true for "asc" +// (oldest first) and false for "desc" (the default, newest first). +func ParseOrder(query url.Values) (bool, error) { + switch raw := query.Get("order"); raw { + case "", "desc": + return false, nil + case "asc": + return true, nil + default: + return false, fmt.Errorf("%w (got %q)", ErrInvalidOrder, raw) + } +} + +// cursorSeparator splits the two halves of an opaque cursor. +const cursorSeparator = "|" + +// EncodeCursor builds the opaque keyset cursor for the last row on a page. +// +// It carries BOTH halves of the sort key (timestamp, uid) because the ordering +// is not unique on the timestamp alone: several rows can share one to the +// microsecond, and a timestamp-only cursor would either repeat or skip them +// at a page boundary. +func EncodeCursor(ts time.Time, uid string) string { + raw := ts.UTC().Format(time.RFC3339Nano) + cursorSeparator + uid + + return base64.RawURLEncoding.EncodeToString([]byte(raw)) +} + +// DecodeCursor parses a cursor produced by EncodeCursor. Anything it cannot +// parse, including the legacy bare-UID cursors that used to be handed out, +// reports !ok and the caller starts from the first page. A malformed cursor is +// a client bug, not a reason to 500. +func DecodeCursor(cursor string) (time.Time, string, bool) { + if cursor == "" { + return time.Time{}, "", false + } + + decoded, err := base64.RawURLEncoding.DecodeString(cursor) + if err != nil { + return time.Time{}, "", false + } + + parts := strings.SplitN(string(decoded), cursorSeparator, 2) + if len(parts) != 2 || parts[1] == "" { + return time.Time{}, "", false + } + + ts, err := time.Parse(time.RFC3339Nano, parts[0]) + if err != nil { + return time.Time{}, "", false + } + + return ts, parts[1], true +} diff --git a/server/internal/handlers/base/cursor_test.go b/server/internal/handlers/base/cursor_test.go new file mode 100644 index 000000000..e6f083c53 --- /dev/null +++ b/server/internal/handlers/base/cursor_test.go @@ -0,0 +1,40 @@ +package base_test + +import ( + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/handlers/base" +) + +func TestParseOrder(t *testing.T) { + t.Parallel() + + for query, want := range map[string]bool{"": false, "order=desc": false, "order=asc": true} { + values, err := url.ParseQuery(query) + require.NoError(t, err) + + got, err := base.ParseOrder(values) + require.NoError(t, err) + require.Equal(t, want, got, query) + } + + _, err := base.ParseOrder(url.Values{"order": {"sideways"}}) + require.ErrorIs(t, err, base.ErrInvalidOrder) +} + +func TestCursorRoundTripAndLegacyUID(t *testing.T) { + t.Parallel() + + at := time.Date(2026, time.October, 1, 9, 0, 0, 123456789, time.UTC) + ts, uid, ok := base.DecodeCursor(base.EncodeCursor(at, "abc")) + require.True(t, ok) + require.True(t, at.Equal(ts)) + require.Equal(t, "abc", uid) + + _, _, ok = base.DecodeCursor("3f2b9c1e-0000-4000-8000-000000000000") + require.False(t, ok, "a bare UID is not a cursor") +} diff --git a/server/internal/handlers/events/cursor_test.go b/server/internal/handlers/events/cursor_test.go new file mode 100644 index 000000000..ad03b76ce --- /dev/null +++ b/server/internal/handlers/events/cursor_test.go @@ -0,0 +1,121 @@ +package events_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/events" + "github.com/fclairamb/solidping/server/internal/utils/clock" +) + +func TestListEventsAscendingWalksEveryEventOnce(t *testing.T) { + t.Parallel() + + ctx := t.Context() + r := require.New(t) + f := newFixture(t) + + base := time.Date(2026, time.October, 1, 9, 0, 0, 0, time.UTC) + f.svc.WithClock(clock.NewFake(base.Add(time.Hour))) + + // Seven events on one instant (page size 3 is smaller than the tie group), + // plus two earlier ones. + want := map[string]bool{} + + for i := 0; i < 9; i++ { + at := base + if i < 2 { + at = base.Add(-time.Minute) + } + + want[f.seed(ctx, t, models.EventTypeCheckCreated, nil, at, "").UID] = true + } + + var ( + order []string + cursor string + prev time.Time + ) + + for page := 0; page < 10; page++ { + resp, err := f.svc.ListEvents(ctx, f.orgSlug, &events.ListEventsOptions{ + Ascending: true, Size: 3, Cursor: cursor, Caller: events.Caller{UserUID: f.admin.UID}, + }) + r.NoError(err) + r.NotEmpty(resp.Pagination.Cursor, "asc always carries a cursor, even on the last page") + + if len(resp.Data) == 0 { + r.Equal(cursor, resp.Pagination.Cursor, "an empty page returns the cursor it was given") + + break + } + + for _, item := range resp.Data { + r.False(item.CreatedAt.Before(prev), "oldest first") + prev = item.CreatedAt + order = append(order, item.UID) + } + + cursor = resp.Pagination.Cursor + } + + r.Len(order, 9) + + for _, uid := range order { + r.True(want[uid]) + delete(want, uid) + } + + r.Empty(want, "every event exactly once") + + // Resume from the stored cursor after new events: only the new ones. + fresh := f.seed(ctx, t, models.EventTypeCheckCreated, nil, base.Add(time.Minute), "") + resp, err := f.svc.ListEvents(ctx, f.orgSlug, &events.ListEventsOptions{ + Ascending: true, Size: 10, Cursor: cursor, Caller: events.Caller{UserUID: f.admin.UID}, + }) + r.NoError(err) + r.Len(resp.Data, 1) + r.Equal(fresh.UID, resp.Data[0].UID) +} + +func TestListEventsAscendingSettleWindow(t *testing.T) { + t.Parallel() + + ctx := t.Context() + r := require.New(t) + f := newFixture(t) + + now := time.Date(2026, time.October, 1, 12, 0, 0, 0, time.UTC) + clk := clock.NewFake(now) + f.svc.WithClock(clk) + + old := f.seed(ctx, t, models.EventTypeCheckCreated, nil, now.Add(-time.Minute), "") + recent := f.seed(ctx, t, models.EventTypeCheckCreated, nil, now.Add(-2*time.Second), "") + + list := func(cursor string, order bool) *events.ListEventsResponse { + resp, err := f.svc.ListEvents(ctx, f.orgSlug, &events.ListEventsOptions{ + Ascending: order, Size: 10, Cursor: cursor, Caller: events.Caller{UserUID: f.admin.UID}, + }) + r.NoError(err) + + return resp + } + + first := list("", true) + r.Len(first.Data, 1, "an event inside the settle window is held back") + r.Equal(old.UID, first.Data[0].UID) + + clk.Advance(10 * time.Second) + + second := list(first.Pagination.Cursor, true) + r.Len(second.Data, 1, "returned by the next call once the window has passed") + r.Equal(recent.UID, second.Data[0].UID) + + // desc is unchanged: no settle window, newest first. + desc := list("", false) + r.Len(desc.Data, 2) + r.Equal(recent.UID, desc.Data[0].UID) +} diff --git a/server/internal/handlers/events/handler.go b/server/internal/handlers/events/handler.go index 75c7ce74d..110df6c6d 100644 --- a/server/internal/handlers/events/handler.go +++ b/server/internal/handlers/events/handler.go @@ -119,6 +119,12 @@ func (h *Handler) ListEvents(writer http.ResponseWriter, req *http.Request) erro } opts.Size = limit + opts.Ascending, err = base.ParseOrder(query) + if err != nil { + return h.WriteErrorErr( + writer, req, http.StatusBadRequest, base.ErrorCodeValidationError, "Invalid order parameter", err) + } + response, err := h.svc.ListEvents(req.Context(), orgSlug, &opts) if err != nil { return h.handleError(writer, req, err) @@ -154,6 +160,12 @@ func (h *Handler) ListIncidentEvents(writer http.ResponseWriter, req *http.Reque } opts.Size = limit + opts.Ascending, err = base.ParseOrder(query) + if err != nil { + return h.WriteErrorErr( + writer, req, http.StatusBadRequest, base.ErrorCodeValidationError, "Invalid order parameter", err) + } + response, err := h.svc.ListEvents(req.Context(), orgSlug, &opts) if err != nil { return h.handleError(writer, req, err) @@ -189,6 +201,12 @@ func (h *Handler) ListCheckEvents(writer http.ResponseWriter, req *http.Request) } opts.Size = limit + opts.Ascending, err = base.ParseOrder(query) + if err != nil { + return h.WriteErrorErr( + writer, req, http.StatusBadRequest, base.ErrorCodeValidationError, "Invalid order parameter", err) + } + response, err := h.svc.ListEvents(req.Context(), orgSlug, &opts) if err != nil { return h.handleError(writer, req, err) diff --git a/server/internal/handlers/events/handler_cover_test.go b/server/internal/handlers/events/handler_cover_test.go index 6d2df9105..76c3f2d90 100644 --- a/server/internal/handlers/events/handler_cover_test.go +++ b/server/internal/handlers/events/handler_cover_test.go @@ -45,12 +45,15 @@ func TestEventsHandler(t *testing.T) { //nolint:tparallel // subtests share one {"list bad since", h.ListEvents, f.orgSlug, "since=yesterday", f.admin, http.StatusBadRequest}, {"list bad until", h.ListEvents, f.orgSlug, "until=tomorrow", f.admin, http.StatusBadRequest}, {"list bad limit", h.ListEvents, f.orgSlug, "limit=abc", f.admin, http.StatusBadRequest}, + {"list bad order", h.ListEvents, f.orgSlug, "order=bogus", f.admin, http.StatusBadRequest}, {"list ghost org", h.ListEvents, "ghost", "", f.admin, http.StatusNotFound}, {"incident events ok", h.ListIncidentEvents, f.orgSlug, "cursor=x&limit=3", f.admin, http.StatusOK}, {"incident events bad limit", h.ListIncidentEvents, f.orgSlug, "limit=-1", f.admin, http.StatusBadRequest}, + {"incident events bad order", h.ListIncidentEvents, f.orgSlug, "order=bogus", f.admin, http.StatusBadRequest}, {"incident events ghost", h.ListIncidentEvents, "ghost", "", f.admin, http.StatusNotFound}, {"check events ok", h.ListCheckEvents, f.orgSlug, "cursor=x&limit=3", f.admin, http.StatusOK}, {"check events bad limit", h.ListCheckEvents, f.orgSlug, "limit=zzz", f.admin, http.StatusBadRequest}, + {"check events bad order", h.ListCheckEvents, f.orgSlug, "order=bogus", f.admin, http.StatusBadRequest}, {"check events ghost", h.ListCheckEvents, "ghost", "", f.admin, http.StatusNotFound}, } diff --git a/server/internal/handlers/events/service.go b/server/internal/handlers/events/service.go index 814d0d3ec..954c5bbfa 100644 --- a/server/internal/handlers/events/service.go +++ b/server/internal/handlers/events/service.go @@ -3,14 +3,14 @@ package events import ( "context" - "encoding/base64" "errors" "fmt" - "strings" "time" "github.com/fclairamb/solidping/server/internal/db" "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/base" + "github.com/fclairamb/solidping/server/internal/utils/clock" ) // Service errors. @@ -35,12 +35,20 @@ var RestrictedEventTypePrefixes = []string{"auth"} // Service provides event listing functionality. type Service struct { - db db.Service + db db.Service + clock clock.Clock } // NewService creates a new event service. func NewService(dbService db.Service) *Service { - return &Service{db: dbService} + return &Service{db: dbService, clock: clock.Real{}} +} + +// WithClock replaces the clock used for the settle window of forward listings. +func (s *Service) WithClock(clk clock.Clock) *Service { + s.clock = clk + + return s } // Caller identifies the authenticated principal reading the trail. The role is @@ -76,8 +84,10 @@ type ListEventsOptions struct { Since *time.Time Until *time.Time Cursor string - Size int - Caller Caller + // Ascending lists oldest first; the cursor then means "strictly after". + Ascending bool + Size int + Caller Caller } // EventResponse represents an event in API responses. @@ -179,11 +189,17 @@ func (s *Service) ListEvents( filter.EventTypes = append(filter.EventTypes, models.EventType(typeStr)) } - if ts, uid, ok := DecodeCursor(opts.Cursor); ok { + if ts, uid, ok := base.DecodeCursor(opts.Cursor); ok { filter.CursorTimestamp = &ts filter.CursorUID = &uid } + if opts.Ascending { + settled := s.clock.Now().Add(-base.SettleWindow) + filter.Ascending = true + filter.SettledBefore = &settled + } + events, err := s.db.ListEvents(ctx, filter) if err != nil { return nil, fmt.Errorf("failed to list events: %w", err) @@ -208,10 +224,15 @@ func (s *Service) ListEvents( response.Data = append(response.Data, toResponse(evt, actors, isAdmin)) } - // Set cursor if there are more results - if hasMore && len(events) > 0 { + switch { + case len(events) > 0 && (hasMore || opts.Ascending): + // Forward mode always hands out a cursor, even on the last page, so + // the consumer can resume from it. lastEvent := events[len(events)-1] - response.Pagination.Cursor = EncodeCursor(lastEvent.CreatedAt, lastEvent.UID) + response.Pagination.Cursor = base.EncodeCursor(lastEvent.CreatedAt, lastEvent.UID) + case opts.Ascending: + // An empty page returns the cursor it was given: no lost position. + response.Pagination.Cursor = opts.Cursor } return response, nil @@ -293,46 +314,3 @@ func (s *Service) resolveActors(ctx context.Context, events []*models.Event) map return actors } - -// cursorSeparator splits the two halves of an opaque cursor. -const cursorSeparator = "|" - -// EncodeCursor builds the opaque keyset cursor for the last row on a page. -// -// It carries BOTH halves of the sort key (created_at, uid) because the ordering -// is not unique on created_at alone — several events can share a timestamp to -// the microsecond, and a timestamp-only cursor would either repeat or skip them -// at a page boundary. Before this the endpoint returned a bare UID and the -// service ignored it entirely, so "next page" silently returned page 1 forever. -func EncodeCursor(createdAt time.Time, uid string) string { - raw := createdAt.UTC().Format(time.RFC3339Nano) + cursorSeparator + uid - - return base64.RawURLEncoding.EncodeToString([]byte(raw)) -} - -// DecodeCursor parses a cursor produced by EncodeCursor. Anything it cannot -// parse — including the legacy bare-UID cursors that used to be handed out — -// reports !ok, and the caller starts from the first page. A malformed cursor is -// a client bug, not a reason to 500. -func DecodeCursor(cursor string) (time.Time, string, bool) { - if cursor == "" { - return time.Time{}, "", false - } - - decoded, err := base64.RawURLEncoding.DecodeString(cursor) - if err != nil { - return time.Time{}, "", false - } - - parts := strings.SplitN(string(decoded), cursorSeparator, 2) - if len(parts) != 2 || parts[1] == "" { - return time.Time{}, "", false - } - - ts, err := time.Parse(time.RFC3339Nano, parts[0]) - if err != nil { - return time.Time{}, "", false - } - - return ts, parts[1], true -} diff --git a/server/internal/handlers/events/service_test.go b/server/internal/handlers/events/service_test.go index 3d965b760..8ccce0c3c 100644 --- a/server/internal/handlers/events/service_test.go +++ b/server/internal/handlers/events/service_test.go @@ -10,6 +10,7 @@ import ( "github.com/fclairamb/solidping/server/internal/db" "github.com/fclairamb/solidping/server/internal/db/models" "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/handlers/base" "github.com/fclairamb/solidping/server/internal/handlers/events" ) @@ -370,15 +371,15 @@ func TestCursorRoundTripAndGarbageTolerance(t *testing.T) { r := require.New(t) at := time.Date(2026, time.August, 21, 9, 30, 15, 123456789, time.UTC) - cursor := events.EncodeCursor(at, "abc-123") + cursor := base.EncodeCursor(at, "abc-123") - ts, uid, ok := events.DecodeCursor(cursor) + ts, uid, ok := base.DecodeCursor(cursor) r.True(ok) r.Equal("abc-123", uid) r.True(at.Equal(ts)) for _, garbage := range []string{"", "not-base64!!", "YWJj", "1e5b9d20-0000-0000-0000-000000000000"} { - _, _, ok := events.DecodeCursor(garbage) + _, _, ok := base.DecodeCursor(garbage) r.Falsef(ok, "%q must decode as 'no cursor', not as a usable position", garbage) } } diff --git a/server/internal/handlers/incidents/cursor_test.go b/server/internal/handlers/incidents/cursor_test.go new file mode 100644 index 000000000..fa9b43a22 --- /dev/null +++ b/server/internal/handlers/incidents/cursor_test.go @@ -0,0 +1,174 @@ +package incidents_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/handlers/incidents" + "github.com/fclairamb/solidping/server/internal/jobs/jobsvc" + "github.com/fclairamb/solidping/server/internal/notifier" + "github.com/fclairamb/solidping/server/internal/utils/clock" +) + +type cursorFixture struct { + svc *incidents.Service + clk *clock.Fake + org *models.Organization + base time.Time + list func(cursor string, asc bool, size int) *incidents.ListIncidentsResponse + add func(at time.Time) string +} + +func newCursorFixture(t *testing.T) *cursorFixture { + t.Helper() + + ctx := t.Context() + r := require.New(t) + + dbSvc, err := sqlite.New(ctx, sqlite.Config{InMemory: true}) + r.NoError(err) + t.Cleanup(func() { _ = dbSvc.Close() }) + r.NoError(dbSvc.Initialize(ctx)) + + base := time.Date(2026, time.October, 1, 9, 0, 0, 0, time.UTC) + clk := clock.NewFake(base.Add(time.Hour)) + jobs := jobsvc.NewService(dbSvc.DB(), dbSvc, notifier.NewLocalEventNotifier(), nil) + svc := incidents.NewService(dbSvc, jobs, clk, nil) + + org := models.NewOrganization("cursor-org", "") + r.NoError(dbSvc.CreateOrganization(ctx, org)) + + check := models.NewCheck(org.UID, "acme-api", "http") + r.NoError(dbSvc.CreateCheck(ctx, check)) + + return &cursorFixture{ + svc: svc, clk: clk, org: org, base: base, + list: func(cursor string, asc bool, size int) *incidents.ListIncidentsResponse { + resp, listErr := svc.ListIncidents(ctx, org.Slug, &incidents.ListIncidentsOptions{ + Cursor: cursor, Ascending: asc, Size: size, + }) + r.NoError(listErr) + + return resp + }, + add: func(at time.Time) string { + inc := models.NewIncident(org.UID, check.UID, at, "outage") + r.NoError(dbSvc.CreateIncident(ctx, inc)) + + return inc.UID + }, + } +} + +func walkIncidents(f *cursorFixture, asc bool) []string { + var seen []string + + cursor := "" + + for page := 0; page < 20; page++ { + resp := f.list(cursor, asc, 3) + for _, item := range resp.Data { + seen = append(seen, item.UID) + } + + if resp.Pagination.Cursor == "" || len(resp.Data) == 0 { + break + } + + cursor = resp.Pagination.Cursor + if len(resp.Data) < 3 && !asc { + break + } + } + + return seen +} + +func TestListIncidentsCursorVisitsEveryIncidentOnce(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := newCursorFixture(t) + + want := map[string]bool{} + + // Eight incidents on one started_at (page size 3 < tie group) plus two others. + for i := 0; i < 10; i++ { + at := f.base + if i >= 8 { + at = f.base.Add(time.Duration(i) * time.Minute) + } + + want[f.add(at)] = true + } + + for _, asc := range []bool{false, true} { + seen := walkIncidents(f, asc) + r.Len(seen, 10) + + got := map[string]bool{} + for _, uid := range seen { + got[uid] = true + } + + r.Equal(want, got, "every incident exactly once (asc=%v)", asc) + } + + asc := walkIncidents(f, true) + desc := walkIncidents(f, false) + r.Equal(asc[9], desc[0], "asc is oldest first, desc newest first") +} + +func TestListIncidentsLegacyBareUIDCursorStartsFromFirstPage(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := newCursorFixture(t) + + newest := f.add(f.base.Add(time.Minute)) + f.add(f.base) + + resp := f.list(newest, false, 10) + r.Len(resp.Data, 2) + r.Equal(newest, resp.Data[0].UID) +} + +func TestListIncidentsAscendingResumeAndSettleWindow(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := newCursorFixture(t) + now := f.clk.Now() + + first := f.add(now.Add(-time.Minute)) + recent := f.add(now.Add(-2 * time.Second)) + + page := f.list("", true, 10) + r.Len(page.Data, 1, "an incident inside the settle window is held back") + r.Equal(first, page.Data[0].UID) + r.NotEmpty(page.Pagination.Cursor) + + f.clk.Advance(10 * time.Second) + + page2 := f.list(page.Pagination.Cursor, true, 10) + r.Len(page2.Data, 1) + r.Equal(recent, page2.Data[0].UID, "only incidents after the cursor") + + empty := f.list(page2.Pagination.Cursor, true, 10) + r.Empty(empty.Data) + r.Equal(page2.Pagination.Cursor, empty.Pagination.Cursor, "an empty page keeps the cursor") + + older := f.add(now.Add(-30 * time.Second)) + _ = older // started before the cursor: not listed, only newer starts are + + later := f.add(now.Add(5 * time.Second)) + f.clk.Advance(time.Minute) + + page3 := f.list(empty.Pagination.Cursor, true, 10) + r.Len(page3.Data, 1) + r.Equal(later, page3.Data[0].UID) +} diff --git a/server/internal/handlers/incidents/handler.go b/server/internal/handlers/incidents/handler.go index 1dbb10c3e..3664da73e 100644 --- a/server/internal/handlers/incidents/handler.go +++ b/server/internal/handlers/incidents/handler.go @@ -133,6 +133,11 @@ func parseListIncidentsOptions(query url.Values) (*ListIncidentsOptions, error) } opts.Size = limit + opts.Ascending, err = base.ParseOrder(query) + if err != nil { + return nil, err + } + applyListIncidentsExtras(query, opts) return opts, nil diff --git a/server/internal/handlers/incidents/handler_cover_test.go b/server/internal/handlers/incidents/handler_cover_test.go index cb6b103d0..06e89611a 100644 --- a/server/internal/handlers/incidents/handler_cover_test.go +++ b/server/internal/handlers/incidents/handler_cover_test.go @@ -57,6 +57,7 @@ func TestIncidentHandlerEndpoints(t *testing.T) { {"list bad kind", http.MethodGet, "/orgs/" + org + "/incidents?kind=bogus", "", http.StatusBadRequest}, {"list bad since", http.MethodGet, "/orgs/" + org + "/incidents?since=x", "", http.StatusBadRequest}, {"list bad until", http.MethodGet, "/orgs/" + org + "/incidents?until=x", "", http.StatusBadRequest}, + {"list bad order", http.MethodGet, "/orgs/" + org + "/incidents?order=bogus", "", http.StatusBadRequest}, {"list bad limit", http.MethodGet, "/orgs/" + org + "/incidents?limit=x", "", http.StatusBadRequest}, {"get", http.MethodGet, inc + "?with=check,members", "", http.StatusOK}, {"get missing", http.MethodGet, "/orgs/" + org + "/incidents/nope", "", http.StatusNotFound}, diff --git a/server/internal/handlers/incidents/service.go b/server/internal/handlers/incidents/service.go index 7d5ef4c91..bd8f95edc 100644 --- a/server/internal/handlers/incidents/service.go +++ b/server/internal/handlers/incidents/service.go @@ -16,6 +16,7 @@ import ( "github.com/fclairamb/solidping/server/internal/db" "github.com/fclairamb/solidping/server/internal/db/models" "github.com/fclairamb/solidping/server/internal/handlers/attachments" + "github.com/fclairamb/solidping/server/internal/handlers/base" "github.com/fclairamb/solidping/server/internal/jobs/jobdef" "github.com/fclairamb/solidping/server/internal/jobs/jobsvc" "github.com/fclairamb/solidping/server/internal/jobs/jobtypes" @@ -2293,6 +2294,8 @@ type ListIncidentsOptions struct { Since *time.Time Until *time.Time Cursor string + // Ascending lists oldest first; the cursor then means "strictly after". + Ascending bool Size int WithCheck bool // Include check details in response WithMembers bool // Include group-incident members and checkGroupSlug in response @@ -2674,6 +2677,38 @@ func buildListIncidentsFilter(orgUID string, opts *ListIncidentsOptions) *models return filter } +// applyListCursor turns the opaque cursor and the order into keyset filter +// fields. In ascending mode it also holds back incidents inside the settle +// window, so a consumer never reads past a row that is not visible yet. +func (s *Service) applyListCursor(filter *models.ListIncidentsFilter, opts *ListIncidentsOptions) { + if ts, uid, ok := base.DecodeCursor(opts.Cursor); ok { + filter.CursorTimestamp = &ts + filter.CursorUID = &uid + } + + if opts.Ascending { + settled := s.clock.Now().Add(-base.SettleWindow) + filter.Ascending = true + filter.SettledBefore = &settled + } +} + +// nextListCursor is the cursor for the next call. Forward mode always hands one +// out, even on the last page, and an empty page returns the cursor it was +// given so the consumer never loses its position. +func nextListCursor(incidents []*models.Incident, hasMore bool, opts *ListIncidentsOptions) string { + switch { + case len(incidents) > 0 && (hasMore || opts.Ascending): + last := incidents[len(incidents)-1] + + return base.EncodeCursor(last.StartedAt, last.UID) + case opts.Ascending: + return opts.Cursor + default: + return "" + } +} + // ListIncidents lists incidents for an organization. func (s *Service) ListIncidents( ctx context.Context, orgSlug string, opts *ListIncidentsOptions, @@ -2706,7 +2741,7 @@ func (s *Service) ListIncidents( filter := buildListIncidentsFilter(org.UID, opts) - // TODO: Parse cursor + s.applyListCursor(filter, opts) incidents, total, err := s.db.ListIncidents(ctx, filter) if err != nil { @@ -2755,11 +2790,7 @@ func (s *Service) ListIncidents( response.Data = append(response.Data, incResponse) } - // Set cursor if there are more results - if hasMore && len(incidents) > 0 { - lastIncident := incidents[len(incidents)-1] - response.Pagination.Cursor = lastIncident.UID - } + response.Pagination.Cursor = nextListCursor(incidents, hasMore, opts) return response, nil } diff --git a/server/internal/mcp/tools_incidents.go b/server/internal/mcp/tools_incidents.go index b9336e8a0..d2c63d449 100644 --- a/server/internal/mcp/tools_incidents.go +++ b/server/internal/mcp/tools_incidents.go @@ -18,8 +18,11 @@ func listIncidentsDef() ToolDefinition { Name: toolListIncidents, Description: "List internal incidents (active or resolved) for the organization, filtered by " + "check, state, or time range. Returns {data, pagination} — total, size, and a cursor " + - "naming the last row of a full page; the cursor is not read back on the next call yet, " + - "so move forward by narrowing since/until. Use diagnose_check for one check's " + + "naming the last row (opaque, pass it back as cursor for the next page). order=asc lists " + + "oldest first and always returns a cursor, even on the last page, so you can store it " + + "and later resume with only the incidents started after it (rows younger than 5 s are " + + "held back). It lists new incidents only; changes to existing " + + "incidents are events (GET /events?type=incident&order=asc). Use diagnose_check for one check's " + "incidents plus current health in one call, and get_status_page include=incidents for the " + "customer-facing entries on a status page. Read-only: works with mcp:read tokens.", InputSchema: objectSchema(map[string]any{ @@ -43,6 +46,10 @@ func listIncidentsDef() ToolDefinition { ), propSize: intProp(descLimit), propCursor: stringProp(descCursor), + "order": stringProp( + "Sort order on start time: \"desc\" (default, newest first) or \"asc\" (oldest first; " + + "the cursor then means strictly after, and is always returned).", + ), }, nil), OutputSchema: objectSchema(map[string]any{ schemaKeyData: arrayOfObjectsProp( @@ -54,7 +61,7 @@ func listIncidentsDef() ToolDefinition { schemaKeyDescription: "Page state for the filter.", schemaKeyProperties: map[string]any{ "total": intProp("Total incidents matching the filter."), - propCursor: stringProp("UID of the last incident on a full page; not applied when passed back."), + propCursor: stringProp("Opaque cursor naming the last incident; pass it back as cursor."), "size": intProp("Page size used for this response."), }, }, @@ -63,6 +70,19 @@ func listIncidentsDef() ToolDefinition { } } +// parseOrderArg maps the order argument to "ascending"; ok is false for any +// value other than empty, desc or asc. +func parseOrderArg(order string) (bool, bool) { + switch order { + case "", "desc": + return false, true + case "asc": + return true, true + default: + return false, false + } +} + func (h *Handler) toolListIncidents(ctx context.Context, orgSlug string, args map[string]any) ToolCallResult { opts := &incidents.ListIncidentsOptions{ Cursor: getStringArg(args, propCursor), @@ -76,6 +96,12 @@ func (h *Handler) toolListIncidents(ctx context.Context, orgSlug string, args ma opts.Size = 100 } + ascending, ok := parseOrderArg(getStringArg(args, "order")) + if !ok { + return errorResult("invalid order: must be asc or desc") + } + opts.Ascending = ascending + if v := getStringArg(args, propCheckUID); v != "" { opts.CheckUIDs = strings.Split(v, ",") } diff --git a/server/internal/mcp/tools_incidents_test.go b/server/internal/mcp/tools_incidents_test.go index f49682a67..f7395abac 100644 --- a/server/internal/mcp/tools_incidents_test.go +++ b/server/internal/mcp/tools_incidents_test.go @@ -357,3 +357,62 @@ func incidentRowJSONType(value any) string { return schemaTypeObject } } + +// TestToolListIncidentsOrderAndCursor walks list_incidents oldest first with +// the returned cursor, and checks that a bad order is an error result. +func TestToolListIncidentsOrderAndCursor(t *testing.T) { + t.Parallel() + r := require.New(t) + ctx := t.Context() + + handler, org := newIncidentToolsEnv(t) + handler.incidentsSvc = newIncidentsServiceForTest(handler) + + check := models.NewCheck(org.UID, "acme-api", "http") + r.NoError(handler.dbService.CreateCheck(ctx, check)) + + base := time.Date(2026, time.October, 1, 9, 0, 0, 0, time.UTC) + want := make([]string, 0, 3) + for i := range 3 { + inc := models.NewIncident(org.UID, check.UID, base.Add(time.Duration(i)*time.Hour), "outage") + r.NoError(handler.dbService.CreateIncident(ctx, inc)) + want = append(want, inc.UID) + } + + page := func(args map[string]any) ([]string, string) { + result := handler.toolListIncidents(ctx, org.Slug, args) + r.False(result.IsError) + + raw, err := json.Marshal(result.StructuredContent) + r.NoError(err) + + var decoded struct { + Data []struct { + UID string `json:"uid"` + } `json:"data"` + Pagination struct { + Cursor string `json:"cursor"` + } `json:"pagination"` + } + r.NoError(json.Unmarshal(raw, &decoded)) + + uids := make([]string, 0, len(decoded.Data)) + for _, row := range decoded.Data { + uids = append(uids, row.UID) + } + + return uids, decoded.Pagination.Cursor + } + + first, cursor := page(map[string]any{"order": "asc", "size": 2}) + r.Equal(want[:2], first) + r.NotEmpty(cursor) + + second, cursor := page(map[string]any{"order": "asc", "size": 2, propCursor: cursor}) + r.Equal(want[2:], second) + r.NotEmpty(cursor, "asc always returns a cursor, even on the last page") + + bad := handler.toolListIncidents(ctx, org.Slug, map[string]any{"order": "bogus"}) + r.True(bad.IsError) + r.Contains(bad.Content[0].Text, "invalid order") +} diff --git a/specs/done/2026/10/2026-10-22-06-events-and-incidents-cursors.md b/specs/done/2026/10/2026-10-22-06-events-and-incidents-cursors.md new file mode 100644 index 000000000..7cffb3f87 --- /dev/null +++ b/specs/done/2026/10/2026-10-22-06-events-and-incidents-cursors.md @@ -0,0 +1,114 @@ +# Cursors on the events and incidents APIs + +## Problem + +A program that follows an org's alerts (an AI agent, a script, a bridge) has to answer one +question after every restart: "what happened since the last thing I saw?". The REST API +cannot answer it reliably today. + +- `GET /api/v1/orgs/{org}/incidents` hands out `pagination.cursor` (the last incident's + UID) but never reads it back: `ListIncidents` in + `server/internal/handlers/incidents/service.go` has a `// TODO: Parse cursor` and the + handler stores `opts.Cursor` for nothing. Asking for page 2 returns page 1 again. The + storage layer already supports a keyset (`CursorTimestamp` / `CursorUID` on the filter, + `started_at, uid` in `postgres.go`); only the service is missing. +- `GET /api/v1/orgs/{org}/events` (and the check and incident variants) has a correct + keyset cursor (`EncodeCursor` / `DecodeCursor` in + `server/internal/handlers/events/service.go`), but only backwards: newest first, the + cursor goes to older pages. A consumer cannot say "give me what came after this event". + `since=` gets close but repeats or skips rows that share a timestamp, and + the last page has no cursor to resume from. + +The live-update WebSocket (`/api/v1/orgs/{org}/events/ws`) tells a client that something +changed, and the ntfy integration pushes alerts. Neither replays what a client missed +while it was disconnected. A forward cursor is the missing piece: push wakes the client up, +the cursor catches it up. + +## Proposal + +### 1. Incidents: make the existing cursor work + +- Parse `cursor` in `ListIncidents` and pass it to the filter as `CursorTimestamp` / + `CursorUID` (keyset on `started_at, uid`, newest first, as the query already sorts). +- Emit an opaque cursor built like the events one (base64url of + `RFC3339Nano|uid`). Move `EncodeCursor` / `DecodeCursor` to a shared place (for example + `server/internal/handlers/base`) instead of copying them. +- A cursor that does not decode (including the bare UIDs handed out until now) starts + from the first page, as events already do. + +### 2. Events: read forward + +Add `order` (`desc` default, `asc`) to the three event list endpoints. + +- `order=asc` sorts oldest first on `(created_at, uid)`, and `cursor` then means "strictly + after this event". +- In `asc` mode the response **always** carries `pagination.cursor` for its last row, + even on the last page, so a consumer stores it and resumes from it next time. An empty + page returns the cursor it was given, so the consumer never loses its position. +- **Settle window.** Two transactions can commit out of `created_at` order: an event + stamped 10:00:00.100 can become visible after one stamped 10:00:00.200 that a consumer + already read past. In `asc` mode, only return events with `created_at` older than + `now() - 5s`. A consumer sees an event at most 5 s late, never skips one. The window is a + constant in the service, not a setting. +- `since` / `until` keep working and combine with `cursor`. +- `desc` behaves exactly as today. + +### 3. Incidents: read forward + +Add the same `order=asc` to the incident list (`started_at, uid`, cursor = strictly after, +cursor always present, same settle window). This lists **new** incidents. Changes to +existing incidents (acked, resolved, reopened) are events: a consumer that wants the full +lifecycle follows `GET /events?type=incident&order=asc`. Say so in the API docs. + +### 4. MCP + +`list_incidents` accepts `cursor` and `order` and returns the next cursor, so an agent +using MCP can page and resume the same way. + +### 5. Docs + +- OpenAPI: `order` on the four list operations, `cursor` described as opaque, the + always-present cursor and the settle window in `asc` mode. +- `wiki/api-specification/` pages for events and incidents. +- A short "Follow events from a program" section in the docs site (where the live-updates + page is): WebSocket or ntfy for the wake-up, `order=asc&cursor=` for the catch-up, store + the cursor after each page. + +## Non-goals + +- No new streaming endpoint (SSE, long poll). The WebSocket hint and ntfy already push. +- No per-consumer state on the server (no stored cursors, no acks). The client keeps its + cursor. +- No change to retention: events and incidents live as long as they do today. +- No renaming of the events `size` parameter to `limit`. + +## Acceptance Criteria + +- [ ] Paging through `GET /incidents` with the returned cursor visits every incident exactly + once, including incidents that share a `started_at` (test with several rows on the + same timestamp and a page size smaller than the tie group). +- [ ] An old bare-UID cursor on `GET /incidents` returns the first page, not an error. +- [ ] `GET /events?order=asc` walked page by page returns every event exactly once, oldest + first, with ties on `created_at` (same kind of test). +- [ ] In `asc` mode the last page and an empty page both return a usable cursor; resuming + from it after inserting new events returns only the new ones. +- [ ] An event newer than the settle window is not returned in `asc` mode and is returned + by the next call once the window has passed (test with an injected clock, not a sleep). +- [ ] `GET /incidents?order=asc&cursor=` lists only incidents started after the cursor. +- [ ] `desc` responses (default) are unchanged for events and incidents. +- [ ] MCP `list_incidents` takes `cursor` and `order` and returns the next cursor. +- [ ] Works on SQLite and Postgres (`make test` and `make test-postgres`). +- [ ] OpenAPI, wiki and docs updated as described. + +## Open questions + +- Settle window length. **Recommended: 5 s.** Long enough for any commit the server makes + in one request, short enough that an agent does not notice. +- Should `order=asc` on events default its starting point to "now" when no cursor and no + `since` are given (pure tail) instead of the oldest event? **Recommended: no.** Oldest + first is predictable; a client that wants the tail passes `since=now`. + +## Resolved open questions + +- Settle window length: 5 s. (autopilot: recommended answer) +- `order=asc` on events with no cursor and no `since` starts at the oldest event, not at "now"; a client that wants the tail passes `since=now`. (autopilot: recommended answer) diff --git a/web/docs/docs/features/live-updates.md b/web/docs/docs/features/live-updates.md index dea78fe7a..c2ab9a456 100644 --- a/web/docs/docs/features/live-updates.md +++ b/web/docs/docs/features/live-updates.md @@ -172,6 +172,36 @@ with a fresh token and re-subscribe to whatever scopes you cared about. Hints carry no history, so reconnecting, re-subscribing, and refetching once is always correct. +## Follow events from a program + +A script, bridge or AI agent that follows an org's alerts has to answer "what happened since +the last thing I saw?" after every restart. Use two pieces: + +1. **Wake-up**: the WebSocket above, or the ntfy integration, tells you something changed. +2. **Catch-up**: `order=asc` with a `cursor` returns what you missed, oldest first. + +```bash +# First run: from the oldest event (add since= to start later). +curl -H "Authorization: Bearer $TOKEN" \ + 'https://your-instance/api/v1/orgs/default/events?order=asc&limit=100' + +# Next runs: pass the cursor you stored after the previous page. +curl -H "Authorization: Bearer $TOKEN" \ + "https://your-instance/api/v1/orgs/default/events?order=asc&limit=100&cursor=$CURSOR" +``` + +Rules: + +- With `order=asc` the response **always** carries `pagination.cursor`, even on the last page. + An empty page returns the cursor you sent. Store it after each page. +- The cursor is opaque. Pass it back verbatim. +- Rows younger than 5 seconds are held back, so a row that commits slightly out of order is + never skipped. You see an event at most 5 s late. +- `GET /incidents?order=asc&cursor=` lists **new** incidents. Changes to existing incidents + (acknowledged, resolved, reopened) are events: follow `GET /events?type=incident&order=asc` + for the full lifecycle. +- The MCP `list_incidents` tool takes the same `cursor` and `order`. + ## Operational notes ### PgBouncer and connection poolers diff --git a/wiki/api-specification/results-incidents.md b/wiki/api-specification/results-incidents.md index 263980a50..a11b96881 100644 --- a/wiki/api-specification/results-incidents.md +++ b/wiki/api-specification/results-incidents.md @@ -77,7 +77,15 @@ Query parameters: - `with` - comma-separated: `check`, `members` (`members` also adds `checkGroupSlug`; both are opt-in — omitted by default, and the default response costs zero extra member/group queries) -- `cursor` - pagination cursor +- `cursor` - opaque keyset cursor over `(started_at, uid)`; hand back `pagination.cursor` + verbatim. An unparseable cursor (including the bare UIDs older versions handed out) + starts from the first page. With `order=asc` it means "strictly after this incident". +- `order` - `desc` (default, newest first) or `asc` (oldest first). `asc` always returns + `pagination.cursor` for the last row, even on the last page, and an empty page returns + the cursor it was given: store it and resume from it. Incidents younger than 5 s are held + back (settle window), so a consumer sees one at most 5 s late and never skips one. + `asc` lists **new** incidents; changes to existing ones (acked, resolved, reopened) are + events: follow `GET /events?type=incident&order=asc` for the full lifecycle. - `limit` - page size (default 20, max 100). Also accepts `?size=` as a deprecated alias. `pagination.total` is the count of incidents matching the request's filters @@ -192,6 +200,12 @@ Query parameters: - `cursor` - pagination cursor. Opaque keyset cursor over `(created_at, uid)`; hand back `pagination.cursor` verbatim. An unparseable cursor is ignored (first page) rather than rejected. +- `order` - `desc` (default, newest first) or `asc` (oldest first on `(created_at, uid)`; + the cursor then means "strictly after this event"). `asc` always returns + `pagination.cursor` for the last row, even on the last page, and an empty page returns + the cursor it was given. Events younger than 5 s are held back (settle window): a consumer + sees an event at most 5 s late, never skips one. `since` / `until` combine with `cursor`. + Also accepted on `/checks/:uid/events` and `/incidents/:uid/events`. - `limit` - page size (default 20, max 100). Also accepts `?size=` as a deprecated alias. Response items carry `uid`, `eventType`, `actorType`