fix: if candidate version is already rolled out gradual rollout evaluator short circuits

apps/workspace-engine/pkg/db/queries/releases.sql

@@ -106,6 +106,22 @@ LEFT JOIN release_variable rv ON rv.release_id = r.id
 WHERE rtr.resource_id = $1 AND rtr.environment_id = $2 AND rtr.deployment_id = $3
 GROUP BY r.id, dv.id;
 
+-- name: GetCurrentVersionIDByReleaseTarget :one
+-- Returns the version_id of the latest successful release for a release target.
+-- Lightweight variant of GetCurrentReleaseByReleaseTarget for callers that
+-- only need the version identifier (e.g., gradual rollout short-circuit).
+SELECT r.version_id
+FROM release r
+JOIN release_job rj ON rj.release_id = r.id
+JOIN job j ON j.id = rj.job_id
+WHERE r.resource_id = @resource_id
+  AND r.environment_id = @environment_id
+  AND r.deployment_id = @deployment_id
+  AND j.status = 'successful'
+  AND j.completed_at IS NOT NULL
+ORDER BY j.completed_at DESC
+LIMIT 1;
+
 -- name: GetCurrentReleaseByReleaseTarget :one
 -- Returns the release associated with the latest successful job for a release target,
 -- including version and variables.

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/gradualrollout/getters.go

@@ -2,9 +2,11 @@ package gradualrollout
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/google/uuid"
+	"github.com/jackc/pgx/v5"
 	"workspace-engine/pkg/db"
 	"workspace-engine/pkg/oapi"
 	"workspace-engine/pkg/store/policies"
@@ -13,6 +15,32 @@ import (
 	"workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression"
 )
 
+type ReleaseTargetIDs struct {
+	EnvironmentID uuid.UUID
+	ResourceID    uuid.UUID
+	DeploymentID  uuid.UUID
+}
+
+func parseReleaseTargetUUIDs(rt *oapi.ReleaseTarget) (ReleaseTargetIDs, error) {
+	resourceID, err := uuid.Parse(rt.ResourceId)
+	if err != nil {
+		return ReleaseTargetIDs{}, fmt.Errorf("parse resource id: %w", err)
+	}
+	environmentID, err := uuid.Parse(rt.EnvironmentId)
+	if err != nil {
+		return ReleaseTargetIDs{}, fmt.Errorf("parse environment id: %w", err)
+	}
+	deploymentID, err := uuid.Parse(rt.DeploymentId)
+	if err != nil {
+		return ReleaseTargetIDs{}, fmt.Errorf("parse deployment id: %w", err)
+	}
+	return ReleaseTargetIDs{
+		EnvironmentID: environmentID,
+		ResourceID:    resourceID,
+		DeploymentID:  deploymentID,
+	}, nil
+}
+
 type approvalGetters = approval.Getters
 type environmentProgressionGetters = environmentprogression.Getters
 
@@ -29,6 +57,7 @@ type Getters interface {
 		versionID, environmentID, resourceID string,
 	) ([]*oapi.PolicySkip, error)
 	HasCurrentRelease(ctx context.Context, releaseTarget *oapi.ReleaseTarget) (bool, error)
+	GetCurrentVersionID(ctx context.Context, releaseTarget *oapi.ReleaseTarget) (*string, error)
 }
 
 // ---------------------------------------------------------------------------
@@ -99,28 +128,46 @@ func (p *PostgresGetters) GetPolicySkips(
 	return ps, nil
 }
 
