From 9db33051a24965ea34ebe12891cc15fedb304977 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 13 Oct 2026 14:47:41 +0200 Subject: [PATCH 1/2] fix(ladder): refuse to execute an execution whose tranche is already settled The execute-time recheck read the config switches and the run status but never the tranche status. An execution that carries a tranche key but is not on the locked retry chain could still buy after the tranche was failed, completed or canceled. GetLadderExecutionGate now returns the tranche status and ladderGateRefusal refuses unless the tranche is scheduled or fired; a missing status refuses too. The refusal is a beforeSubmission error as before. Refs #118 Co-Authored-By: Claude Sonnet 5.5 --- ...re_postgres_ladder_gate_integration_test.go | 2 ++ .../config/store_postgres_ladder_reconcile.go | 7 +++++-- internal/purchase/ladder_execution_recheck.go | 4 ++++ .../purchase/ladder_execution_recheck_test.go | 18 ++++++++++++++---- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/config/store_postgres_ladder_gate_integration_test.go b/internal/config/store_postgres_ladder_gate_integration_test.go index 48592b97..115e3882 100644 --- a/internal/config/store_postgres_ladder_gate_integration_test.go +++ b/internal/config/store_postgres_ladder_gate_integration_test.go @@ -39,6 +39,8 @@ func TestGetLadderExecutionGate(t *testing.T) { assert.True(t, *g.ConfigEnabled) assert.Equal(t, "email_approval", *g.ConfigMode) assert.Equal(t, "executing", *g.RunStatus) + require.NotNil(t, g.TrancheStatus) + assert.Equal(t, "fired", *g.TrancheStatus) }) t.Run("non-ladder keys are not an error", func(t *testing.T) { diff --git a/internal/config/store_postgres_ladder_reconcile.go b/internal/config/store_postgres_ladder_reconcile.go index ce68ec7f..edb957eb 100644 --- a/internal/config/store_postgres_ladder_reconcile.go +++ b/internal/config/store_postgres_ladder_reconcile.go @@ -141,10 +141,13 @@ func (s *PostgresStore) LockLadderTrancheForRetryTx(ctx context.Context, tx pgx. // call, about the ladder tranche an execution belongs to. Each field is a // pointer because the foreign keys are ON DELETE SET NULL: a nil field means // the row is gone, which the caller must treat as "refuse", not as a default. +// TrancheStatus is the tranche's own status: an operator may have settled the +// tranche while an execution carrying its key is still waiting to run. type LadderExecutionGate struct { ConfigEnabled *bool ConfigMode *string RunStatus *string + TrancheStatus *string } // GetLadderExecutionGate finds the ladder tranche whose id equals an @@ -159,11 +162,11 @@ func (s *PostgresStore) GetLadderExecutionGate(ctx context.Context, idempotencyK } var g LadderExecutionGate err := s.db.QueryRow(ctx, ` - SELECT c.enabled, c.mode, r.status + SELECT c.enabled, c.mode, r.status, t.status FROM ladder_tranches t LEFT JOIN ladder_configs c ON c.id = t.config_id LEFT JOIN ladder_runs r ON r.id = t.run_id - WHERE t.id::text = $1`, idempotencyKey).Scan(&g.ConfigEnabled, &g.ConfigMode, &g.RunStatus) + WHERE t.id::text = $1`, idempotencyKey).Scan(&g.ConfigEnabled, &g.ConfigMode, &g.RunStatus, &g.TrancheStatus) if errors.Is(err, pgx.ErrNoRows) { return nil, nil } diff --git a/internal/purchase/ladder_execution_recheck.go b/internal/purchase/ladder_execution_recheck.go index 0a64889f..44c29b82 100644 --- a/internal/purchase/ladder_execution_recheck.go +++ b/internal/purchase/ladder_execution_recheck.go @@ -62,6 +62,10 @@ func (m *Manager) ladderFlagsRefusal(ctx context.Context) error { func ladderGateRefusal(gate *config.LadderExecutionGate) error { switch { + case gate.TrancheStatus == nil: + return errors.New("refusing to execute ladder purchase: its tranche status is unknown") + case config.LadderTrancheSettled(ladder.TrancheStatus(*gate.TrancheStatus)): + return fmt.Errorf("refusing to execute ladder purchase: its tranche is %s", *gate.TrancheStatus) case gate.ConfigEnabled == nil || gate.ConfigMode == nil: return errors.New("refusing to execute ladder purchase: its ladder config no longer exists") case !*gate.ConfigEnabled: diff --git a/internal/purchase/ladder_execution_recheck_test.go b/internal/purchase/ladder_execution_recheck_test.go index e4d1023c..747ce4f5 100644 --- a/internal/purchase/ladder_execution_recheck_test.go +++ b/internal/purchase/ladder_execution_recheck_test.go @@ -26,7 +26,7 @@ func ptr[T any](v T) *T { return &v } // openGate is a gate that allows execution; each case flips one field. func openGate() *config.LadderExecutionGate { - return &config.LadderExecutionGate{ConfigEnabled: ptr(true), ConfigMode: ptr("auto_approve"), RunStatus: ptr("executing")} + return &config.LadderExecutionGate{ConfigEnabled: ptr(true), ConfigMode: ptr("auto_approve"), RunStatus: ptr("executing"), TrancheStatus: ptr("fired")} } func ladderGlobal(planning, execution bool) *config.GlobalConfig { @@ -85,6 +85,10 @@ func TestLadderRecheck_RefusesBeforeProviderCall(t *testing.T) { {"run canceled", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = ptr(string(ladder.RunStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "run was canceled"}, {"config deleted", gateWith(func(g *config.LadderExecutionGate) { g.ConfigEnabled, g.ConfigMode = nil, nil }), nil, ladderGlobal(true, true), nil, "config no longer exists"}, {"run deleted", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = nil }), nil, ladderGlobal(true, true), nil, "run no longer exists"}, + {"tranche failed by operator", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusFailed)) }), nil, ladderGlobal(true, true), nil, "its tranche is failed"}, + {"tranche completed", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCompleted)) }), nil, ladderGlobal(true, true), nil, "its tranche is completed"}, + {"tranche cancelled", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "its tranche is cancelled"}, + {"tranche status unknown", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = nil }), nil, ladderGlobal(true, true), nil, "tranche status is unknown"}, {"tranche lookup DB error", nil, errors.New("connection reset"), ladderGlobal(true, true), nil, "tranche lookup failed"}, {"global config read error", openGate(), nil, nil, errors.New("connection reset"), "global config unreadable"}, {"global config nil", openGate(), nil, nil, nil, "global config unavailable"}, @@ -110,9 +114,15 @@ func TestLadderRecheck_RefusesBeforeProviderCall(t *testing.T) { } func TestLadderRecheck_AllowsWhenEverythingIsOn(t *testing.T) { - mgr, store, rec, exec := ladderHarness(t, ladderTrancheKey, openGate(), nil, ladderGlobal(true, true), nil) - runViaSQS(t, mgr, store, exec) - assert.Equal(t, 1, rec.count(), "an armed ladder execution must still buy exactly once") + for _, status := range []ladder.TrancheStatus{ladder.TrancheStatusFired, ladder.TrancheStatusScheduled} { + t.Run(string(status), func(t *testing.T) { + gate := openGate() + gate.TrancheStatus = ptr(string(status)) + mgr, store, rec, exec := ladderHarness(t, ladderTrancheKey, gate, nil, ladderGlobal(true, true), nil) + runViaSQS(t, mgr, store, exec) + assert.Equal(t, 1, rec.count(), "an armed ladder execution on a %s tranche must still buy exactly once", status) + }) + } } // Non-ladder keys must be untouched: the lookup returns no tranche for them, From d8338bc0f918ec15291779e5439d70764492d636 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 13 Oct 2026 14:57:14 +0200 Subject: [PATCH 2/2] test(ladder): fix misspell lint and refuse an unknown tranche status Refs #118 Co-Authored-By: Claude Sonnet 5.5 --- internal/purchase/ladder_execution_recheck_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/purchase/ladder_execution_recheck_test.go b/internal/purchase/ladder_execution_recheck_test.go index 747ce4f5..a047db7a 100644 --- a/internal/purchase/ladder_execution_recheck_test.go +++ b/internal/purchase/ladder_execution_recheck_test.go @@ -87,7 +87,8 @@ func TestLadderRecheck_RefusesBeforeProviderCall(t *testing.T) { {"run deleted", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = nil }), nil, ladderGlobal(true, true), nil, "run no longer exists"}, {"tranche failed by operator", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusFailed)) }), nil, ladderGlobal(true, true), nil, "its tranche is failed"}, {"tranche completed", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCompleted)) }), nil, ladderGlobal(true, true), nil, "its tranche is completed"}, - {"tranche cancelled", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "its tranche is cancelled"}, + {"tranche cancelled", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "its tranche is cancelled"}, //nolint:misspell // "cancelled" is the stored ladder status value + {"tranche status not known to the code", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr("bogus") }), nil, ladderGlobal(true, true), nil, "its tranche is bogus"}, {"tranche status unknown", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = nil }), nil, ladderGlobal(true, true), nil, "tranche status is unknown"}, {"tranche lookup DB error", nil, errors.New("connection reset"), ladderGlobal(true, true), nil, "tranche lookup failed"}, {"global config read error", openGate(), nil, nil, errors.New("connection reset"), "global config unreadable"},