diff --git a/frontend/src/__tests__/history-truncation-notice.test.ts b/frontend/src/__tests__/history-truncation-notice.test.ts new file mode 100644 index 000000000..55eb3a0ce --- /dev/null +++ b/frontend/src/__tests__/history-truncation-notice.test.ts @@ -0,0 +1,112 @@ +/** + * History truncation banner (issue #248): the API flags `truncated` when a + * fetch hit its row cap; the UI must say so above the queue and the table. + */ + +import { loadHistory } from '../history'; + +jest.mock('../api', () => ({ + getHistory: jest.fn(), + getConfig: jest.fn().mockResolvedValue({ global: {} }), + listAccounts: jest.fn().mockResolvedValue([]), + getDeploymentInfo: jest.fn().mockResolvedValue({}), +})); +jest.mock('../navigation', () => ({ switchTab: jest.fn() })); +jest.mock('../utils', () => ({ + formatCurrency: jest.fn((val) => `$${val || 0}`), + formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')), + formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)), + escapeHtml: jest.fn((str) => str || ''), + escapeHtmlAttr: jest.fn((str: string | null | undefined) => str || ''), + populateAccountFilter: jest.fn(() => Promise.resolve()), +})); +jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); +jest.mock('../toast', () => ({ showToast: jest.fn() })); +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(), + getCurrentProvider: jest.fn().mockReturnValue(''), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), + getAmortizeUpfront: jest.fn().mockReturnValue(false), + setAmortizeUpfront: jest.fn(), + subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), +})); +jest.mock('../recommendations', () => ({ getAccountName: jest.fn((id: string) => id) })); + +import * as api from '../api'; + +function setupDOM(): void { + while (document.body.firstChild) document.body.removeChild(document.body.firstChild); + const ids = ['history-start', 'history-end']; + for (const id of ids) { + const el = document.createElement('input'); + el.type = 'date'; + el.id = id; + document.body.appendChild(el); + } + for (const id of ['history-summary', 'history-list', 'purchases-approval-queue']) { + const el = document.createElement('div'); + el.id = id; + document.body.appendChild(el); + } + for (const id of ['purchases-truncation-notice', 'history-truncation-notice']) { + const el = document.createElement('p'); + el.id = id; + el.hidden = true; + document.body.appendChild(el); + } +} + +describe('History truncation banner (issue #248)', () => { + beforeEach(() => { + setupDOM(); + jest.clearAllMocks(); + }); + + test('shows the banner above both tables when the API reports truncation', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [], truncated: true, limit: 100, executions_limit: 100 }); + await loadHistory(); + for (const id of ['purchases-truncation-notice', 'history-truncation-notice']) { + const el = document.getElementById(id)!; + expect(el.hidden).toBe(false); + expect(el.textContent).toContain('100'); + } + }); + + test('keeps the banner hidden when the response is not truncated', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [], truncated: false, limit: 100 }); + await loadHistory(); + for (const id of ['purchases-truncation-notice', 'history-truncation-notice']) { + expect(document.getElementById(id)!.hidden).toBe(true); + } + }); + + test('clears a previous banner on a later untruncated load', async () => { + (api.getHistory as jest.Mock).mockResolvedValueOnce({ summary: {}, purchases: [], truncated: true, limit: 100, executions_limit: 100 }); + await loadHistory(); + (api.getHistory as jest.Mock).mockResolvedValueOnce({ summary: {}, purchases: [] }); + await loadHistory(); + expect(document.getElementById('history-truncation-notice')!.hidden).toBe(true); + expect(document.getElementById('history-truncation-notice')!.textContent).toBe(''); + }); + + test('hides a previous banner when the next load fails', async () => { + (api.getHistory as jest.Mock).mockResolvedValueOnce({ summary: {}, purchases: [], truncated: true, limit: 100, executions_limit: 100 }); + await loadHistory(); + (api.getHistory as jest.Mock).mockRejectedValueOnce(new Error('boom')); + jest.spyOn(console, 'error').mockImplementation(() => {}); + await loadHistory(); + for (const id of ['purchases-truncation-notice', 'history-truncation-notice']) { + expect(document.getElementById(id)!.hidden).toBe(true); + } + }); +}); diff --git a/frontend/src/history.ts b/frontend/src/history.ts index 3de24240b..7e7ab4a8c 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -270,6 +270,7 @@ export async function viewPlanHistory(planId: string): Promise { ]); if (requestEpoch !== historyMutationEpoch) return; renderHistorySummary(data.summary ?? null); + renderTruncationNotices(data); const purchases = data.purchases || []; renderApprovalQueue(purchases); renderHistoryList(purchases); @@ -277,6 +278,7 @@ export async function viewPlanHistory(planId: string): Promise { } catch (error) { if (requestEpoch !== historyMutationEpoch) return; console.error('Failed to load plan history:', error); + renderTruncationNotices({}); const err = error as Error; const list = document.getElementById('history-list'); if (list) { @@ -362,12 +364,14 @@ export async function loadHistory(): Promise { ]); if (requestEpoch !== historyMutationEpoch) return; renderHistorySummary(data.summary ?? null); + renderTruncationNotices(data); const purchases = data.purchases || []; renderApprovalQueue(purchases); renderHistoryList(purchases); } catch (error) { if (requestEpoch !== historyMutationEpoch) return; console.error('Failed to load history:', error); + renderTruncationNotices({}); const err = error as Error; const list = document.getElementById('history-list'); if (list) { @@ -382,6 +386,23 @@ export async function loadHistory(): Promise { } } +/** + * Show or hide the "results capped" banner above the approval queue and the + * history table. The API sets `truncated` when a fetch hit its row cap, in + * which case pending approvals and the summary totals can under-report. + */ +function renderTruncationNotices(data: HistoryResponse): void { + const message = data.truncated + ? `Results are capped at the most recent ${data.limit ?? '?'} completed purchases and ${data.executions_limit ?? '?'} pending, failed or other non-completed executions. Older rows are not shown, so pending approvals and totals may be incomplete. Narrow the date range or filters to see them.` + : ''; + for (const id of ['purchases-truncation-notice', 'history-truncation-notice']) { + const el = document.getElementById(id); + if (!el) continue; + el.textContent = message; + el.hidden = !data.truncated; + } +} + function renderHistorySummary(summary: HistorySummary | null): void { const container = document.getElementById('history-summary'); if (!container) return; diff --git a/frontend/src/index.html b/frontend/src/index.html index 5e41088bd..34acdfc0f 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -180,6 +180,7 @@

Planned Purchases

Approval queue

Purchases awaiting approval. Review and approve or cancel each one. The same actions are available from the Purchase History table below.

+
@@ -249,6 +250,7 @@

Purchase History

+
diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 89663abb3..f1bd751e1 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -193,6 +193,12 @@ export interface SavePlanData { export interface HistoryResponse { summary?: HistorySummary; purchases?: HistoryPurchase[]; + /** True when a fetch hit its row cap, so older rows may be missing. */ + truncated?: boolean; + /** Row cap on completed purchase history rows for the request. */ + limit?: number; + /** Fixed row cap on non-completed executions. */ + executions_limit?: number; } export interface HistorySummary { diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index 4f2e0e5f6..e1d8f999d 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -61,7 +61,7 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR // status. On servers the transitions fire in a goroutine; on Lambda they // run synchronously before returning because the execution environment // freezes once the response is out (issue #1170). - extra, staleExecs := h.fetchExecutionsAsHistory(ctx, filters) + extra, staleExecs, executionsCapped := h.fetchExecutionsAsHistory(ctx, filters) h.expireStaleExecutions(staleExecs) all := make([]config.PurchaseHistoryRecord, 0, len(completed)+len(extra)) @@ -79,9 +79,15 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR return all[i].Timestamp.After(all[j].Timestamp) }) + // Truncation is judged on the raw fetches (before the in-memory and scope + // filters) because the cap drops rows before they run. It is a single + // boolean: a count would reveal other tenants' row volume to a scoped user. return HistoryResponse{ - Summary: summarizePurchaseHistory(all), - Purchases: all, + Summary: summarizePurchaseHistory(all), + Purchases: all, + Truncated: len(completed) >= filters.Limit || executionsCapped, + Limit: filters.Limit, + ExecutionsLimit: config.DefaultListLimit, }, nil } @@ -145,19 +151,25 @@ const approvalExpiryWindow = 7 * 24 * time.Hour // The filter set (issue #701) is applied in Go against the synthetic row: // provider via the recs' collapsed provider, account via CloudAccountID, // date via ScheduledDate. -func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution) { +// +// The third return value reports that the fetch reached its cap, so older +// executions may have been dropped before the filters ran. +func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) { executions, err := h.config.GetExecutionsByStatuses(ctx, historyExecutionStatuses, config.DefaultListLimit) if err != nil { logging.Warnf("history: failed to load non-completed executions: %v", err) - return nil, nil + return nil, nil, false } if len(executions) == 0 { - return nil, nil + return nil, nil, false } approver := h.resolvePendingApproverEmail(ctx) userEmailCache := h.resolveUserEmails(ctx, executions) out := make([]config.PurchaseHistoryRecord, 0, len(executions)) var staleExecs []config.PurchaseExecution + // Rows the History view can surface; clean completed executions do not + // count toward the cap signal (the store already excludes them). + surfaced := 0 for _rvc := range executions { exec := executions[_rvc] // Dedup: a normal completed execution is already represented by its @@ -168,6 +180,7 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF if exec.Status == "completed" && exec.Error == "" { continue } + surfaced++ // Collect stale pending/notified executions for the post-assembly // expire sweep. We do NOT mutate status here to keep the GET // read-only: the response reflects current DB state; the sweep @@ -184,7 +197,7 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF } out = append(out, executionToHistoryRow(exec, approver, createdByEmail)) } - return out, staleExecs + return out, staleExecs, surfaced >= config.DefaultListLimit } // isStaleExecution reports whether the execution is a pending/notified diff --git a/internal/api/handler_history_truncation_test.go b/internal/api/handler_history_truncation_test.go new file mode 100644 index 000000000..4fe6dfc6c --- /dev/null +++ b/internal/api/handler_history_truncation_test.go @@ -0,0 +1,164 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func historyRows(n int) []config.PurchaseHistoryRecord { + rows := make([]config.PurchaseHistoryRecord, n) + for i := range rows { + rows[i] = config.PurchaseHistoryRecord{PurchaseID: fmt.Sprintf("p-%d", i), Timestamp: time.Now()} + } + return rows +} + +func pendingExecutions(n int) []config.PurchaseExecution { + execs := make([]config.PurchaseExecution, n) + for i := range execs { + execs[i] = config.PurchaseExecution{ + ExecutionID: fmt.Sprintf("e-%d", i), + Status: "pending", + ScheduledDate: time.Now(), + } + } + return execs +} + +// historyJSON round-trips the response through JSON so the assertion sees the +// wire shape a client sees, not the Go struct. +func historyJSON(t *testing.T, resp any) map[string]any { + t.Helper() + raw, err := json.Marshal(resp) + require.NoError(t, err) + var out map[string]any + require.NoError(t, json.Unmarshal(raw, &out)) + return out +} + +func runAdminHistory(t *testing.T, completed []config.PurchaseHistoryRecord, execs []config.PurchaseExecution, params map[string]string) map[string]any { + t.Helper() + ctx := context.Background() + mockStore := new(MockConfigStore) + mockStore.On("GetAllPurchaseHistory", ctx, mock.Anything).Return(completed, nil) + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil).Maybe() + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore} + + result, err := handler.getHistory(ctx, req, params) + require.NoError(t, err) + return historyJSON(t, result) +} + +func TestHandler_getHistory_TruncatedWhenCompletedAtCap(t *testing.T) { + body := runAdminHistory(t, historyRows(config.DefaultListLimit), nil, map[string]string{}) + assert.Equal(t, true, body["truncated"], "a full page of completed rows may hide older ones") + assert.EqualValues(t, config.DefaultListLimit, body["limit"]) +} + +func TestHandler_getHistory_TruncatedWhenExecutionsAtCap(t *testing.T) { + // The execution fetch is capped before the in-memory filters run, so a + // full page means pending approvals may have been dropped. + body := runAdminHistory(t, nil, pendingExecutions(config.DefaultListLimit), map[string]string{}) + assert.Equal(t, true, body["truncated"]) +} + +func cleanCompletedExecutions(n int) []config.PurchaseExecution { + execs := pendingExecutions(n) + for i := range execs { + execs[i].Status = "completed" + } + return execs +} + +// Clean completed executions are skipped by the handler (and excluded by the +// store query), so a mature tenant must not see a permanent banner. +func TestHandler_getHistory_NotTruncatedByCleanCompletedExecutions(t *testing.T) { + body := runAdminHistory(t, historyRows(3), cleanCompletedExecutions(config.DefaultListLimit), map[string]string{}) + assert.Equal(t, false, body["truncated"]) + assert.Len(t, body["purchases"], 3) +} + +func TestHandler_getHistory_TruncatedByFailedExecutions(t *testing.T) { + execs := pendingExecutions(config.DefaultListLimit) + for i := range execs { + execs[i].Status = "failed" + } + body := runAdminHistory(t, nil, execs, map[string]string{}) + assert.Equal(t, true, body["truncated"]) +} + +func TestHandler_getHistory_AuditGapCompletedExecutionsCountTowardCap(t *testing.T) { + execs := cleanCompletedExecutions(config.DefaultListLimit) + for i := range execs { + execs[i].Error = "history write failed" + } + body := runAdminHistory(t, nil, execs, map[string]string{}) + assert.Equal(t, true, body["truncated"]) +} + +func TestHandler_getHistory_ExecutionsLimitIsFixed(t *testing.T) { + body := runAdminHistory(t, historyRows(2), pendingExecutions(config.DefaultListLimit), map[string]string{"limit": "500"}) + assert.EqualValues(t, 500, body["limit"]) + assert.EqualValues(t, config.DefaultListLimit, body["executions_limit"]) + assert.Equal(t, true, body["truncated"]) +} + +func TestHandler_getHistory_TruncatedHonorsCustomLimit(t *testing.T) { + body := runAdminHistory(t, historyRows(5), nil, map[string]string{"limit": "5"}) + assert.Equal(t, true, body["truncated"]) + assert.EqualValues(t, 5, body["limit"]) +} + +func TestHandler_getHistory_NotTruncatedUnderLimit(t *testing.T) { + body := runAdminHistory(t, historyRows(config.DefaultListLimit-1), pendingExecutions(config.DefaultListLimit-1), map[string]string{}) + assert.Equal(t, false, body["truncated"]) + assert.EqualValues(t, config.DefaultListLimit, body["limit"]) +} + +// A scoped user's visible rows can be few while the fetch hit the cap: the +// flag is derived from the pre-scope fetch (the cap drops rows before scope +// filtering) but must expose only a boolean, never other tenants' counts. +func TestHandler_getHistory_ScopedUserTruncationLeaksNoCounts(t *testing.T) { + ctx := context.Background() + const scopedUserID = "scoped-user-id" + own := scopedUserID + execs := pendingExecutions(config.DefaultListLimit) + execs[0].CreatedByUserID = &own + + mockStore := new(MockConfigStore) + mockStore.On("GetAllPurchaseHistory", ctx, mock.Anything).Return([]config.PurchaseHistoryRecord{}, nil) + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return nil, nil + } + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", ctx, "scoped-token").Return(&Session{UserID: scopedUserID, Email: "s@example.com"}, nil) + mockAuth.On("HasPermissionAPI", ctx, scopedUserID, "view", "purchases").Return(true, nil) + mockAuth.On("GetAllowedAccountsAPI", ctx, scopedUserID).Return([]string{"aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa"}, nil) + mockAuth.On("GetUser", ctx, mock.Anything).Return(&User{Email: "s@example.com"}, nil).Maybe() + handler := &Handler{auth: mockAuth, config: mockStore} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer scoped-token"}} + + result, err := handler.getHistory(ctx, req, map[string]string{}) + require.NoError(t, err) + body := historyJSON(t, result) + + assert.Len(t, body["purchases"], 1, "scoped user sees only their own row") + assert.Equal(t, true, body["truncated"]) + assert.NotContains(t, body, "total", "no count of rows outside the caller's scope may be exposed") + summary, ok := body["summary"].(map[string]any) + require.True(t, ok) + assert.EqualValues(t, 1, summary["total_purchases"]) +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index ec8a7ab3e..0669b705c 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -3429,6 +3429,18 @@ components: type: array items: $ref: '#/components/schemas/PurchaseHistoryRecord' + truncated: + type: boolean + description: >- + True when a fetch reached its row cap, so older rows (and possibly + pending approvals) may be missing from purchases and summary. May + be a false positive when exactly `limit` or `executions_limit` rows exist. + limit: + type: integer + description: Row cap on completed purchase history rows for this request. + executions_limit: + type: integer + description: Row cap on non-completed executions (fixed, ignores the limit parameter). HistorySummary: type: object diff --git a/internal/api/types.go b/internal/api/types.go index 709fd4bf1..0841f93cd 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -922,6 +922,15 @@ type CreatePlannedPurchasesResponse struct { type HistoryResponse struct { Purchases []config.PurchaseHistoryRecord `json:"purchases"` Summary HistorySummary `json:"summary"` + // Truncated is true when a fetch reached its row cap, so older rows (and + // possibly pending approvals) may be missing from Purchases and Summary. + // It can be a false positive when exactly Limit (or ExecutionsLimit) rows exist. + Truncated bool `json:"truncated"` + // Limit is the row cap on completed purchase_history rows for this request. + Limit int `json:"limit"` + // ExecutionsLimit is the row cap on non-completed executions, which is + // fixed regardless of the request's limit parameter. + ExecutionsLimit int `json:"executions_limit"` } // HistorySummary provides aggregate statistics for purchase history. diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 63c284904..e952067ee 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1360,6 +1360,10 @@ func (s *PostgresStore) CancelScheduledExecutionAtomic(ctx context.Context, tx p // handler to merge pending/failed/expired rows alongside completed purchases // without changing the narrower GetPendingExecutions contract the scheduler // depends on. +// +// Executions with status 'completed' and an empty error are excluded: they are +// already represented by their purchase_history rows, so returning them would +// only consume the cap (the History handler skips them anyway). func (s *PostgresStore) GetExecutionsByStatuses(ctx context.Context, statuses []string, limit int) ([]PurchaseExecution, error) { if len(statuses) == 0 { return nil, nil @@ -1381,6 +1385,7 @@ func (s *PostgresStore) GetExecutionsByStatuses(ctx context.Context, statuses [] idempotency_key, scheduled_execution_at FROM purchase_executions WHERE status = ANY($1) + AND NOT (status = 'completed' AND COALESCE(error, '') = '') ORDER BY scheduled_date DESC LIMIT $2 ` diff --git a/internal/config/store_postgres_executions_by_statuses_integration_test.go b/internal/config/store_postgres_executions_by_statuses_integration_test.go new file mode 100644 index 000000000..9de546ba7 --- /dev/null +++ b/internal/config/store_postgres_executions_by_statuses_integration_test.go @@ -0,0 +1,56 @@ +//go:build integration +// +build integration + +package config + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestIntegration_GetExecutionsByStatuses_RowClasses runs the real query over +// each row class the History view cares about. A clean completed execution +// (status completed, empty error) must be excluded; a completed execution with +// an error (the audit-gap case) and every other requested status must be +// returned, newest first and bounded by the limit. +func TestIntegration_GetExecutionsByStatuses_RowClasses(t *testing.T) { + ctx := context.Background() + store := setupRampStepStore(ctx, t) + + base := time.Now().Add(-time.Hour) + save := func(offset int, status, execErr string) string { + exec := &PurchaseExecution{ + ExecutionID: uuid.New().String(), + IdempotencyKey: uuid.New().String(), + Status: status, + Error: execErr, + ScheduledDate: base.Add(time.Duration(offset) * time.Minute), + } + require.NoError(t, store.SavePurchaseExecution(ctx, exec)) + return exec.ExecutionID + } + + save(1, "completed", "") + auditGap := save(2, "completed", "history write failed") + failed := save(3, "failed", "boom") + pending := save(4, "pending", "") + + got, err := store.GetExecutionsByStatuses(ctx, []string{"pending", "completed", "failed"}, 100) + require.NoError(t, err) + ids := make([]string, len(got)) + for i := range got { + ids[i] = got[i].ExecutionID + } + assert.Equal(t, []string{pending, failed, auditGap}, ids, "clean completed excluded; newest first") + + limited, err := store.GetExecutionsByStatuses(ctx, []string{"pending", "completed", "failed"}, 2) + require.NoError(t, err) + require.Len(t, limited, 2) + assert.Equal(t, pending, limited[0].ExecutionID) + assert.Equal(t, failed, limited[1].ExecutionID) +} diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index a80988043..83eacf144 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -3062,3 +3062,22 @@ func TestPGXMock_CountExecutionsByPlanAndStatus_QueryError(t *testing.T) { assert.Contains(t, err.Error(), "failed to count executions by plan and status") assert.NoError(t, mock.ExpectationsWereMet()) } + +// TestPGXMock_GetExecutionsByStatuses_ExcludesCleanCompleted guards the SQL +// predicate that keeps clean completed executions (status completed, empty +// error) out of the result: they only consume the cap, because the History +// handler skips them. Dropping the predicate lets them push pending +// approvals past the cap again. +func TestPGXMock_GetExecutionsByStatuses_ExcludesCleanCompleted(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + + mock.ExpectQuery(`(?s)FROM purchase_executions\s+WHERE status = ANY\(\$1\)\s+AND NOT \(status = 'completed' AND COALESCE\(error, ''\) = ''\)\s+ORDER BY scheduled_date DESC\s+LIMIT \$2`). + WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()). + WillReturnRows(pgxmock.NewRows(stuckExecCols())) + + execs, err := store.GetExecutionsByStatuses(context.Background(), []string{"pending", "completed"}, 100) + require.NoError(t, err) + assert.Empty(t, execs) + assert.NoError(t, mock.ExpectationsWereMet()) +}