-func (p *PostgresGetters) HasCurrentRelease(
+func (p *PostgresGetters) GetCurrentVersionID(
 	ctx context.Context,
 	releaseTarget *oapi.ReleaseTarget,
-) (bool, error) {
-	resourceIDUUID, err := uuid.Parse(releaseTarget.ResourceId)
+) (*string, error) {
+	ids, err := parseReleaseTargetUUIDs(releaseTarget)
 	if err != nil {
-		return false, fmt.Errorf("parse resource id: %w", err)
+		return nil, err
 	}
-	environmentIDUUID, err := uuid.Parse(releaseTarget.EnvironmentId)
+	versionID, err := p.queries.GetCurrentVersionIDByReleaseTarget(
+		ctx,
+		db.GetCurrentVersionIDByReleaseTargetParams{
+			ResourceID:    ids.ResourceID,
+			EnvironmentID: ids.EnvironmentID,
+			DeploymentID:  ids.DeploymentID,
+		},
+	)
 	if err != nil {
-		return false, fmt.Errorf("parse environment id: %w", err)
+		if errors.Is(err, pgx.ErrNoRows) {
+			return nil, nil
+		}
+		return nil, err
 	}
-	deploymentIDUUID, err := uuid.Parse(releaseTarget.DeploymentId)
+	s := versionID.String()
+	return &s, nil
+}
+
+func (p *PostgresGetters) HasCurrentRelease(
+	ctx context.Context,
+	releaseTarget *oapi.ReleaseTarget,
+) (bool, error) {
+	ids, err := parseReleaseTargetUUIDs(releaseTarget)
 	if err != nil {
-		return false, fmt.Errorf("parse deployment id: %w", err)
+		return false, err
 	}
 	releases, err := p.queries.ListReleasesByReleaseTarget(
 		ctx,
 		db.ListReleasesByReleaseTargetParams{
-			ResourceID:    resourceIDUUID,
-			EnvironmentID: environmentIDUUID,
-			DeploymentID:  deploymentIDUUID,
+			ResourceID:    ids.ResourceID,
+			EnvironmentID: ids.EnvironmentID,
+			DeploymentID:  ids.DeploymentID,
 		},
 	)
 	if err != nil {

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/gradualrollout/gradualrollout.go

@@ -357,6 +357,19 @@ func (e *GradualRolloutEvaluator) Evaluate(
 			WithDetail("error", err.Error())
 	}
 
+	currentVersionID, err := e.getters.GetCurrentVersionID(ctx, releaseTarget)
+	if err != nil {
+		return results.
+			NewDeniedResult(fmt.Sprintf("Failed to get current version: %v", err)).
+			WithDetail("error", err.Error())
+	}
+	if currentVersionID != nil && *currentVersionID == version.Id {
+		return results.
+			NewAllowedResult("Resource already on this version; gradual rollout does not revert").
+			WithDetail("resource", resource).
+			WithDetail("current_version_id", version.Id)
+	}
+
 	releaseTargets, err := e.getReleaseTargets(ctx, environment, version)
 	if err != nil {
 		return results.

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/gradualrollout/gradualrollout_test.go

@@ -17,12 +17,13 @@ import (
 
 // mockGetters implements the full Getters interface for testing.
 type mockGetters struct {
-	resources       map[string]*oapi.Resource
-	releaseTargets  []*oapi.ReleaseTarget
-	policies        []*oapi.Policy
-	policySkips     []*oapi.PolicySkip
-	hasRelease      bool
-	approvalRecords []*oapi.UserApprovalRecord
+	resources        map[string]*oapi.Resource
+	releaseTargets   []*oapi.ReleaseTarget
+	policies         []*oapi.Policy
+	policySkips      []*oapi.PolicySkip
+	hasRelease       bool
+	currentVersionID *string
+	approvalRecords  []*oapi.UserApprovalRecord
 
 	environments   map[string]*oapi.Environment
 	deployments    map[string]*oapi.Deployment
@@ -153,6 +154,13 @@ func (m *mockGetters) HasCurrentRelease(_ context.Context, _ *oapi.ReleaseTarget
 	return m.hasRelease, nil
 }
 
+func (m *mockGetters) GetCurrentVersionID(
+	_ context.Context,
+	_ *oapi.ReleaseTarget,
+) (*string, error) {
+	return m.currentVersionID, nil
+}
+
 func (m *mockGetters) GetJobsForEnvironmentAndVersion(
 	_ context.Context,
 	_, _ string,
@@ -1436,6 +1444,72 @@ func TestGradualRolloutEvaluator_DeploymentWindow_DenyWindowPreventsFrontloading
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Tests: Already-on-version short-circuit (rollback prevention)
+// ---------------------------------------------------------------------------
+
+// Scenario: a resource advanced to version V while a policy override was active.
+// The override has now expired. Without the short-circuit, gradual rollout
+// would re-evaluate the curve, find this resource's slot hasn't been reached,
+// return Pending, and the desired-release loop would fall back to V-1 — rolling
+// the resource back. With the short-circuit, evaluating V against a resource
+// already on V returns Allowed regardless of the curve.
+func TestGradualRolloutEvaluator_AlreadyOnVersion_AllowsRegardlessOfCurve(t *testing.T) {
+	baseTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
+	ts := newTestSetup(5, baseTime)
+
+	// Mock all resources as already on this version (simulates the override-
+	// induced advancement). The mock is global per-test, so every release target
+	// reports the same current version — that's fine; we only assert on the one
+	// resource whose curve slot the test actually cares about.
+	ts.mock.currentVersionID = &ts.version.Id
+
+	// Current time is 30 seconds in — without the short-circuit, position 4's
+	// curve slot is at t+240s and would return Pending.
+	thirtySecLater := baseTime.Add(30 * time.Second)
+	rule := createGradualRolloutRule(oapi.GradualRolloutRuleRolloutTypeLinear, 60)
+	eval := ts.eval(rule, func() time.Time { return thirtySecLater })
+
+	result := eval.Evaluate(ts.ctx, ts.scope(4))
+	assert.True(
+		t,
+		result.Allowed,
+		"resource already on this version should be allowed regardless of curve position",
+	)
+}
+
+// A resource on a DIFFERENT version (e.g., the prior version) must still be
+// gated by the curve — the short-circuit must not leak into the normal case.
+func TestGradualRolloutEvaluator_OnDifferentVersion_CurveStillGates(t *testing.T) {
+	baseTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
+	ts := newTestSetup(5, baseTime)
+
+	priorVersionID := uuid.New().String()
+	ts.mock.currentVersionID = &priorVersionID
+
+	thirtySecLater := baseTime.Add(30 * time.Second)
+	rule := createGradualRolloutRule(oapi.GradualRolloutRuleRolloutTypeLinear, 60)
+	eval := ts.eval(rule, func() time.Time { return thirtySecLater })
+
+	result := eval.Evaluate(ts.ctx, ts.scope(4))
+	assert.False(t, result.Allowed, "resource not yet on candidate version must wait for its slot")
+}
+
+// No prior release at all: evaluator falls through to normal curve evaluation.
+func TestGradualRolloutEvaluator_NoCurrentVersion_CurveStillGates(t *testing.T) {
+	baseTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
+	ts := newTestSetup(5, baseTime)
+
+	ts.mock.currentVersionID = nil // never deployed
+
+	thirtySecLater := baseTime.Add(30 * time.Second)
+	rule := createGradualRolloutRule(oapi.GradualRolloutRuleRolloutTypeLinear, 60)
+	eval := ts.eval(rule, func() time.Time { return thirtySecLater })
+
+	result := eval.Evaluate(ts.ctx, ts.scope(4))
+	assert.False(t, result.Allowed, "first-time deploys must still respect the curve")
+}
+
 // ---------------------------------------------------------------------------
 // Tests: NewEvaluator constructor
 // ---------------------------------------------------------------------------

apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.go

@@ -97,6 +97,12 @@ func (m *mockGetter) GetApprovalRecords(
 func (m *mockGetter) HasCurrentRelease(_ context.Context, _ *oapi.ReleaseTarget) (bool, error) {
 	return false, nil
 }
+func (m *mockGetter) GetCurrentVersionID(
+	_ context.Context,
+	_ *oapi.ReleaseTarget,
+) (*string, error) {
+	return nil, nil
+}
 func (m *mockGetter) GetPolicySkips(_ context.Context, _, _, _ string) ([]*oapi.PolicySkip, error) {
 	return m.policySkips, m.policySkipsErr
 }

apps/workspace-engine/svc/controllers/desiredrelease/reconcile_test.go

@@ -126,6 +126,13 @@ func (m *mockReconcileGetter) HasCurrentRelease(
 	return false, nil
 }
 
+func (m *mockReconcileGetter) GetCurrentVersionID(
+	_ context.Context,
+	_ *oapi.ReleaseTarget,
+) (*string, error) {
+	return nil, nil
+}
+
 func (m *mockReconcileGetter) GetEnvironment(
 	_ context.Context,
 	_ string,

apps/workspace-engine/test/controllers/harness/mocks.go

@@ -151,6 +151,11 @@ type DesiredReleaseGetter struct {
 
 	// HasCurrentReleaseFn allows per-release-target logic when set.
 	HasCurrentReleaseFn func(rt *oapi.ReleaseTarget) bool
+
+	CurrentVersionID *string
+
+	// CurrentVersionIDFn allows per-release-target logic when set.
+	CurrentVersionIDFn func(rt *oapi.ReleaseTarget) *string
 }
 
 func (g *DesiredReleaseGetter) ReleaseTargetExists(
@@ -209,6 +214,16 @@ func (g *DesiredReleaseGetter) HasCurrentRelease(
 	return g.HasRelease, nil
 }
 
+func (g *DesiredReleaseGetter) GetCurrentVersionID(
+	_ context.Context,
+	rt *oapi.ReleaseTarget,
+) (*string, error) {
+	if g.CurrentVersionIDFn != nil {
+		return g.CurrentVersionIDFn(rt), nil
+	}
+	return g.CurrentVersionID, nil
+}
+
 func (g *DesiredReleaseGetter) GetCurrentRelease(
 	_ context.Context,
 	_ *desiredrelease.ReleaseTarget,