diff --git a/internal/api/handler_execution_settle.go b/internal/api/handler_execution_settle.go new file mode 100644 index 000000000..92667924c --- /dev/null +++ b/internal/api/handler_execution_settle.go @@ -0,0 +1,159 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "strings" + "time" + "unicode/utf8" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" +) + +const ( + executionSettleReasonMin = 10 + executionSettleReasonMax = 300 + executionSettleBodyMax = 2048 +) + +// executionSettleRequest is the body of POST /api/purchases/{id}/settle. +type executionSettleRequest struct { + Reason string `json:"reason"` + Acknowledge bool `json:"acknowledge_unknown_outcome"` +} + +type executionSettleResponse struct { + ExecutionID string `json:"execution_id"` + Status string `json:"status"` + SettledAt string `json:"outcome_settled_at"` + SettledBy string `json:"outcome_settled_by"` + Settlement string `json:"outcome_settlement"` + Replayed bool `json:"replayed"` +} + +// executionSettleStore is the part of the store the endpoint needs. +type executionSettleStore interface { + SettleFailedExecution(ctx context.Context, req config.ExecutionSettleRequest) (*config.ExecutionSettlement, error) +} + +// settleExecution records an operator's audited settlement of a failed plan +// execution whose outcome is unknown (issue #999), so it stops blocking the +// delete of its editor-managed plan. The row stays failed and keeps counting +// as possibly bought; it can no longer be retried. Gated like a purchase: +// execute:purchases, account access to the execution, and the grant's account +// constraints with strict scope. +func (h *Handler) settleExecution(ctx context.Context, req *events.LambdaFunctionURLRequest, id string) (any, error) { + if err := validateUUID(id); err != nil { + return nil, err + } + session, err := h.requirePermission(ctx, req, auth.ActionExecute, auth.ResourcePurchases) + if err != nil { + return nil, err + } + body, err := decodeExecutionSettle(req.Body) + if err != nil { + return nil, err + } + if authErr := h.authorizeExecutionSettle(ctx, session, id); authErr != nil { + return nil, authErr + } + store, ok := h.config.(executionSettleStore) + if !ok { + return nil, NewClientError(503, "Execution settle storage is unavailable") + } + res, err := store.SettleFailedExecution(ctx, config.ExecutionSettleRequest{ + ExecutionID: id, Actor: session.UserID, Reason: body.Reason, Acknowledge: body.Acknowledge, + }) + if err != nil { + return nil, mapExecutionSettleError(err) + } + if !res.Replayed { + logging.Infof("purchase execution settled by operator: execution=%s actor=%q via=%s acknowledged_unknown_outcome=true settlement=%q", + id, session.UserID, credentialKind(ctx, session), res.Settlement) + } + return executionSettleResponse{ + ExecutionID: res.ExecutionID, Status: res.Status, SettledAt: res.SettledAt.UTC().Format(time.RFC3339), + SettledBy: res.SettledBy, Settlement: res.Settlement, Replayed: res.Replayed, + }, nil +} + +// authorizeExecutionSettle checks account access to the execution, then the +// execute:purchases constraints against what the execution would have bought +// (its recommendations' providers, services, regions and total commitment) +// and its accounts. StrictScope makes a dimension the execution leaves +// unknown, such as the provider of a step with no recommendations, require an +// unconstrained grant. +func (h *Handler) authorizeExecutionSettle(ctx context.Context, session *Session, id string) error { + if err := h.requireExecutionAccess(ctx, session, id); err != nil { + return err + } + execution, err := h.config.GetExecutionByID(ctx, id) + if errors.Is(err, config.ErrNotFound) || (err == nil && execution == nil) { + return errNotFound + } + if err != nil { + return fmt.Errorf("failed to get execution: %w", err) + } + accounts, err := h.executionAccounts(ctx, execution, nil) + if err != nil { + return err + } + ids := make([]string, 0, len(accounts)) + for i := range accounts { + ids = append(ids, accounts[i].ID) + } + sets := purchaseConstraintSets(execution.Recommendations) + if len(sets) == 0 { + sets = []auth.PermissionConstraints{{}} + } + for i := range sets { + sets[i].AccountIDs, sets[i].StrictScope = ids, true + } + return h.requirePermissionConstraints(ctx, session, auth.ActionExecute, auth.ResourcePurchases, sets) +} + +func mapExecutionSettleError(err error) error { + switch { + case errors.Is(err, config.ErrExecutionSettleNotFound): + return errNotFound + case errors.Is(err, config.ErrExecutionSettleInvalid): + return NewClientError(400, err.Error()) + case errors.Is(err, config.ErrExecutionSettleConflict): + return NewClientError(409, err.Error()) + case errors.Is(err, config.ErrExecutionSettleNotReady): + return NewClientError(503, err.Error()) + } + return err +} + +func decodeExecutionSettle(raw string) (executionSettleRequest, error) { + var body executionSettleRequest + if len(raw) > executionSettleBodyMax { + return body, NewClientError(400, "Settle body is too large") + } + decoder := json.NewDecoder(strings.NewReader(raw)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&body); err != nil { + return body, NewClientError(400, "Invalid settle body") + } + if err := decoder.Decode(new(any)); err != io.EOF { + return body, NewClientError(400, "Expected one settle body") + } + body.Reason = strings.TrimSpace(body.Reason) + if !body.Acknowledge { + return body, NewClientError(400, "acknowledge_unknown_outcome must be true: this purchase may have been submitted; check the cloud account first") + } + if n := utf8.RuneCountInString(body.Reason); n < executionSettleReasonMin || n > executionSettleReasonMax { + return body, NewClientError(400, fmt.Sprintf("reason must be %d to %d characters", executionSettleReasonMin, executionSettleReasonMax)) + } + if hasControlRune(body.Reason) { + return body, NewClientError(400, "reason must not contain control characters") + } + return body, nil +} diff --git a/internal/api/handler_execution_settle_integration_test.go b/internal/api/handler_execution_settle_integration_test.go new file mode 100644 index 000000000..2e3812592 --- /dev/null +++ b/internal/api/handler_execution_settle_integration_test.go @@ -0,0 +1,112 @@ +//go:build integration + +package api + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +const settleBody = `{"reason":"checked the subscription: nothing was bought","acknowledge_unknown_outcome":true}` + +var purchaserPerms = []auth.Permission{ + {Action: auth.ActionAdmin, Resource: auth.ResourceAll}, + {Action: auth.ActionExecute, Resource: auth.ResourcePurchases}, +} + +func (f *createConcurrencyFixture) settledAt(ctx context.Context, t *testing.T, id string) *time.Time { + t.Helper() + var at *time.Time + require.NoError(t, f.pool.QueryRow(ctx, `SELECT outcome_settled_at FROM purchase_executions WHERE execution_id = $1`, id).Scan(&at)) + return at +} + +// The issue #999 scenario through the HTTP surface: a failed row with an +// unknown outcome makes the managed delete 409 until a purchaser settles it. +func TestSettleUnblocksManagedPlanDeleteOverHTTP(t *testing.T) { + ctx := context.Background() + f := newCreateConcurrencyFixture(ctx, t) + setPlanManaged(ctx, t, f, true) + id := f.saveRow(ctx, t, "failed", false, time.Now(), true) + requireLiveExecutionsRefusal(t, f.deletePlan(ctx, f.handler)) + + h := limitedHandler(f, purchaserPerms, nil) + h.auth.(*MockAuthService).On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil) + req := managedGuardRequest(settleBody) + req.Headers["X-CSRF-Token"] = "test-csrf" + req.RequestContext.HTTP.Method = "POST" + req.RequestContext.HTTP.Path = "/api/purchases/" + id + "/settle" + resp, err := h.HandleRequest(ctx, req) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode, resp.Body) + var got executionSettleResponse + require.NoError(t, json.Unmarshal([]byte(resp.Body), &got)) + require.Equal(t, id, got.ExecutionID) + require.Equal(t, "failed", got.Status) + require.Equal(t, "00000000-0000-0000-0000-000000000001", got.SettledBy) + require.Contains(t, got.Settlement, `reason="checked the subscription: nothing was bought"`) + require.False(t, got.Replayed) + + require.NoError(t, f.deletePlan(ctx, f.handler)) + row, err := f.store.GetExecutionByID(ctx, id) + require.NoError(t, err) + require.Equal(t, "failed", row.Status) + require.NotNil(t, f.settledAt(ctx, t, id)) +} + +func TestSettleAuthorizationAndValidation(t *testing.T) { + ctx := context.Background() + f := newCreateConcurrencyFixture(ctx, t) + setPlanManaged(ctx, t, f, true) + id := f.saveRow(ctx, t, "failed", false, time.Now(), true) + completed := f.saveRow(ctx, t, "completed", false, time.Now(), true) + var acct string + require.NoError(t, f.pool.QueryRow(ctx, `SELECT account_id::text FROM plan_accounts WHERE plan_id = $1`, f.planID).Scan(&acct)) + constrained := func(c auth.PermissionConstraints) []auth.Permission { + return []auth.Permission{{Action: auth.ActionExecute, Resource: auth.ResourcePurchases, Constraints: &c}} + } + cases := []struct { + name string + perms []auth.Permission + scoped []string + id string + body string + status int + }{ + {"admin without execute", []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}, nil, id, settleBody, http.StatusForbidden}, + {"out of account scope", purchaserPerms, []string{"other-account"}, id, settleBody, http.StatusNotFound}, + {"execute constrained to another account", constrained(auth.PermissionConstraints{AccountIDs: []string{"other-account"}}), nil, id, settleBody, http.StatusForbidden}, + {"execute constrained to another provider", constrained(auth.PermissionConstraints{AccountIDs: []string{acct}, Providers: []string{"aws"}}), nil, id, settleBody, http.StatusForbidden}, + {"no acknowledgement", purchaserPerms, nil, id, `{"reason":"checked the subscription"}`, http.StatusBadRequest}, + {"short reason", purchaserPerms, nil, id, `{"reason":"ok","acknowledge_unknown_outcome":true}`, http.StatusBadRequest}, + {"unknown field", purchaserPerms, nil, id, `{"reason":"checked the subscription","acknowledge_unknown_outcome":true,"force":true}`, http.StatusBadRequest}, + {"completed row", purchaserPerms, nil, completed, settleBody, http.StatusConflict}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := limitedHandler(f, tc.perms, tc.scoped) + _, err := h.settleExecution(ctx, managedGuardRequest(tc.body), tc.id) + var ce *clientError + if errors.As(err, &ce) { + require.Equal(t, tc.status, ce.code, ce.message) + } else { + require.True(t, IsNotFoundError(err) && tc.status == http.StatusNotFound, "got %v", err) + } + require.Nil(t, f.settledAt(ctx, t, tc.id), "a refused settle writes nothing") + }) + } + + t.Run("constrained to the plan's account", func(t *testing.T) { + h := limitedHandler(f, constrained(auth.PermissionConstraints{AccountIDs: []string{acct}}), nil) + _, err := h.settleExecution(ctx, managedGuardRequest(settleBody), id) + require.NoError(t, err) + }) +} diff --git a/internal/api/handler_execution_settle_test.go b/internal/api/handler_execution_settle_test.go new file mode 100644 index 000000000..d4e7e942d --- /dev/null +++ b/internal/api/handler_execution_settle_test.go @@ -0,0 +1,45 @@ +package api + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/require" +) + +func TestDecodeExecutionSettle(t *testing.T) { + ok, err := decodeExecutionSettle(`{"reason":" checked the account ","acknowledge_unknown_outcome":true}`) + require.NoError(t, err) + require.Equal(t, "checked the account", ok.Reason) + + for name, body := range map[string]string{ + "empty": ``, + "no ack": `{"reason":"checked the account"}`, + "ack false": `{"reason":"checked the account","acknowledge_unknown_outcome":false}`, + "short": `{"reason":" short ","acknowledge_unknown_outcome":true}`, + "long": `{"reason":"` + strings.Repeat("x", executionSettleReasonMax+1) + `","acknowledge_unknown_outcome":true}`, + "control": `{"reason":"checked the\naccount","acknowledge_unknown_outcome":true}`, + "unknown field": `{"reason":"checked the account","acknowledge_unknown_outcome":true,"x":1}`, + "two bodies": `{"reason":"checked the account","acknowledge_unknown_outcome":true}{}`, + "too large": strings.Repeat(" ", executionSettleBodyMax+1), + } { + _, err := decodeExecutionSettle(body) + var ce *clientError + require.True(t, errors.As(err, &ce), name) + require.Equal(t, 400, ce.code, name) + } +} + +func TestMapExecutionSettleError(t *testing.T) { + require.True(t, IsNotFoundError(mapExecutionSettleError(config.ErrExecutionSettleNotFound))) + for sentinel, code := range map[error]int{config.ErrExecutionSettleInvalid: 400, config.ErrExecutionSettleConflict: 409} { + var ce *clientError + require.True(t, errors.As(mapExecutionSettleError(fmt.Errorf("%w: detail", sentinel)), &ce)) + require.Equal(t, code, ce.code) + } + other := errors.New("db down") + require.Equal(t, other, mapExecutionSettleError(other), "an internal error is not a client error") +} diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index adb03d3d8..434d95928 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -2848,7 +2848,7 @@ func (h *Handler) persistRetrySuccessorTx(ctx context.Context, failedExec, newEx map[string]any{}) case errors.Is(err, errRetryLinkRaceLost): return NewClientErrorWithDetails(409, - fmt.Sprintf("execution %s was already retried; act on its descendant instead", failedExec.ExecutionID), + fmt.Sprintf("execution %s was already retried or settled; reload it", failedExec.ExecutionID), map[string]any{}) case errors.As(err, &dup): return NewClientErrorWithDetails(409, diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 020c96f3f..d7b1da79e 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -1487,6 +1487,92 @@ paths: '500': $ref: '#/components/responses/InternalError' + /api/purchases/{id}/settle: + parameters: + - $ref: '#/components/parameters/ResourceID' + post: + operationId: settlePurchaseExecution + tags: [Purchases] + summary: Record an operator settlement of an unknown purchase outcome + description: > + Records an audited settlement of a failed plan execution after the + operator checks the cloud account and acknowledges its unknown outcome. + Requires execute permission on purchases, execution account access and + matching permission constraints. The row remains failed and counts as + possibly bought; settlement permanently prevents retry and can unblock + deletion of its managed plan. A failed execution proven never + submitted can be settled too, to stop it being retried. A retry chain + that may still buy is refused. Repeating an identical settlement returns the stored result. + parameters: + - $ref: '#/components/parameters/CSRFToken' + requestBody: + required: true + content: + application/json: + schema: + type: object + additionalProperties: false + required: [reason, acknowledge_unknown_outcome] + properties: + reason: + type: string + minLength: 10 + maxLength: 300 + description: > + Trimmed explanation without control characters, at most + 1000 UTF-8 bytes. The entire request body is limited to 2048 bytes. + acknowledge_unknown_outcome: + type: boolean + enum: [true] + responses: + '200': + description: Settlement recorded or an identical settlement replayed + content: + application/json: + schema: + type: object + required: [execution_id, status, outcome_settled_at, outcome_settled_by, outcome_settlement, replayed] + properties: + execution_id: + type: string + format: uuid + status: + type: string + enum: [failed] + outcome_settled_at: + type: string + format: date-time + outcome_settled_by: + type: string + outcome_settlement: + type: string + replayed: + type: boolean + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + description: Execution cannot be settled, or an earlier settlement differs + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '500': + $ref: '#/components/responses/InternalError' + '503': + description: >- + Settlement is unavailable: its storage is down, or the database + migration that adds the settlement columns is not applied yet. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /api/purchases/{id}/revoke: parameters: - name: id diff --git a/internal/api/router.go b/internal/api/router.go index f5c2782b0..5bb2f961b 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -186,6 +186,10 @@ func (r *Router) registerRoutes() { // AuthUser: router-level gate; per-handler requirePermission is the // real security boundary. {PathPrefix: "/api/purchases/", PathSuffix: "/revoke", Method: "POST", Handler: r.revokePurchaseHandler, Auth: AuthUser}, + // Settle a failed plan execution whose outcome is unknown (issue #999), + // so it stops blocking the delete of its editor-managed plan. The + // handler enforces execute:purchases, account access and constraints. + {PathPrefix: "/api/purchases/", PathSuffix: "/settle", Method: "POST", Handler: r.settleExecutionHandler, Auth: AuthUser}, // GET /api/purchases/{id}/revoke/calculate: returns the Azure refund quote // (amount + currency) for the two-step quote-then-confirm revoke UX // (issue #290 Finding #4). No state mutation; result used to populate @@ -633,6 +637,10 @@ func (r *Router) cancelPurchaseHandler(ctx context.Context, req *events.LambdaFu return r.h.cancelPurchase(ctx, req, params["id"], token) } +func (r *Router) settleExecutionHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.settleExecution(ctx, req, params["id"]) +} + func (r *Router) retryPurchaseHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.retryPurchase(ctx, req, params["id"]) } diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 68a12844f..7195d312d 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1280,11 +1280,19 @@ func (s *PostgresStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, // // Returns (true, nil) when this call won the race and wrote the linkage, // (false, nil) when zero rows were affected because a concurrent retry -// already claimed the row (or it is no longer 'failed'), and (false, err) +// already claimed the row (or it is no longer 'failed', or an operator +// settled it: SettleFailedExecution, issue #999), and (false, err) // on a real DB error. Must be called inside the same tx that inserts the // successor row, after that insert, so the FK on retry_execution_id is // satisfied. func (s *PostgresStore) LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID string) (linked bool, err error) { + // Before the settlement migration no row can be settled (#999). + notSettled := "" + if ready, readyErr := settlementSchemaReady(ctx, tx); readyErr != nil { + return false, readyErr + } else if ready { + notSettled = "AND outcome_settled_at IS NULL" + } q := ` UPDATE purchase_executions SET retry_execution_id = $2, @@ -1292,7 +1300,7 @@ func (s *PostgresStore) LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, WHERE execution_id = $1 AND status = 'failed' AND retry_execution_id IS NULL - ` + ` + notSettled tag, err := tx.Exec(ctx, q, executionID, retryExecutionID) if err != nil { return false, fmt.Errorf("failed to link retry execution: %w", err) diff --git a/internal/config/store_postgres_execution_settle.go b/internal/config/store_postgres_execution_settle.go new file mode 100644 index 000000000..97b83e60b --- /dev/null +++ b/internal/config/store_postgres_execution_settle.go @@ -0,0 +1,262 @@ +package config + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/jackc/pgx/v5" +) + +// Outcomes of an operator settle that the store distinguishes. The handler +// maps them to 404, 400 and 409. +var ( + ErrExecutionSettleNotFound = errors.New("execution not found") + ErrExecutionSettleInvalid = errors.New("invalid execution settle request") + ErrExecutionSettleConflict = errors.New("execution cannot be settled") + // ErrExecutionSettleNotReady: the settlement migration is not applied yet. + ErrExecutionSettleNotReady = errors.New("execution settlement is not available until the database migration is applied") +) + +// executionSettleReasonMax bounds the stored record; the column has no CHECK. +const executionSettleReasonMax = 1000 + +// ExecutionSettleRequest asks to settle a failed plan execution whose outcome +// is unknown. Actor is the acting principal's id. Acknowledge must be true: +// the operator states they checked the cloud account and accept that the row +// will never be retried. +type ExecutionSettleRequest struct { + ExecutionID string + Actor string + Reason string + Acknowledge bool +} + +// ExecutionSettlement is the stored settlement. Replayed is true when an +// identical earlier request had already settled the row and nothing was +// written. +type ExecutionSettlement struct { + ExecutionID string + Status string + SettledAt time.Time + SettledBy string + Settlement string + Replayed bool +} + +// settleTarget is the locked row SettleFailedExecution judges. +type settleTarget struct { + status string + retryID *string + planID *string + settledAt *time.Time + settledBy *string + settlement *string +} + +// SettleFailedExecution records an operator's audited settlement of a failed +// plan execution, so it stops blocking the delete of its editor-managed plan +// (requireManagedDeleteTx). It never changes the status or no_submission, so +// the ramp ledger and the retention trigger count the row exactly as before +// (possibly bought, unless it was never submitted). A settled row can +// no longer be retried (LinkRetryExecutionAtomic), because a retry of a row +// whose plan was deleted would buy with no plan. +// +// Locks, in the order the editor command and DeletePurchasePlan use: the plan +// row FOR SHARE, the execution FOR UPDATE, then its retry-chain tail FOR +// UPDATE. It refuses while the chain may still buy (a tail that is not +// terminal, a failed tail that can still be retried, or a chain longer than +// LadderChainMaxDepth). +func (s *PostgresStore) SettleFailedExecution(ctx context.Context, req ExecutionSettleRequest) (*ExecutionSettlement, error) { + if err := validateSettleRequest(req); err != nil { + return nil, err + } + var res *ExecutionSettlement + err := s.WithTx(ctx, func(tx pgx.Tx) error { + if ready, err := settlementSchemaReady(ctx, tx); err != nil || !ready { + if err != nil { + return err + } + return ErrExecutionSettleNotReady + } + target, err := lockSettleTarget(ctx, tx, req.ExecutionID) + if err != nil { + return err + } + if target.settledAt != nil { + res, err = replaySettlement(req, target) + return err + } + if checkErr := checkSettleTarget(target); checkErr != nil { + return checkErr + } + tail, err := lockSettleChainTail(ctx, tx, req.ExecutionID, target.retryID) + if err != nil { + return err + } + record := settlementRecord(req, tail) + res = &ExecutionSettlement{ExecutionID: req.ExecutionID, Status: target.status, SettledBy: req.Actor, Settlement: record} + return tx.QueryRow(ctx, `UPDATE purchase_executions + SET outcome_settled_at = NOW(), outcome_settled_by = $2, outcome_settlement = $3 + WHERE execution_id = $1 AND outcome_settled_at IS NULL + RETURNING outcome_settled_at`, req.ExecutionID, req.Actor, record).Scan(&res.SettledAt) + }) + if err != nil { + return nil, err + } + return res, nil +} + +func validateSettleRequest(req ExecutionSettleRequest) error { + switch { + case !req.Acknowledge: + return fmt.Errorf("%w: the operator must acknowledge the unknown outcome", ErrExecutionSettleInvalid) + case req.Actor == "": + return fmt.Errorf("%w: settling needs an actor", ErrExecutionSettleInvalid) + case req.Reason == "" || len(req.Reason) > executionSettleReasonMax: + return fmt.Errorf("%w: reason must be 1 to %d bytes", ErrExecutionSettleInvalid, executionSettleReasonMax) + } + return nil +} + +// lockSettleTarget takes the plan lock, then the execution lock, and re-reads +// the row under both. A row whose plan changed in between (deleted, so +// plan_id went NULL) is refused rather than judged against the wrong plan. +func lockSettleTarget(ctx context.Context, tx pgx.Tx, executionID string) (*settleTarget, error) { + var planID *string + err := tx.QueryRow(ctx, `SELECT plan_id::text FROM purchase_executions WHERE execution_id = $1`, executionID).Scan(&planID) + if errors.Is(err, pgx.ErrNoRows) { + return nil, ErrExecutionSettleNotFound + } + if err != nil { + return nil, fmt.Errorf("read execution %s: %w", executionID, err) + } + if planID == nil { + return readDetachedSettleTarget(ctx, tx, executionID) + } + if _, lockErr := tx.Exec(ctx, `SELECT 1 FROM purchase_plans WHERE id = $1 FOR SHARE`, *planID); lockErr != nil { + return nil, fmt.Errorf("lock plan %s: %w", *planID, lockErr) + } + var t settleTarget + err = tx.QueryRow(ctx, `SELECT status, retry_execution_id::text, plan_id::text, + outcome_settled_at, outcome_settled_by, outcome_settlement + FROM purchase_executions WHERE execution_id = $1 FOR UPDATE`, executionID).Scan( + &t.status, &t.retryID, &t.planID, &t.settledAt, &t.settledBy, &t.settlement) + if err != nil { + return nil, fmt.Errorf("lock execution %s: %w", executionID, err) + } + if t.planID == nil || *t.planID != *planID { + return nil, fmt.Errorf("%w: the execution's plan changed while settling", ErrExecutionSettleConflict) + } + return &t, nil +} + +// readDetachedSettleTarget serves a row with no plan. Only an already settled +// one is returned, so an identical repeat request still replays after the plan +// was deleted; anything else cannot be settled. The settlement never changes +// once written, so no lock is needed. +func readDetachedSettleTarget(ctx context.Context, tx pgx.Tx, executionID string) (*settleTarget, error) { + var t settleTarget + err := tx.QueryRow(ctx, `SELECT status, retry_execution_id::text, + outcome_settled_at, outcome_settled_by, outcome_settlement + FROM purchase_executions WHERE execution_id = $1`, executionID).Scan( + &t.status, &t.retryID, &t.settledAt, &t.settledBy, &t.settlement) + if err != nil { + return nil, fmt.Errorf("read execution %s: %w", executionID, err) + } + if t.settledAt == nil { + return nil, fmt.Errorf("%w: only an execution of a purchase plan can be settled", ErrExecutionSettleConflict) + } + return &t, nil +} + +// checkSettleTarget allows only a failed row. One proven never submitted is +// allowed too: retry still accepts it, so settling is how it stops being retried. +func checkSettleTarget(t *settleTarget) error { + if t.status != StatusFailed { + return fmt.Errorf("%w: execution is %s; only a failed execution can be settled", ErrExecutionSettleConflict, t.status) + } + return nil +} + +// settleChainTail is the last execution of the retry chain from the target. +type settleChainTail struct { + id string + status string +} + +// lockSettleChainTail follows retry_execution_id from the target to the tail +// and locks the tail. It refuses while the chain may still buy: a tail outside +// the terminal allowlist, a failed tail that is not itself settled (retry +// accepts even a no_submission tail), or a chain cut short by the depth cap or +// changed between the walk and the lock. +func lockSettleChainTail(ctx context.Context, tx pgx.Tx, executionID string, next *string) (settleChainTail, error) { + tail, err := walkSettleChain(ctx, tx, executionID, next) + if err != nil || tail.id == executionID { + return tail, err + } + var retryID *string + var settled bool + err = tx.QueryRow(ctx, `SELECT status, retry_execution_id::text, outcome_settled_at IS NOT NULL + FROM purchase_executions WHERE execution_id = $1 FOR UPDATE`, tail.id).Scan(&tail.status, &retryID, &settled) + if err != nil { + return tail, fmt.Errorf("lock execution %s: %w", tail.id, err) + } + if retryID != nil { + return tail, fmt.Errorf("%w: the retry chain changed while settling", ErrExecutionSettleConflict) + } + retryable := tail.status == StatusFailed && !settled + if !ladderResolvableNodeStatus(tail.status) || retryable { + return tail, fmt.Errorf("%w: retry %s is %s and may still buy or be retried; let it finish or settle it first", ErrExecutionSettleConflict, tail.id, tail.status) + } + return tail, nil +} + +// walkSettleChain reads the retry chain from the target, unlocked, and returns +// its last node; the target itself when it was never retried. +func walkSettleChain(ctx context.Context, tx pgx.Tx, executionID string, next *string) (settleChainTail, error) { + tail := settleChainTail{id: executionID, status: StatusFailed} + for depth := 0; next != nil; depth++ { + if depth >= LadderChainMaxDepth { + return tail, fmt.Errorf("%w: retry chain is longer than %d executions", ErrExecutionSettleConflict, LadderChainMaxDepth) + } + node, err := readChainNode(ctx, tx, *next, false) + if err != nil { + return tail, err + } + tail, next = settleChainTail{id: node.ExecutionID, status: node.Status}, node.RetryExecutionID + } + return tail, nil +} + +// settlementRecord is the exact string stored in outcome_settlement. Fields are +// fixed and quoted, so a repeat of the same request compares equal and a +// different one cannot. The "settled_unknown_outcome" tag also covers a +// never-submitted row; it stays fixed so replays keep comparing equal. +func settlementRecord(req ExecutionSettleRequest, tail settleChainTail) string { + return fmt.Sprintf("operator:settled_unknown_outcome: ack=true chain_tail=%s tail_status=%s reason=%q", + tail.id, tail.status, req.Reason) +} + +// replaySettlement answers a request for a row that is already settled: the +// stored settlement when this actor sent the same reason, else a conflict. The +// conflict goes back to the caller, who may not be the earlier settler, so it +// names only the time; the earlier settler is logged server-side. +func replaySettlement(req ExecutionSettleRequest, t *settleTarget) (*ExecutionSettlement, error) { + if t.settledBy == nil || t.settlement == nil { + return nil, fmt.Errorf("execution %s has an incomplete settlement", req.ExecutionID) + } + if *t.settledBy != req.Actor || !settlementHasReason(*t.settlement, req.Reason) { + at := t.settledAt.UTC().Format(time.RFC3339) + logging.Infof("execution settle refused: execution %s was already settled by %s at %s", req.ExecutionID, *t.settledBy, at) + return nil, fmt.Errorf("%w: already settled at %s", ErrExecutionSettleConflict, at) + } + return &ExecutionSettlement{ExecutionID: req.ExecutionID, Status: t.status, SettledAt: *t.settledAt, SettledBy: *t.settledBy, Settlement: *t.settlement, Replayed: true}, nil +} + +func settlementHasReason(record, reason string) bool { + suffix := fmt.Sprintf(" reason=%q", reason) + return len(record) >= len(suffix) && record[len(record)-len(suffix):] == suffix +} diff --git a/internal/config/store_postgres_execution_settle_integration_test.go b/internal/config/store_postgres_execution_settle_integration_test.go new file mode 100644 index 000000000..c03101d1d --- /dev/null +++ b/internal/config/store_postgres_execution_settle_integration_test.go @@ -0,0 +1,324 @@ +//go:build integration +// +build integration + +package config + +import ( + "context" + "os" + "regexp" + "strconv" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/require" +) + +const settleActor = "00000000-0000-0000-0000-0000000000a9" + +// managedSettleFixture is a managed 4-step ramp plan with one account. +func managedSettleFixture(ctx context.Context, t *testing.T) *rampFixture { + t.Helper() + f := newRampFixture(ctx, t, 1, 1, 4) + _, err := f.store.db.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID) + require.NoError(t, err) + return f +} + +func (f *rampFixture) settle(ctx context.Context, execID, reason string) (*ExecutionSettlement, error) { + return f.store.SettleFailedExecution(ctx, ExecutionSettleRequest{ExecutionID: execID, Actor: settleActor, Reason: reason, Acknowledge: true}) +} + +func (f *rampFixture) settled(ctx context.Context, t *testing.T, execID string) bool { + t.Helper() + var settled bool + require.NoError(t, f.store.db.QueryRow(ctx, `SELECT outcome_settled_at IS NOT NULL FROM purchase_executions WHERE execution_id = $1`, execID).Scan(&settled)) + return settled +} + +// A failed row with an unknown outcome blocks the managed delete until an +// operator settles it; then the delete detaches it without changing it, and the +// ledger still counts it as possibly bought (issue #999). +func TestSettleFailedExecutionLetsManagedDeleteProceed(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + rec := baselineRec(f.accounts[0], "m5.large", 4) + failed := f.rowWithRecs(ctx, t, 1, &f.accounts[0], StatusFailed, false, rec) + retry := f.retryInOneTx(ctx, t, failed, uuid.NewString(), StatusCompleted) + retry.Recommendations = []RecommendationRecord{rec} + require.NoError(t, f.store.SavePurchaseExecution(ctx, retry)) + ledger := f.committed(ctx, t) + + require.ErrorIs(t, f.store.DeletePurchasePlan(ctx, f.planID), ErrManagedPlanLiveExecutions) + + res, err := f.settle(ctx, failed.ExecutionID, "checked the account: retry bought it") + require.NoError(t, err) + require.False(t, res.Replayed) + require.Equal(t, settleActor, res.SettledBy) + require.Contains(t, res.Settlement, "chain_tail="+retry.ExecutionID+" tail_status=completed") + require.Equal(t, ledger, f.committed(ctx, t), "settling must not change what the ledger counts") + + require.NoError(t, f.store.DeletePurchasePlan(ctx, f.planID)) + got, err := f.store.GetExecutionByID(ctx, failed.ExecutionID) + require.NoError(t, err) + require.Equal(t, StatusFailed, got.Status, "the settled row keeps its failed status") + require.False(t, got.NoSubmission, "settling is not a no_submission proof") + require.Empty(t, got.PlanID) + require.True(t, f.settled(ctx, t, failed.ExecutionID), "the audit record survives the delete") +} + +func TestSettleFailedExecutionRefusals(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + acct := &f.accounts[0] + conflict := map[string]string{ + "completed": f.row(ctx, t, 1, acct, StatusCompleted).ExecutionID, + "pending": f.row(ctx, t, 1, acct, "pending").ExecutionID, + "canceled": f.row(ctx, t, 1, acct, StatusCanceled).ExecutionID, + } + for _, tail := range []string{"pending", "running", StatusFailed} { + head := f.row(ctx, t, 2, acct, StatusFailed) + f.retryInOneTx(ctx, t, head, uuid.NewString(), tail) + conflict["retry tail "+tail] = head.ExecutionID + } + adHoc := &PurchaseExecution{ExecutionID: uuid.NewString(), Status: StatusFailed, StepNumber: 1} + require.NoError(t, f.store.SavePurchaseExecution(ctx, adHoc)) + conflict["no plan"] = adHoc.ExecutionID + + for name, id := range conflict { + _, err := f.settle(ctx, id, "operator checked the account") + require.ErrorIs(t, err, ErrExecutionSettleConflict, name) + require.False(t, f.settled(ctx, t, id), name) + } + _, err := f.settle(ctx, uuid.NewString(), "operator checked the account") + require.ErrorIs(t, err, ErrExecutionSettleNotFound) + + failed := f.row(ctx, t, 3, acct, StatusFailed) + for name, req := range map[string]ExecutionSettleRequest{ + "no ack": {ExecutionID: failed.ExecutionID, Actor: settleActor, Reason: "checked"}, + "no actor": {ExecutionID: failed.ExecutionID, Reason: "checked", Acknowledge: true}, + "no reason": {ExecutionID: failed.ExecutionID, Actor: settleActor, Acknowledge: true}, + } { + _, err := f.store.SettleFailedExecution(ctx, req) + require.ErrorIs(t, err, ErrExecutionSettleInvalid, name) + } + require.False(t, f.settled(ctx, t, failed.ExecutionID)) +} + +// A failed tail blocks its predecessor's settle only while it can still be +// retried: once the tail is settled, the head can be too. +func TestSettleFailedExecutionChainOfUnknownOutcomes(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + head := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + tail := f.retryInOneTx(ctx, t, head, uuid.NewString(), StatusFailed) + + _, err := f.settle(ctx, head.ExecutionID, "operator checked the account") + require.ErrorIs(t, err, ErrExecutionSettleConflict) + _, err = f.settle(ctx, tail.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.ErrorIs(t, f.store.DeletePurchasePlan(ctx, f.planID), ErrManagedPlanLiveExecutions, "the unsettled head still blocks") + _, err = f.settle(ctx, head.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.NoError(t, f.store.DeletePurchasePlan(ctx, f.planID)) +} + +func TestSettleFailedExecutionReplayAndRetry(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + failed := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + first, err := f.settle(ctx, failed.ExecutionID, "operator checked the account") + require.NoError(t, err) + + again, err := f.settle(ctx, failed.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.True(t, again.Replayed) + require.True(t, first.SettledAt.Equal(again.SettledAt), "a replay writes nothing") + _, err = f.settle(ctx, failed.ExecutionID, "a different reason entirely") + require.ErrorIs(t, err, ErrExecutionSettleConflict) + + successor := &PurchaseExecution{ExecutionID: uuid.NewString(), PlanID: f.planID, Status: "pending", StepNumber: 1} + require.NoError(t, f.store.WithTx(ctx, func(tx pgx.Tx) error { + require.NoError(t, f.store.SavePurchaseExecutionTx(ctx, tx, successor)) + linked, linkErr := f.store.LinkRetryExecutionAtomic(ctx, tx, failed.ExecutionID, successor.ExecutionID) + require.NoError(t, linkErr) + require.False(t, linked, "a settled row must not be retried") + return nil + })) +} + +// Once the plan is deleted the settled row is detached, yet an identical +// repeat request still replays the stored result; a different one conflicts. +func TestSettleReplayAfterPlanDelete(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + failed := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + first, err := f.settle(ctx, failed.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.NoError(t, f.store.DeletePurchasePlan(ctx, f.planID)) + + again, err := f.settle(ctx, failed.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.True(t, again.Replayed) + require.True(t, first.SettledAt.Equal(again.SettledAt)) + _, err = f.settle(ctx, failed.ExecutionID, "a different reason entirely") + require.ErrorIs(t, err, ErrExecutionSettleConflict) +} + +// A never-submitted failed row can still be retried, so it must be settleable: +// otherwise a head whose tail is such a row could never be settled. +func TestSettleFailedNoSubmissionChain(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + head := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + tail := f.retryInOneTx(ctx, t, head, uuid.NewString(), StatusFailed) + _, err := f.store.db.Exec(ctx, `UPDATE purchase_executions SET no_submission = true WHERE execution_id = $1`, tail.ExecutionID) + require.NoError(t, err) + ledger := f.committed(ctx, t) + + _, err = f.settle(ctx, head.ExecutionID, "operator checked the account") + require.ErrorIs(t, err, ErrExecutionSettleConflict, "the unsettled tail may still be retried") + require.False(t, f.settled(ctx, t, head.ExecutionID)) + _, err = f.settle(ctx, tail.ExecutionID, "operator checked the account") + require.NoError(t, err) + _, err = f.settle(ctx, head.ExecutionID, "operator checked the account") + require.NoError(t, err) + require.Equal(t, ledger, f.committed(ctx, t), "settling must not change what the ledger counts") + + require.NoError(t, f.store.DeletePurchasePlan(ctx, f.planID)) + successor := &PurchaseExecution{ExecutionID: uuid.NewString(), Status: "pending", StepNumber: 1} + require.NoError(t, f.store.WithTx(ctx, func(tx pgx.Tx) error { + require.NoError(t, f.store.SavePurchaseExecutionTx(ctx, tx, successor)) + linked, linkErr := f.store.LinkRetryExecutionAtomic(ctx, tx, tail.ExecutionID, successor.ExecutionID) + require.NoError(t, linkErr) + require.False(t, linked, "a settled tail must not be retried") + return nil + })) +} + +// A never-submitted row on its own is settleable and stays never-submitted. +func TestSettleFailedNoSubmissionRow(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + row := f.rowWithRecs(ctx, t, 1, &f.accounts[0], StatusFailed, true) + _, err := f.settle(ctx, row.ExecutionID, "operator checked the account") + require.NoError(t, err) + got, err := f.store.GetExecutionByID(ctx, row.ExecutionID) + require.NoError(t, err) + require.True(t, got.NoSubmission) +} + +// A retry that links the row first wins: the settle waits on the row lock, +// then sees the live successor and refuses. +func TestSettleWaitsForRetryAndRefusesItsLiveSuccessor(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + failed := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + retryTx, err := f.store.db.Begin(ctx) + require.NoError(t, err) + t.Cleanup(func() { _ = retryTx.Rollback(ctx) }) + successor := &PurchaseExecution{ExecutionID: uuid.NewString(), PlanID: f.planID, Status: "pending", StepNumber: 1} + require.NoError(t, f.store.SavePurchaseExecutionTx(ctx, retryTx, successor)) + linked, err := f.store.LinkRetryExecutionAtomic(ctx, retryTx, failed.ExecutionID, successor.ExecutionID) + require.NoError(t, err) + require.True(t, linked) + + done := make(chan error, 1) + go func() { + _, settleErr := f.settle(ctx, failed.ExecutionID, "operator checked the account") + done <- settleErr + }() + testhelpers.WaitForLockWait(ctx, t, f.store.db, "%FOR UPDATE%") + require.NoError(t, retryTx.Commit(ctx)) + require.ErrorIs(t, testhelpers.Await(t, done), ErrExecutionSettleConflict) + require.False(t, f.settled(ctx, t, failed.ExecutionID)) +} + +// A delete that holds the plan lock first wins: the settle waits, then finds +// the row detached and refuses instead of settling it against no plan. +func TestSettleWaitsForDeleteAndRefusesDetachedRow(t *testing.T) { + ctx := context.Background() + f := newRampFixture(ctx, t, 1, 1, 4) + failed := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + del := testhelpers.HoldCommandFootprint(ctx, t, f.store.db, f.planID) + + done := make(chan error, 1) + go func() { + _, settleErr := f.settle(ctx, failed.ExecutionID, "operator checked the account") + done <- settleErr + }() + testhelpers.WaitForLockWait(ctx, t, f.store.db, "%FOR SHARE%") + _, err := del.Exec(ctx, `DELETE FROM purchase_plans WHERE id = $1`, f.planID) + require.NoError(t, err) + require.NoError(t, del.Commit(ctx)) + require.ErrorIs(t, testhelpers.Await(t, done), ErrExecutionSettleConflict) + require.False(t, f.settled(ctx, t, failed.ExecutionID)) +} + +// A delete that queues behind a settle sees the committed settlement. +func TestDeleteWaitsForSettleAndSeesIt(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + failed := f.row(ctx, t, 1, &f.accounts[0], StatusFailed) + settleTx, err := f.store.db.Begin(ctx) + require.NoError(t, err) + t.Cleanup(func() { _ = settleTx.Rollback(ctx) }) + // The settle's own lock footprint and write, paused before commit. + _, err = settleTx.Exec(ctx, `SELECT 1 FROM purchase_plans WHERE id = $1 FOR SHARE`, f.planID) + require.NoError(t, err) + _, err = settleTx.Exec(ctx, `UPDATE purchase_executions SET outcome_settled_at = now(), + outcome_settled_by = $2, outcome_settlement = 'operator:settled_unknown_outcome' WHERE execution_id = $1`, failed.ExecutionID, settleActor) + require.NoError(t, err) + + done := make(chan error, 1) + go func() { done <- f.store.DeletePurchasePlan(ctx, f.planID) }() + testhelpers.WaitForLockWait(ctx, t, f.store.db, "%FOR UPDATE%") + require.NoError(t, settleTx.Commit(ctx)) + require.NoError(t, testhelpers.Await(t, done)) +} + +// migrationBelow is the highest migration on disk below version, so a test can +// roll back exactly one migration whatever gaps the numbering has. +func migrationBelow(t *testing.T, version int) uint { + t.Helper() + entries, err := os.ReadDir(getTestMigrationsPath()) + require.NoError(t, err) + re := regexp.MustCompile(`^(\d{6})_.*\.up\.sql$`) + best := 0 + for _, e := range entries { + if m := re.FindStringSubmatch(e.Name()); m != nil { + if n, _ := strconv.Atoi(m[1]); n < version && n > best { + best = n + } + } + } + require.NotZero(t, best) + return uint(best) +} + +// main deploys on merge while prod migrations are applied separately, so this +// code must work on a schema without the settlement columns (#999): delete +// and retry behave as before, and settling is refused instead of erroring. +func TestSettlementPathsBeforeTheMigration(t *testing.T) { + ctx := context.Background() + f := managedSettleFixture(ctx, t) + rec := baselineRec(f.accounts[0], "m5.large", 4) + failed := f.rowWithRecs(ctx, t, 1, &f.accounts[0], StatusFailed, false, rec) + + conn, ok := f.store.db.(*database.Connection) + require.True(t, ok) + require.NoError(t, migrations.MigrateToVersion(ctx, conn.Pool(), getTestMigrationsPath(), migrationBelow(t, settlementMigrationVersion))) + var cols int + require.NoError(t, conn.QueryRow(ctx, `SELECT count(*) FROM information_schema.columns WHERE table_name = 'purchase_executions' AND column_name = 'outcome_settled_at'`).Scan(&cols)) + require.Zero(t, cols, "precondition: the settlement columns are gone") + + require.ErrorIs(t, f.store.DeletePurchasePlan(ctx, f.planID), ErrManagedPlanLiveExecutions, + "the delete guard works and still refuses, rather than failing on a missing column") + _, err := f.settle(ctx, failed.ExecutionID, "checked the account") + require.ErrorIs(t, err, ErrExecutionSettleNotReady) + f.retryInOneTx(ctx, t, failed, uuid.NewString(), StatusCompleted) // the retry link still works +} diff --git a/internal/config/store_postgres_execution_settle_test.go b/internal/config/store_postgres_execution_settle_test.go new file mode 100644 index 000000000..b6833c5da --- /dev/null +++ b/internal/config/store_postgres_execution_settle_test.go @@ -0,0 +1,60 @@ +package config + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestBlocksManagedDelete(t *testing.T) { + cases := []struct { + status string + noSubmission bool + settled bool + blocks bool + }{ + {StatusFailed, false, false, true}, + {StatusFailed, false, true, false}, + {StatusFailed, true, false, false}, + {"pending", false, true, true}, + {"running", false, true, true}, + {StatusCompleted, false, false, false}, + {StatusPartiallyCompleted, false, false, false}, + {StatusCanceled, false, false, false}, + {StatusExpired, false, false, false}, + {"some_future_status", false, true, true}, + } + for _, tc := range cases { + e := &PurchaseExecution{Status: tc.status, NoSubmission: tc.noSubmission} + require.Equal(t, tc.blocks, blocksManagedDelete(e, tc.settled), "%+v", tc) + } +} + +func TestSettlementRecordReplayMatchesOnlyTheSameReason(t *testing.T) { + req := ExecutionSettleRequest{Reason: `checked "eu" account`} + record := settlementRecord(req, settleChainTail{id: "tail", status: StatusCompleted}) + require.Equal(t, `operator:settled_unknown_outcome: ack=true chain_tail=tail tail_status=completed reason="checked \"eu\" account"`, record) + require.True(t, settlementHasReason(record, req.Reason)) + require.False(t, settlementHasReason(record, "account")) + require.False(t, settlementHasReason(record, `checked "eu" account2`)) +} + +// The conflict text is the 409 body, and the caller may not be the earlier +// settler, so it must not carry that settler's principal ID. +func TestReplaySettlementConflictOmitsTheEarlierSettler(t *testing.T) { + at := time.Date(2026, 11, 4, 12, 0, 0, 0, time.UTC) + by := "settler-principal-7f3a" + record := settlementRecord(ExecutionSettleRequest{Reason: "checked the account"}, settleChainTail{id: "e1", status: "completed"}) + target := &settleTarget{status: "failed", settledAt: &at, settledBy: &by, settlement: &record} + for name, req := range map[string]ExecutionSettleRequest{ + "another actor": {ExecutionID: "e1", Actor: "someone-else", Reason: "checked the account"}, + "another reason": {ExecutionID: "e1", Actor: by, Reason: "a different reason"}, + } { + _, err := replaySettlement(req, target) + require.ErrorIs(t, err, ErrExecutionSettleConflict, name) + require.False(t, strings.Contains(err.Error(), by), "%s: %q names the earlier settler", name, err.Error()) + require.Contains(t, err.Error(), "2026-11-04T12:00:00Z", name) + } +} diff --git a/internal/config/store_postgres_managed_guard.go b/internal/config/store_postgres_managed_guard.go index c7d4a523b..b90b9ff72 100644 --- a/internal/config/store_postgres_managed_guard.go +++ b/internal/config/store_postgres_managed_guard.go @@ -132,21 +132,44 @@ func sameAccountSet(a, b []string) bool { // hold or buy capacity: the FK would detach it (ON DELETE SET NULL) and it // would then buy with no plan. Judged under the caller's plan lock, so a root // inserted by a command that committed first is seen. Settled history -// (completed, canceled, expired, failed without submission) does not block. +// (completed, canceled, expired, failed without submission) does not block, +// and neither does a failed row with an unknown outcome that an operator +// settled (SettleFailedExecution): it can no longer be retried and keeps its +// failed status, so detaching it hides nothing. func requireManagedDeleteTx(ctx context.Context, tx pgx.Tx, planID string) error { - rows, err := tx.Query(ctx, `SELECT status, no_submission FROM purchase_executions WHERE plan_id = $1`, planID) + // Before the settlement migration no row can be settled (#999). + settledCol := "false" + if ready, err := settlementSchemaReady(ctx, tx); err != nil { + return err + } else if ready { + settledCol = "outcome_settled_at IS NOT NULL" + } + rows, err := tx.Query(ctx, `SELECT status, no_submission, `+settledCol+` FROM purchase_executions WHERE plan_id = $1`, planID) if err != nil { return fmt.Errorf("failed to read plan executions: %w", err) } defer rows.Close() for rows.Next() { var e PurchaseExecution - if err := rows.Scan(&e.Status, &e.NoSubmission); err != nil { + var settled bool + if err := rows.Scan(&e.Status, &e.NoSubmission, &settled); err != nil { return fmt.Errorf("failed to scan plan execution: %w", err) } - if holdsCapacity(&e) && e.Status != StatusCompleted && e.Status != StatusPartiallyCompleted { + if blocksManagedDelete(&e, settled) { return fmt.Errorf("%w: plan %s", ErrManagedPlanLiveExecutions, planID) } } return rows.Err() } + +// blocksManagedDelete reports whether a row of a managed plan may still hold +// or buy capacity that no operator has accounted for. +func blocksManagedDelete(e *PurchaseExecution, settled bool) bool { + switch { + case !holdsCapacity(e), e.Status == StatusCompleted, e.Status == StatusPartiallyCompleted: + return false + case e.Status == StatusFailed: + return !settled + } + return true +} diff --git a/internal/config/store_postgres_recommendations.go b/internal/config/store_postgres_recommendations.go index cfc297312..4d78be030 100644 --- a/internal/config/store_postgres_recommendations.go +++ b/internal/config/store_postgres_recommendations.go @@ -3,7 +3,6 @@ package config import ( "context" "encoding/json" - "errors" "fmt" "math" "strings" @@ -407,16 +406,11 @@ func (s *PostgresStore) checkAzurePricingMigration(ctx context.Context, required if !required { return nil } - var version int - var dirty bool - err := s.db.QueryRow(ctx, `SELECT version, dirty FROM schema_migrations`).Scan(&version, &dirty) - if errors.Is(err, pgx.ErrNoRows) { - return ErrAzurePricingNotReady - } + version, dirty, found, err := readSchemaVersion(ctx, s.db) if err != nil { return fmt.Errorf("check Azure pricing migration: %w", err) } - if version < azurePricingMigrationVersion || dirty { + if !found || version < azurePricingMigrationVersion || dirty { return ErrAzurePricingNotReady } return nil diff --git a/internal/config/store_postgres_schema_version.go b/internal/config/store_postgres_schema_version.go new file mode 100644 index 000000000..731feba5d --- /dev/null +++ b/internal/config/store_postgres_schema_version.go @@ -0,0 +1,47 @@ +package config + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" +) + +// settlementMigrationVersion is the migration that adds the outcome-settlement +// columns to purchase_executions (issue #999). +const settlementMigrationVersion = 155 + +// rowQuerier is satisfied by both the store's connection and a pgx.Tx, so a +// schema check can run inside the caller's transaction. +type rowQuerier interface { + QueryRow(ctx context.Context, sql string, args ...any) pgx.Row +} + +// readSchemaVersion returns the applied migration version and its dirty flag. +// found is false when schema_migrations has no row (nothing applied yet). +func readSchemaVersion(ctx context.Context, q rowQuerier) (version int, dirty, found bool, err error) { + err = q.QueryRow(ctx, `SELECT version, dirty FROM schema_migrations`).Scan(&version, &dirty) + if errors.Is(err, pgx.ErrNoRows) { + return 0, false, false, nil + } + if err != nil { + return 0, false, false, fmt.Errorf("read schema version: %w", err) + } + return version, dirty, true, nil +} + +// settlementSchemaReady reports whether the settlement columns exist. main +// deploys on merge while prod migrations are applied separately, so the code +// must run against a schema without them: the retry link and the managed +// delete guard fall back to their pre-settlement queries, and settling is +// refused (ErrExecutionSettleNotReady). A later version means the columns +// exist even if that later migration is dirty; at exactly this version a +// dirty flag means it may not have finished. +func settlementSchemaReady(ctx context.Context, q rowQuerier) (bool, error) { + version, dirty, found, err := readSchemaVersion(ctx, q) + if err != nil || !found { + return false, err + } + return version > settlementMigrationVersion || (version == settlementMigrationVersion && !dirty), nil +} diff --git a/internal/config/store_postgres_schema_version_test.go b/internal/config/store_postgres_schema_version_test.go new file mode 100644 index 000000000..dc51d7474 --- /dev/null +++ b/internal/config/store_postgres_schema_version_test.go @@ -0,0 +1,55 @@ +package config + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeVersionRow struct { + version int + dirty bool + err error +} + +func (r fakeVersionRow) Scan(dest ...any) error { + if r.err != nil { + return r.err + } + *dest[0].(*int) = r.version + *dest[1].(*bool) = r.dirty + return nil +} + +type fakeVersionQuerier struct{ row fakeVersionRow } + +func (q fakeVersionQuerier) QueryRow(context.Context, string, ...any) pgx.Row { return q.row } + +// main deploys before prod migrations are applied, so the settlement paths +// must know whether the columns exist (#999). +func TestSettlementSchemaReady(t *testing.T) { + t.Parallel() + v := settlementMigrationVersion + cases := []struct { + name string + row fakeVersionRow + want bool + }{ + {"nothing applied", fakeVersionRow{err: pgx.ErrNoRows}, false}, + {"before the migration", fakeVersionRow{version: v - 1}, false}, + {"at the migration", fakeVersionRow{version: v}, true}, + {"at the migration, dirty", fakeVersionRow{version: v, dirty: true}, false}, + {"after it, even with a later dirty migration", fakeVersionRow{version: v + 1, dirty: true}, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := settlementSchemaReady(context.Background(), fakeVersionQuerier{tc.row}) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.down.sql b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.down.sql new file mode 100644 index 000000000..aec64696d --- /dev/null +++ b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.down.sql @@ -0,0 +1,20 @@ +-- Reverse 000155 (issue #999). Refuses while any settlement exists: dropping +-- the columns would let a settled row be retried and block its plan's delete +-- again. The lock and the check share one DO block, so no settlement can land +-- between them. +DO $$ +BEGIN + LOCK TABLE purchase_executions IN ACCESS EXCLUSIVE MODE; + IF EXISTS (SELECT 1 FROM purchase_executions WHERE outcome_settled_at IS NOT NULL) THEN + RAISE EXCEPTION 'cannot reverse 000155: purchase_executions has operator settlements'; + END IF; + DROP TRIGGER IF EXISTS purchase_executions_refuse_retry_of_settled ON purchase_executions; + DROP FUNCTION IF EXISTS purchase_executions_refuse_retry_of_settled(); + ALTER TABLE purchase_executions + DROP CONSTRAINT IF EXISTS purchase_executions_outcome_settlement_complete; + ALTER TABLE purchase_executions + DROP COLUMN IF EXISTS outcome_settlement, + DROP COLUMN IF EXISTS outcome_settled_by, + DROP COLUMN IF EXISTS outcome_settled_at; +END +$$; diff --git a/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.up.sql b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.up.sql new file mode 100644 index 000000000..4fe592bf7 --- /dev/null +++ b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement.up.sql @@ -0,0 +1,44 @@ +-- Migration 000155 (issue #999): an operator's audited settlement of a failed +-- execution whose outcome is unknown (failed without no_submission proof). +-- +-- Settling never changes status and never makes the row count as unbought: +-- the ramp ledger (holdsCapacity, ramp_ledger_holds) and the retention trigger +-- still treat it as possibly bought. It only lets a managed plan's delete +-- detach the row, and it stops the row from being retried (a retry of a row +-- detached from its plan would buy with no plan). +-- +-- outcome_settled_by is the acting principal as text (a users.id, or the admin +-- API key sentinel), so the audit survives the user being deleted. +-- outcome_settlement is the complete, fixed-format record the store wrote. +ALTER TABLE purchase_executions + ADD COLUMN IF NOT EXISTS outcome_settled_at TIMESTAMPTZ NULL, + ADD COLUMN IF NOT EXISTS outcome_settled_by TEXT NULL, + ADD COLUMN IF NOT EXISTS outcome_settlement TEXT NULL; + +ALTER TABLE purchase_executions + ADD CONSTRAINT purchase_executions_outcome_settlement_complete + CHECK ((outcome_settled_at IS NULL) = (outcome_settled_by IS NULL) + AND (outcome_settled_at IS NULL) = (outcome_settlement IS NULL)) + NOT VALID; +-- NOT VALID still checks every new write and skips the table scan under the +-- ACCESS EXCLUSIVE lock; 000156 validates it. + +-- A settled row is never retried. LinkRetryExecutionAtomic enforces it, and +-- this trigger holds it for any other writer, such as a binary from before +-- this migration during a rollout or rollback. +CREATE OR REPLACE FUNCTION purchase_executions_refuse_retry_of_settled() +RETURNS TRIGGER AS $$ +BEGIN + IF OLD.retry_execution_id IS NULL AND NEW.retry_execution_id IS NOT NULL + AND (OLD.outcome_settled_at IS NOT NULL OR NEW.outcome_settled_at IS NOT NULL) THEN + RAISE EXCEPTION 'execution % is settled and cannot be retried', OLD.execution_id + USING ERRCODE = 'check_violation'; + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS purchase_executions_refuse_retry_of_settled ON purchase_executions; +CREATE TRIGGER purchase_executions_refuse_retry_of_settled + BEFORE UPDATE OF retry_execution_id ON purchase_executions + FOR EACH ROW EXECUTE FUNCTION purchase_executions_refuse_retry_of_settled(); diff --git a/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement_test.go b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement_test.go new file mode 100644 index 000000000..d9ee4bc12 --- /dev/null +++ b/internal/database/postgres/migrations/000155_purchase_executions_outcome_settlement_test.go @@ -0,0 +1,134 @@ +//go:build integration + +package migrations_test + +import ( + "context" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/stretchr/testify/require" +) + +// TestMigration_PurchaseExecutionsOutcomeSettlement covers 000155 and its +// VALIDATE follow-up 000156: up, down and the all-or-nothing CHECK (#999). +func TestMigration_PurchaseExecutionsOutcomeSettlement(t *testing.T) { + ctx := context.Background() + container := testhelpers.RequirePostgresContainer(ctx, t) + t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + pool, path := container.DB.Pool(), getMigrationsPath() + const version, validateVersion = 155, 156 + prev := previousMigrationVersion(t, version) + validated := func() bool { + var ok bool + require.NoError(t, pool.QueryRow(ctx, `SELECT convalidated FROM pg_constraint + WHERE conname = 'purchase_executions_outcome_settlement_complete'`).Scan(&ok)) + return ok + } + + columns := func() int { + var n int + require.NoError(t, pool.QueryRow(ctx, ` + SELECT count(*) FROM information_schema.columns + WHERE table_name = 'purchase_executions' + AND column_name IN ('outcome_settled_at', 'outcome_settled_by', 'outcome_settlement')`).Scan(&n)) + return n + } + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, prev)) + require.Zero(t, columns(), "before 000155 the columns must not exist") + _, err := pool.Exec(ctx, ` + INSERT INTO purchase_executions (execution_id, status, step_number, scheduled_date, recommendations) + VALUES ('00000000-0000-0000-0000-0000000009a1', 'failed', 1, now(), '[]')`) + require.NoError(t, err) + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, version)) + require.Equal(t, 3, columns()) + require.False(t, validated(), "000155 adds the CHECK NOT VALID to avoid a table scan under lock") + var settled bool + require.NoError(t, pool.QueryRow(ctx, `SELECT outcome_settled_at IS NOT NULL FROM purchase_executions + WHERE execution_id = '00000000-0000-0000-0000-0000000009a1'`).Scan(&settled)) + require.False(t, settled, "existing rows start unsettled") + + _, err = pool.Exec(ctx, `UPDATE purchase_executions + SET outcome_settled_at = now(), outcome_settled_by = 'admin-api-key', outcome_settlement = 'operator:settled' + WHERE execution_id = '00000000-0000-0000-0000-0000000009a1'`) + require.NoError(t, err, "a complete settlement is valid") + for name, set := range map[string]string{ + "time only": `outcome_settled_at = now()`, + "no record": `outcome_settled_at = now(), outcome_settled_by = 'u'`, + "no actor": `outcome_settled_at = now(), outcome_settlement = 's'`, + "record only": `outcome_settlement = 's'`, + } { + _, err = pool.Exec(ctx, `INSERT INTO purchase_executions (execution_id, status, step_number, scheduled_date, recommendations) + VALUES (gen_random_uuid(), 'failed', 1, now(), '[]')`) + require.NoError(t, err) + _, err = pool.Exec(ctx, `UPDATE purchase_executions SET `+set+` WHERE outcome_settled_at IS NULL AND outcome_settlement IS NULL`) + require.Error(t, err, "CHECK must reject a partial settlement (%s)", name) + } + + _, err = pool.Exec(ctx, `UPDATE purchase_executions SET outcome_settled_at = NULL, outcome_settled_by = NULL, + outcome_settlement = NULL`) + require.NoError(t, err) + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, validateVersion)) + require.True(t, validated(), "000156 validates the CHECK") + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, prev)) + require.Zero(t, columns(), "down must drop the columns") + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, validateVersion)) + require.Equal(t, 3, columns(), "up must re-apply cleanly after down") +} + +// The database itself refuses to link a retry to a settled row, so a binary +// from before the settlement code cannot undo "a settled row is never retried". +func TestMigration_OutcomeSettlementRefusesRetryLink(t *testing.T) { + ctx := context.Background() + container := testhelpers.RequirePostgresContainer(ctx, t) + t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + pool := container.DB.Pool() + require.NoError(t, migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), 155)) + + insert := func(id, settled string) { + _, err := pool.Exec(ctx, `INSERT INTO purchase_executions (execution_id, status, step_number, scheduled_date, recommendations, + outcome_settled_at, outcome_settled_by, outcome_settlement) + VALUES ($1, 'failed', 1, now(), '[]', `+settled+`)`, id) + require.NoError(t, err) + } + const settledID, openID, succ1, succ2 = "00000000-0000-0000-0000-0000000009b1", "00000000-0000-0000-0000-0000000009b2", + "00000000-0000-0000-0000-0000000009b3", "00000000-0000-0000-0000-0000000009b4" + insert(settledID, `now(), 'u', 's'`) + insert(openID, `NULL, NULL, NULL`) + insert(succ1, `NULL, NULL, NULL`) + insert(succ2, `NULL, NULL, NULL`) + + link := `UPDATE purchase_executions SET retry_execution_id = $2 WHERE execution_id = $1` + _, err := pool.Exec(ctx, link, settledID, succ1) + require.Error(t, err, "linking a retry to a settled row must fail in the database") + _, err = pool.Exec(ctx, link, openID, succ2) + require.NoError(t, err, "an unsettled failed row can still be retried") +} + +// Down refuses to drop settlements, which would let a settled row be retried +// and its plan's delete refused again; the refusal leaves the schema intact. +func TestMigration_OutcomeSettlementDownRefusesWithSettlements(t *testing.T) { + ctx := context.Background() + container := testhelpers.RequirePostgresContainer(ctx, t) + t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + pool, path := container.DB.Pool(), getMigrationsPath() + prev := previousMigrationVersion(t, 155) + require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 155)) + _, err := pool.Exec(ctx, `INSERT INTO purchase_executions (execution_id, status, step_number, scheduled_date, recommendations, + outcome_settled_at, outcome_settled_by, outcome_settlement) + VALUES (gen_random_uuid(), 'failed', 1, now(), '[]', now(), 'u', 's')`) + require.NoError(t, err) + + require.Error(t, migrations.MigrateToVersion(ctx, pool, path, prev), "down must refuse while a settlement exists") + var cols, triggers int + require.NoError(t, pool.QueryRow(ctx, `SELECT count(*) FROM information_schema.columns + WHERE table_name = 'purchase_executions' AND column_name LIKE 'outcome_settle%'`).Scan(&cols)) + require.NoError(t, pool.QueryRow(ctx, `SELECT count(*) FROM pg_trigger + WHERE tgname = 'purchase_executions_refuse_retry_of_settled'`).Scan(&triggers)) + require.Equal(t, 3, cols, "a refused down leaves the columns") + require.Equal(t, 1, triggers, "a refused down leaves the trigger") +} diff --git a/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.down.sql b/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.down.sql new file mode 100644 index 000000000..6988bfabc --- /dev/null +++ b/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.down.sql @@ -0,0 +1,9 @@ +-- A validated constraint cannot be un-validated; re-add it NOT VALID, as the +-- state 000155 leaves (same pattern as 000122). +ALTER TABLE purchase_executions + DROP CONSTRAINT purchase_executions_outcome_settlement_complete; +ALTER TABLE purchase_executions + ADD CONSTRAINT purchase_executions_outcome_settlement_complete + CHECK ((outcome_settled_at IS NULL) = (outcome_settled_by IS NULL) + AND (outcome_settled_at IS NULL) = (outcome_settlement IS NULL)) + NOT VALID; diff --git a/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.up.sql b/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.up.sql new file mode 100644 index 000000000..9ff474186 --- /dev/null +++ b/internal/database/postgres/migrations/000156_validate_purchase_executions_outcome_settlement.up.sql @@ -0,0 +1,4 @@ +-- Validate the CHECK that 000155 added NOT VALID (issue #999); same split as +-- 000121/000122. VALIDATE scans without blocking writes. +ALTER TABLE purchase_executions + VALIDATE CONSTRAINT purchase_executions_outcome_settlement_complete; diff --git a/known_issues/35_legacy_guard_followups.md b/known_issues/resolved/35_legacy_guard_followups.md similarity index 75% rename from known_issues/35_legacy_guard_followups.md rename to known_issues/resolved/35_legacy_guard_followups.md index 10fe316ca..7b5b40da3 100644 --- a/known_issues/35_legacy_guard_followups.md +++ b/known_issues/resolved/35_legacy_guard_followups.md @@ -1,6 +1,6 @@ # Known Issues: follow-ups from the legacy-writer guards (#631) -> **Audit status (2026-10-20):** `1 needs triage · 1 resolved` +> **Audit status (2026-10-23):** `0 needs triage · 2 resolved` ## ~~LOW: cleanup lambda deletes by expires_at with no status predicate~~ — RESOLVED @@ -18,7 +18,7 @@ **Resolved by:** `deleteExpired` and `dryRunCount` now share `expiredExecutionsWhere`: a lapsed `expires_at` deletes a row only when its status is in `TerminalExecutionStatuses` and not in `HealthScoredExecutionStatuses`, so live rows survive and failed or canceled rows stay with `CleanupOldExecutions`. Covered by the real-Postgres tests in `cmd/cleanup-lambda/main_integration_test.go`, including a writer that turns a settled row live while the DELETE waits for its lock. -## MEDIUM: a failed-with-submission row blocks deleting a managed plan forever +## ~~MEDIUM: a failed-with-submission row blocks deleting a managed plan forever~~ — RESOLVED **Files:** `internal/config/store_postgres_managed_guard.go` (`requireManagedDeleteTx`), `internal/config/store_postgres_ramp_baseline.go` (`holdsCapacity`) @@ -28,4 +28,6 @@ **Issue:** #999 (follow-up of #631) -**Status:** Needs triage. +**Status:** ✔️ Resolved + +**Resolved by:** an explicit, audited operator settle, `POST /api/purchases/{id}/settle` (`SettleFailedExecution`, the outcome-settlement migration 000155, validated by 000156). It needs `execute:purchases`, account access and the grant's constraints, a reason and `acknowledge_unknown_outcome: true`, and refuses while the row's retry chain may still buy. The row stays `failed` and keeps counting as possibly bought in the ramp ledger; once settled it can no longer be retried and no longer blocks `requireManagedDeleteTx`. Delete itself gained no flag.