From 5860fe1fff3efb72486d4342b6436e61357d4c8c Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 02:00:21 +0100 Subject: [PATCH 1/4] feat(db): record operator settlements on purchase executions (#999) Migration 000143 adds outcome_settled_at, outcome_settled_by and outcome_settlement to purchase_executions, with a CHECK that they are set together. They hold an operator's audited settlement of a failed execution whose outcome is unknown. Settling never changes status or the ramp ledger's view of the row. Co-Authored-By: Claude Opus 5.5 --- ...ase_executions_outcome_settlement.down.sql | 8 +++ ...chase_executions_outcome_settlement.up.sql | 21 ++++++ ...hase_executions_outcome_settlement_test.go | 68 +++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.down.sql create mode 100644 internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.up.sql create mode 100644 internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement_test.go diff --git a/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.down.sql b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.down.sql new file mode 100644 index 000000000..46b11cc0e --- /dev/null +++ b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.down.sql @@ -0,0 +1,8 @@ +-- Reverse 000143 (issue #999). Lossy: operator settlements are dropped, so a +-- settled failed row blocks its managed plan's delete again. +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; diff --git a/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.up.sql b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.up.sql new file mode 100644 index 000000000..55894f5d4 --- /dev/null +++ b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement.up.sql @@ -0,0 +1,21 @@ +-- Migration 000143 (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)); diff --git a/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement_test.go b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement_test.go new file mode 100644 index 000000000..5d78393a9 --- /dev/null +++ b/internal/database/postgres/migrations/000143_purchase_executions_outcome_settlement_test.go @@ -0,0 +1,68 @@ +//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 000143 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 = 143 + prev := previousMigrationVersion(t, version) + + 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 000143 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()) + 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) + } + + 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, version)) + require.Equal(t, 3, columns(), "up must re-apply cleanly after down") +} From d75cf1faf7ee5d2942fdbece55d14fe4c49eb80a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 02:00:21 +0100 Subject: [PATCH 2/4] feat(plans): settle a failed-with-submission execution so a managed plan can be deleted (#999) SettleFailedExecution records an operator's acknowledged, reasoned settlement of a failed plan execution with no no_submission proof. It locks the plan FOR SHARE, then the row and its retry-chain tail FOR UPDATE, and refuses while the chain may still buy: a tail outside the terminal allowlist or a failed tail that can still be retried. Replaying the same request returns the stored settlement. The row stays failed and keeps counting as possibly bought in the ramp ledger. requireManagedDeleteTx skips a settled failed row, and LinkRetryExecutionAtomic refuses one, since a retry of a row detached from its deleted plan would buy with no plan. Co-Authored-By: Claude Opus 5.5 --- internal/config/store_postgres.go | 4 +- .../config/store_postgres_execution_settle.go | 233 ++++++++++++++++++ .../store_postgres_execution_settle_test.go | 40 +++ .../config/store_postgres_managed_guard.go | 24 +- 4 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 internal/config/store_postgres_execution_settle.go create mode 100644 internal/config/store_postgres_execution_settle_test.go diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index c90c71ab5..1f0f8b6b6 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1193,7 +1193,8 @@ 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. @@ -1205,6 +1206,7 @@ func (s *PostgresStore) LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, WHERE execution_id = $1 AND status = 'failed' AND retry_execution_id IS NULL + AND outcome_settled_at IS NULL ` tag, err := tx.Exec(ctx, q, executionID, retryExecutionID) if err != nil { diff --git a/internal/config/store_postgres_execution_settle.go b/internal/config/store_postgres_execution_settle.go new file mode 100644 index 000000000..e22a907c3 --- /dev/null +++ b/internal/config/store_postgres_execution_settle.go @@ -0,0 +1,233 @@ +package config + +import ( + "context" + "errors" + "fmt" + "time" + + "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") +) + +// 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 + noSubmission bool + retryID *string + planID *string + settledAt *time.Time + settledBy *string + settlement *string +} + +// SettleFailedExecution records an operator's audited settlement of a failed +// plan execution that has no no_submission proof, so it stops blocking the +// delete of its editor-managed plan (requireManagedDeleteTx). It never changes +// the status and never makes the row count as unbought: the ramp ledger and +// the retention trigger keep treating it as possibly bought. 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 { + 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 nil, fmt.Errorf("%w: only an execution of a purchase plan can be settled", ErrExecutionSettleConflict) + } + 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, no_submission, 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.noSubmission, &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 +} + +// checkSettleTarget allows only the rows that block a managed delete for an +// unknown outcome: failed without a no_submission proof. +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) + } + if t.noSubmission { + return fmt.Errorf("%w: execution is proven never submitted and needs no settlement", ErrExecutionSettleConflict) + } + 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 can still be retried (no +// no_submission proof and not settled itself), 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 noSubmission, settled bool + err = tx.QueryRow(ctx, `SELECT status, no_submission, retry_execution_id::text, outcome_settled_at IS NOT NULL + FROM purchase_executions WHERE execution_id = $1 FOR UPDATE`, tail.id).Scan(&tail.status, &noSubmission, &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 && !noSubmission && !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. +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. +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) { + return nil, fmt.Errorf("%w: already settled by %s at %s", ErrExecutionSettleConflict, *t.settledBy, t.settledAt.UTC().Format(time.RFC3339)) + } + 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_test.go b/internal/config/store_postgres_execution_settle_test.go new file mode 100644 index 000000000..9018e7fc6 --- /dev/null +++ b/internal/config/store_postgres_execution_settle_test.go @@ -0,0 +1,40 @@ +package config + +import ( + "testing" + + "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`)) +} diff --git a/internal/config/store_postgres_managed_guard.go b/internal/config/store_postgres_managed_guard.go index fc4b0064b..bcb734307 100644 --- a/internal/config/store_postgres_managed_guard.go +++ b/internal/config/store_postgres_managed_guard.go @@ -123,21 +123,37 @@ 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) + rows, err := tx.Query(ctx, `SELECT status, no_submission, outcome_settled_at IS NOT NULL 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 +} From 981a04f4c0a78a91f9cc02b9af4e71e90164d379 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 02:00:21 +0100 Subject: [PATCH 3/4] test(plans): prove settle, delete and retry on real Postgres, including races (#999) Covers the issue scenario (a failed row whose retry completed blocks the managed delete until settled; the ledger is unchanged), every refusal, chains of unknown outcomes, replay, the retry CAS, and the three orderings that matter: a retry that links first, a delete that holds the plan first, and a delete that queues behind a settle. Co-Authored-By: Claude Opus 5.5 --- ...tgres_execution_settle_integration_test.go | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 internal/config/store_postgres_execution_settle_integration_test.go 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..67be9d2ad --- /dev/null +++ b/internal/config/store_postgres_execution_settle_integration_test.go @@ -0,0 +1,217 @@ +//go:build integration +// +build integration + +package config + +import ( + "context" + "testing" + + "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, + "failed no_submission": f.rowWithRecs(ctx, t, 1, acct, StatusFailed, true).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 + })) +} + +// 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)) +} From 73ac5a13aa7c99d2f5b7dcc4cc9561a5af26620b Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 02:00:21 +0100 Subject: [PATCH 4/4] feat(api): add POST /api/purchases/{id}/settle for unknown-outcome failures (#999) The endpoint needs execute:purchases, account access to the execution and the grant's constraints against what the execution would have bought (strict scope), a 10-300 character reason and acknowledge_unknown_outcome: true. It logs the settlement with the actor and credential kind. Retry's lost-race 409 now says the row may have been settled. Marks the known issue resolved. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_execution_settle.go | 157 ++++++++++++++++++ ...ndler_execution_settle_integration_test.go | 112 +++++++++++++ internal/api/handler_execution_settle_test.go | 45 +++++ internal/api/handler_purchases.go | 4 +- internal/api/router.go | 8 + known_issues/35_legacy_guard_followups.md | 8 +- 6 files changed, 330 insertions(+), 4 deletions(-) create mode 100644 internal/api/handler_execution_settle.go create mode 100644 internal/api/handler_execution_settle_integration_test.go create mode 100644 internal/api/handler_execution_settle_test.go diff --git a/internal/api/handler_execution_settle.go b/internal/api/handler_execution_settle.go new file mode 100644 index 000000000..0676f40e8 --- /dev/null +++ b/internal/api/handler_execution_settle.go @@ -0,0 +1,157 @@ +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()) + } + 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 af7c61b96..8f5b49084 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -2599,8 +2599,10 @@ func (h *Handler) persistRetrySuccessorTx(ctx context.Context, failedExec, newEx map[string]any{}) } if lostRace { + // Zero rows linked: a concurrent retry won, or an operator settled the + // row (issue #999), which retires it from retry for good. 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{}) } return fmt.Errorf("failed to save retry execution: %w", err) diff --git a/internal/api/router.go b/internal/api/router.go index 9b5ba7b4e..3641ba592 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 @@ -624,6 +628,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/known_issues/35_legacy_guard_followups.md b/known_issues/35_legacy_guard_followups.md index 4b800b0ff..a68883f35 100644 --- a/known_issues/35_legacy_guard_followups.md +++ b/known_issues/35_legacy_guard_followups.md @@ -1,6 +1,6 @@ # Known Issues: follow-ups from the legacy-writer guards (#631) -> **Audit status (2026-10-16):** `2 needs triage · 0 resolved` +> **Audit status (2026-10-22):** `1 needs triage · 1 resolved` ## LOW: cleanup lambda deletes by expires_at with no status predicate @@ -14,7 +14,7 @@ **Status:** Needs triage. -## 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`) @@ -24,4 +24,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`, migration 000143). 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.