Skip to content

Converge deployment preview and deploy onto shared eligibility logic

Placeholder ppxd requested to merge converge-preview-deploy-eligibility into main

Summary

Make the deployment preview and the real deployment reach every target/step/action eligibility decision through the same underlying primitive, so they can only differ in describe-vs-execute (preview assumes success; deploy uses real failure-state + variables) — never in logic. Audited every decision; closed the two residual divergence points.

  • Machine selection — preview had a private ApplyMachineSelection identical to the shared DeploymentTargetFinder.ApplyMachineSelection the real deployment uses. Routed preview through the shared primitive and deleted the duplicate, so a future change to selection (e.g. tag-based) can't silently diverge.
  • Dead divergent health filter — removed DeploymentTargetFinder.FilterByHealthStatus: a public, divergent filter with no production callers (superseded by TransientDeploymentTargetEvaluator.ApplyProjectPolicy) that hardcoded Unhealthy/Unavailable exclusion and bypassed the project's transient-target policy. A caller using it would have skipped project policy. Coverage is subsumed by TransientDeploymentTargetEvaluatorTests.
  • SkipActionIds (behavior fix) — PlanDeploymentPhase built the shadow plan without the manual-skip set, so ExecuteStepsPhase.SplitActionsUsingPlan found a skipped action in the plan and ran it. The preview already passes SkipActionIds to the same planner; the deploy side now forwards it from the deployment payload too.

Behavior change

A manually-skipped action (deploy dialog "skip", DeploymentRequestPayload.SkipActionIds) previously ran on the real deployment while the preview correctly showed it skipped. After this change it is skipped on deploy as well, matching the preview and the feature's intent. No interface/signature/wire-contract change. Blast radius is limited to deployments that explicitly skip actions.

Convergence map (post-change)

Every planner input and machine-resolution decision now has a single shared implementation:

Decision Shared primitive
env + disabled filter MachineDataProvider.GetMachinesByFilterAsync
machine selection DeploymentTargetFinder.ApplyMachineSelection
transient/health policy TransientDeploymentTargetEvaluator.ApplyProjectPolicy
step→target role match StepRoleMatcher
action eligibility (env/channel/skip) StepEligibilityEvaluator.EvaluateAction
step condition (Success/Failure/Variable) StepEligibilityEvaluator.EvaluateStep (runtime-only by design)
steps from snapshot ProcessSnapshotStepConverter.Convert

Test plan

  • DeploymentTargetFinderTests.Selection_PreviewPrimitive_MatchesDeployPath (Theory, 4 cases) — pins preview == deploy selection
  • PlanDeploymentPhaseTests.ExecuteAsync_ForwardsSkipActionIdsToPlanner_SoDeployHonoursManualSkip — written test-first (RED before the fix, GREEN after)
  • Removed HealthStatusFilterTests (coverage subsumed by TransientDeploymentTargetEvaluatorTests.Defaults_ReproduceHistoricalExclusion)
  • Full unit suite green (5904/5904)
  • Full solution build (0 errors)

Merge request reports