fix(ansible): page the inventory sync history by page[number]/page[size] (#761)

backend/internal/api/v2/handlers/ansible/inventory_syncs.go

@@ -4,7 +4,6 @@ package ansible
 
 import (
 	"net/http"
-	"strconv"
 
 	"github.com/gin-gonic/gin"
 	"github.com/google/uuid"
@@ -79,10 +78,16 @@ func (h *InventorySyncHandler) List(c *gin.Context) {
 		return
 	}
 
-	limit, _ := strconv.Atoi(c.DefaultQuery("limit", "20"))
-	offset, _ := strconv.Atoi(c.DefaultQuery("offset", "0"))
+	// page[number]/page[size]. This used to read limit/offset while reporting a correct
+	// six-member block including the true total, which is the most dangerous combination there
+	// is: total-pages can exceed 1, so a client is invited to ask for page 2, and the offset it
+	// sends is never read - it gets page 1 again. That is the inventory-sources bug in #761,
+	// which rendered every row twice. It did not bite here only because the one caller sends
+	// ?limit=50 and never pages. Reporting a true total and honouring page[number] are one
+	// feature; this endpoint had the first half without the second.
+	page, perPage := jsonapi.PageParams(c, 20)
 
-	syncs, total, err := h.syncRepo.ListByInventory(inventoryID, limit, offset)
+	syncs, total, err := h.syncRepo.ListByInventory(inventoryID, perPage, jsonapi.Offset(page, perPage))
 	if err != nil {
 		response.InternalError(c, "Failed to list inventory syncs")
 		return
@@ -92,10 +97,7 @@ func (h *InventorySyncHandler) List(c *gin.Context) {
 	for i := range syncs {
 		data = append(data, formatInventorySyncResponse(&syncs[i], false))
 	}
-	// limit/offset paging, reported as pages so this collection reads like every other one.
-	// It previously emitted a bare {"total": n} with no page information at all, so a client
-	// could not tell which page it had received.
-	jsonapi.WriteDocumentMeta(c, http.StatusOK, data, jsonapi.NewPaginationMeta(offset/max(limit, 1)+1, limit, total))
+	jsonapi.WriteDocumentMeta(c, http.StatusOK, data, jsonapi.NewPaginationMeta(page, perPage, total))
 }
 
 // Get returns one sync run including its captured output.

backend/internal/api/v2/routes/inventory_syncs_pagination_test.go

@@ -0,0 +1,130 @@
+//go:build integration
+// +build integration
+
+// Copyright (c) 2026 VH & Co BV. Licensed under the Business Source License 1.1. See LICENSE for details.
+
+package routes_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/michielvha/stackweaver/core/models"
+)
+
+// The inventory sync history must page by page[number]/page[size].
+//
+// This was the last endpoint carrying the combination #761 showed to be the most dangerous one:
+// a correct six-member meta block, including a true total, over paging that read limit/offset.
+// A true total means total-pages can exceed 1, so a client is invited to request page 2 - and
+// the offset it sends was never read, so it got page 1 back. That is the inventory-sources bug
+// that rendered every row twice.
+//
+// It never bit in production because the single caller sent ?limit=50 and never paged. That is
+// not a safety property, it is a coincidence about today's only consumer, which is why this is
+// pinned rather than left alone.
+func TestInventorySyncsListHonoursJSONAPIPagination(t *testing.T) {
+	h := setupGoldenHarness(t)
+
+	inventoryID, ok := firstCol(h.db, "ansible_inventories", "id")
+	if !ok {
+		t.Skip("no seeded ansible inventory to page against")
+	}
+	invUUID, err := uuid.Parse(inventoryID)
+	if err != nil {
+		t.Fatalf("seeded inventory id %q is not a uuid: %v", inventoryID, err)
+	}
+
+	// The harness database is created and dropped per test, so seeding here touches nothing real.
+	var before int64
+	h.db.Model(&models.AnsibleInventorySync{}).Where("inventory_id = ?", invUUID).Count(&before)
+	for i := 1; i <= 3; i++ {
+		started := time.Now().Add(-time.Duration(i) * time.Minute)
+		sync := models.AnsibleInventorySync{
+			InventoryID: invUUID,
+			Status:      models.InventorySyncStatusSuccessful,
+			TriggeredBy: "manual",
+			StartedAt:   &started,
+		}
+		if err := h.db.Create(&sync).Error; err != nil {
+			t.Fatalf("seed inventory sync %d: %v", i, err)
+		}
+	}
+	total := before + 3
+
+	get := func(t *testing.T, path string) map[string]any {
+		t.Helper()
+		req := httptest.NewRequest(http.MethodGet, path, nil)
+		req.Header.Set("Authorization", "Bearer "+h.token)
+		rec := httptest.NewRecorder()
+		h.router.ServeHTTP(rec, req)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("GET %s = %d, want 200: %s", path, rec.Code, rec.Body.String())
+		}
+		var body map[string]any
+		if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+			t.Fatalf("GET %s returned non-JSON: %v", path, err)
+		}
+		return body
+	}
+
+	ids := func(t *testing.T, body map[string]any) []string {
+		t.Helper()
+		data, ok := body["data"].([]any)
+		if !ok {
+			t.Fatalf("response has no data array; keys were %v", keysOf(body))
+		}
+		out := make([]string, 0, len(data))
+		for _, raw := range data {
+			res, _ := raw.(map[string]any)
+			id, _ := res["id"].(string)
+			out = append(out, id)
+		}
+		return out
+	}
+
+	base := fmt.Sprintf("/api/v2/ansible/inventories/%s/syncs", inventoryID)
+
+	t.Run("page[number] moves the window instead of repeating page one", func(t *testing.T) {
+		first := ids(t, get(t, base+"?page%5Bsize%5D=1&page%5Bnumber%5D=1"))
+		second := ids(t, get(t, base+"?page%5Bsize%5D=1&page%5Bnumber%5D=2"))
+		if len(first) != 1 || len(second) != 1 {
+			t.Fatalf("expected one row per page, got %d and %d", len(first), len(second))
+		}
+		if first[0] == second[0] {
+			t.Errorf("page 2 returned the same row as page 1 (%s) - page[number] is ignored, so a "+
+				"client walking the pages collects duplicates and never reaches the tail", first[0])
+		}
+	})
+
+	t.Run("total-count describes the whole collection", func(t *testing.T) {
+		body := get(t, base+"?page%5Bsize%5D=1&page%5Bnumber%5D=1")
+		meta, ok := body["meta"].(map[string]any)
+		if !ok {
+			t.Fatalf("response has no meta object; keys were %v", keysOf(body))
+		}
+		pag, ok := meta["pagination"].(map[string]any)
+		if !ok {
+			t.Fatalf("meta has no pagination block; meta keys were %v", keysOf(meta))
+		}
+		if got, _ := pag["total-count"].(float64); int64(got) != total {
+			t.Errorf("total-count = %v, want %d", got, total)
+		}
+	})
+
+	// PageParams falls back to the handler default rather than passing 0 through. An unclamped
+	// parser sends LIMIT 0, which answers every page with zero rows while still advertising
+	// total-pages - a client paging that never terminates on data it can see.
+	t.Run("a zero page[size] falls back to the default rather than returning nothing", func(t *testing.T) {
+		body := get(t, base+"?page%5Bsize%5D=0")
+		if rows := ids(t, body); len(rows) == 0 && total > 0 {
+			t.Errorf("page[size]=0 returned no rows out of %d - the parameter is passed straight "+
+				"through to LIMIT instead of falling back to the default", total)
+		}
+	})
+}