Skip PostDeploy convention when the main script fails

src/Squid.Calamari/Commands/Conventions/ConventionScriptStep.cs

@@ -40,6 +40,7 @@ internal sealed class ConventionScriptStep : ExecutionStep<RunScriptCommandConte
 {
     private readonly string _conventionName;
     private readonly IScriptEngine _scriptEngine;
+    private readonly bool _skipWhenDeployFailed;
 
     /// <summary>
     /// Construct a convention hook bound to a script filename (without
@@ -49,12 +50,18 @@ internal sealed class ConventionScriptStep : ExecutionStep<RunScriptCommandConte
     /// is case-sensitive by default to match Octopus's case-preserving
     /// behaviour.
     /// </summary>
-    public ConventionScriptStep(string conventionName, IScriptEngine scriptEngine)
+    /// <param name="skipWhenDeployFailed">When <c>true</c>, the hook is skipped
+    /// once the deploy has failed (<see cref="RunScriptCommandContext.DeployHasFailed"/>) —
+    /// used for PostDeploy so a smoke test / traffic switch never runs against a
+    /// failed deploy. PreDeploy passes <c>false</c> (the default): it runs before
+    /// the main script, so the failure state is never set when it is evaluated.</param>
+    public ConventionScriptStep(string conventionName, IScriptEngine scriptEngine, bool skipWhenDeployFailed = false)
     {
         if (string.IsNullOrWhiteSpace(conventionName))
             throw new ArgumentException("Convention name MUST be non-empty.", nameof(conventionName));
         _conventionName = conventionName;
         _scriptEngine = scriptEngine ?? throw new ArgumentNullException(nameof(scriptEngine));
+        _skipWhenDeployFailed = skipWhenDeployFailed;
     }
 
     /// <summary>The convention name — exposed for test pinning + log output.</summary>
@@ -68,6 +75,11 @@ public override bool IsEnabled(RunScriptCommandContext context)
         if (string.IsNullOrEmpty(context.WorkingDirectory)) return false;
         if (context.Variables is null) return false;
 
+        // PostDeploy must not run once the deploy has failed (non-zero main-script
+        // exit or a prior step exception). PreDeploy passes skipWhenDeployFailed=false
+        // and runs before the main script, so this gate never trips for it.
+        if (_skipWhenDeployFailed && context.DeployHasFailed) return false;
+
         return ConventionScriptResolver.Resolve(
             context.WorkingDirectory, _conventionName, PreferredSyntax(context)) is not null;
     }

src/Squid.Calamari/Commands/Conventions/DeployFailedConventionStep.cs

@@ -52,11 +52,11 @@ public bool IsEnabled(RunScriptCommandContext context)
         if (string.IsNullOrEmpty(context.WorkingDirectory)) return false;
         if (context.Variables is null) return false;
 
-        // Predicate: at least one of (a) something threw before cleanup
-        // phase, or (b) main script ran but exit code is non-zero.
-        var hadFailure = context.ExecutionFailed
-                         || (context.ScriptResult is not null && context.ScriptResult.ExitCode != 0);
-        if (!hadFailure) return false;
+        // Predicate: the deploy failed — something threw before the cleanup
+        // phase, or the main script ran but exited non-zero. Same single source
+        // of truth the PostDeploy gate consults (inverted), so the two can never
+        // disagree about whether the deploy failed.
+        if (!context.DeployHasFailed) return false;
 
         return ConventionScriptResolver.Resolve(
             context.WorkingDirectory, ConventionName, PreferredSyntax(context)) is not null;

src/Squid.Calamari/Commands/RunScriptCommand.cs

@@ -30,7 +30,9 @@ namespace Squid.Calamari.Commands;
 ///   <item>WriteBootstrappedBashScript — prepend `export VAR=` for the main script.</item>
 ///   <item>ExecuteScriptWithEngine — run the operator's main script.</item>
 ///   <item><b>PostDeploy convention (G1.5)</b> — runs <c>PostDeploy.sh</c>.
-///         Smoke tests, cache warm-up, service-mesh registration, etc.</item>
+///         Smoke tests, cache warm-up, service-mesh registration, etc.
+///         Skipped if the deploy already failed (non-zero main-script exit
+///         or a prior step exception).</item>
 ///   <item>BuildRunScriptCommandResult — collect exit code + outputs.</item>
 ///   <item>CleanupTemporaryFiles — best-effort cleanup (always-runs).</item>
 /// </list></para>
@@ -79,8 +81,10 @@ public RunScriptCommand(IScriptEngine scriptEngine)
             new ExecuteScriptWithEngineStep(scriptEngine),
             // G1.5 — PostDeploy convention hook. Runs only when the package
             // ships a `PostDeploy.sh` file. Smoke tests, cache warm-up,
-            // registration with a service mesh, etc.
-            new ConventionScriptStep(ConventionScriptNames.PostDeploy, scriptEngine),
+            // registration with a service mesh, etc. skipWhenDeployFailed=true:
+            // a non-zero main-script exit is a failed deploy, so PostDeploy is
+            // skipped (a smoke test against a failed deploy is worse than none).
+            new ConventionScriptStep(ConventionScriptNames.PostDeploy, scriptEngine, skipWhenDeployFailed: true),
             new BuildRunScriptCommandResultStep(),
             // DeployFailed convention — IAlwaysRun (cleanup phase). Fires only
             // when an upstream step threw OR the main script returned non-zero.

src/Squid.Calamari/Commands/RunScriptCommandPipeline.cs

@@ -43,6 +43,17 @@ internal sealed class RunScriptCommandContext : IPathBasedExecutionContext, IVar
     /// </summary>
     public bool ExecutionFailed { get; set; }
 
+    /// <summary>
+    /// True once the deploy has failed: either an upstream step threw
+    /// (<see cref="ExecutionFailed"/>) or the main script ran but returned a
+    /// non-zero exit code. Single source of truth for the two consumers that
+    /// must agree on "did the deploy fail?": the DeployFailed cleanup hook
+    /// (fires when true) and the PostDeploy convention (skipped when true, so a
+    /// smoke test / traffic switch never runs against a failed deploy).
+    /// </summary>
+    public bool DeployHasFailed
+        => ExecutionFailed || (ScriptResult is not null && ScriptResult.ExitCode != 0);
+
     /// <summary>
     /// PR-5 — structured per-step outcomes. Each rewriter / extract /
     /// convention / main-script step appends one entry as it finishes.

tests/Squid.Calamari.Tests/Calamari/Commands/Conventions/ConventionScriptStepTests.cs

@@ -90,6 +90,78 @@ public void IsEnabled_WorkingDirNull_SkipsStep_NoCrash()
         step.IsEnabled(ctx).ShouldBeFalse();
     }
 
+    // ── skipWhenDeployFailed gating (PostDeploy) ─────────────────────────────
+
+    [Fact]
+    public void IsEnabled_SkipWhenDeployFailed_MainExitedNonZero_SkipsEvenWithScriptPresent()
+    {
+        // PostDeploy is wired with skipWhenDeployFailed=true. A non-zero main-
+        // script exit is a failed deploy → PostDeploy MUST be skipped so a smoke
+        // test / traffic switch never runs against a failed deploy.
+        File.WriteAllText(Path.Combine(_workDir, "PostDeploy.sh"), "echo hi");
+        var step = new ConventionScriptStep(ConventionScriptNames.PostDeploy, new StubScriptEngine(), skipWhenDeployFailed: true);
+        var ctx = BuildContext();
+        ctx.ScriptResult = new ScriptExecutionResult(2);
+
+        step.IsEnabled(ctx).ShouldBeFalse();
+    }
+
+    [Fact]
+    public void IsEnabled_SkipWhenDeployFailed_ExecutionFailed_SkipsEvenWithScriptPresent()
+    {
+        // The other failure signal: an upstream step threw before PostDeploy.
+        File.WriteAllText(Path.Combine(_workDir, "PostDeploy.sh"), "echo hi");
+        var step = new ConventionScriptStep(ConventionScriptNames.PostDeploy, new StubScriptEngine(), skipWhenDeployFailed: true);
+        var ctx = BuildContext();
+        ctx.ExecutionFailed = true;
+
+        step.IsEnabled(ctx).ShouldBeFalse();
+    }
+
+    [Fact]
+    public void IsEnabled_SkipWhenDeployFailed_MainSucceeded_RunsWhenScriptPresent()
+    {
+        // Happy path: main script exit 0 → PostDeploy still runs.
+        File.WriteAllText(Path.Combine(_workDir, "PostDeploy.sh"), "echo hi");
+        var step = new ConventionScriptStep(ConventionScriptNames.PostDeploy, new StubScriptEngine(), skipWhenDeployFailed: true);
+        var ctx = BuildContext();
+        ctx.ScriptResult = new ScriptExecutionResult(0);
+
+        step.IsEnabled(ctx).ShouldBeTrue();
+    }
+
+    [Fact]
+    public void IsEnabled_DefaultFlag_IgnoresFailureState()
+    {
+        // PreDeploy passes skipWhenDeployFailed=false (the default). Even with a
+        // failure state on the context it stays gated only on file presence —
+        // proves the flag is opt-in and PreDeploy semantics are unchanged.
+        File.WriteAllText(Path.Combine(_workDir, "PreDeploy.sh"), "echo hi");
+        var step = new ConventionScriptStep(ConventionScriptNames.PreDeploy, new StubScriptEngine());
+        var ctx = BuildContext();
+        ctx.ScriptResult = new ScriptExecutionResult(9);
+        ctx.ExecutionFailed = true;
+
+        step.IsEnabled(ctx).ShouldBeTrue();
+    }
+
+    // ── DeployHasFailed predicate (single source of truth) ───────────────────
+
+    [Theory]
+    [InlineData(false, null, false)]   // no throw, main not run → not failed
+    [InlineData(false, 0, false)]      // main exit 0 → not failed
+    [InlineData(false, 1, true)]       // main exit non-zero → failed
+    [InlineData(true, null, true)]     // upstream threw → failed
+    [InlineData(true, 0, true)]        // threw wins even if a result exists
+    public void DeployHasFailed_Predicate(bool executionFailed, int? exitCode, bool expected)
+    {
+        var ctx = BuildContext();
+        ctx.ExecutionFailed = executionFailed;
+        ctx.ScriptResult = exitCode is null ? null : new ScriptExecutionResult(exitCode.Value);
+
+        ctx.DeployHasFailed.ShouldBe(expected);
+    }
+
     // ── Execute happy path ──────────────────────────────────────────────────
 
     [Fact]

tests/Squid.Calamari.Tests/Calamari/Commands/RunScriptCommandTests.cs

@@ -42,4 +42,122 @@ public async Task ExecuteWithResultAsync_ReturnsExitCode_And_OutputVariables()
                 Directory.Delete(tempDir, recursive: true);
         }
     }
+
+    [Fact]
+    public async Task MainScriptExitsNonZero_SkipsPostDeploy_RunsDeployFailed()
+    {
+        // The core fix: a non-zero main-script exit is a failed deploy. PostDeploy
+        // (smoke tests / traffic switch) MUST NOT run against a failed deploy, and
+        // the DeployFailed hook MUST run. Real bash, real convention scripts.
+        if (OperatingSystem.IsWindows())
+            return;
+
+        var tempDir = Path.Combine(Path.GetTempPath(), "squid-calamari-cmd-" + Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(tempDir);
+
+        var postDeploySentinel = Path.Combine(tempDir, "postdeploy-ran.txt");
+        var deployFailedSentinel = Path.Combine(tempDir, "deployfailed-ran.txt");
+
+        try
+        {
+            File.WriteAllText(Path.Combine(tempDir, "command.sh"), "echo main-running\nexit 3\n");
+            File.WriteAllText(Path.Combine(tempDir, "PostDeploy.sh"), $"echo ran > '{postDeploySentinel}'\n");
+            File.WriteAllText(Path.Combine(tempDir, "DeployFailed.sh"), $"echo ran > '{deployFailedSentinel}'\n");
+
+            var result = await new RunScriptCommand().ExecuteWithResultAsync(
+                Path.Combine(tempDir, "command.sh"), Path.Combine(tempDir, "variables.json"), null, null, CancellationToken.None);
+
+            result.ExitCode.ShouldBe(3);
+            result.Succeeded.ShouldBeFalse();
+
+            File.Exists(postDeploySentinel).ShouldBeFalse(
+                customMessage: "PostDeploy ran against a FAILED main script (exit 3). A smoke test / traffic " +
+                               "switch must not run on a failed deploy — inspect ConventionScriptStep gating.");
+            File.Exists(deployFailedSentinel).ShouldBeTrue(
+                customMessage: "DeployFailed did NOT run after a non-zero main-script exit. The failure hook " +
+                               "must fire so operators can alert / capture forensics.");
+        }
+        finally
+        {
+            if (Directory.Exists(tempDir))
+                Directory.Delete(tempDir, recursive: true);
+        }
+    }
+
+    [Fact]
+    public async Task MainScriptSucceeds_RunsPostDeploy_SkipsDeployFailed()
+    {
+        // Regression guard for the happy path: a successful main script still runs
+        // PostDeploy and must NOT trigger DeployFailed.
+        if (OperatingSystem.IsWindows())
+            return;
+
+        var tempDir = Path.Combine(Path.GetTempPath(), "squid-calamari-cmd-" + Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(tempDir);
+
+        var postDeploySentinel = Path.Combine(tempDir, "postdeploy-ran.txt");
+        var deployFailedSentinel = Path.Combine(tempDir, "deployfailed-ran.txt");
+
+        try
+        {
+            File.WriteAllText(Path.Combine(tempDir, "command.sh"), "echo main-running\nexit 0\n");
+            File.WriteAllText(Path.Combine(tempDir, "PostDeploy.sh"), $"echo ran > '{postDeploySentinel}'\n");
+            File.WriteAllText(Path.Combine(tempDir, "DeployFailed.sh"), $"echo ran > '{deployFailedSentinel}'\n");
+
+            var result = await new RunScriptCommand().ExecuteWithResultAsync(
+                Path.Combine(tempDir, "command.sh"), Path.Combine(tempDir, "variables.json"), null, null, CancellationToken.None);
+
+            result.ExitCode.ShouldBe(0);
+            result.Succeeded.ShouldBeTrue();
+
+            File.Exists(postDeploySentinel).ShouldBeTrue(
+                customMessage: "PostDeploy did NOT run after a successful main script — the gate over-skipped.");
+            File.Exists(deployFailedSentinel).ShouldBeFalse(
+                customMessage: "DeployFailed ran on a SUCCESSFUL deploy — the failure predicate is wrong.");
+        }
+        finally
+        {
+            if (Directory.Exists(tempDir))
+                Directory.Delete(tempDir, recursive: true);
+        }
+    }
+
+    [Fact]
+    public async Task PreDeployThrows_SkipsPostDeploy_RunsDeployFailed()
+    {
+        // The other failure signal (ExecutionFailed via a thrown convention step): a
+        // non-zero PreDeploy aborts the deploy before the main script. PostDeploy must not
+        // run; DeployFailed must run. The command re-throws the original PreDeploy failure.
+        if (OperatingSystem.IsWindows())
+            return;
+
+        var tempDir = Path.Combine(Path.GetTempPath(), "squid-calamari-cmd-" + Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(tempDir);
+
+        var postDeploySentinel = Path.Combine(tempDir, "postdeploy-ran.txt");
+        var deployFailedSentinel = Path.Combine(tempDir, "deployfailed-ran.txt");
+
+        try
+        {
+            File.WriteAllText(Path.Combine(tempDir, "command.sh"), "echo main-running\nexit 0\n");
+            File.WriteAllText(Path.Combine(tempDir, "PreDeploy.sh"), "echo pre-failing\nexit 4\n");
+            File.WriteAllText(Path.Combine(tempDir, "PostDeploy.sh"), $"echo ran > '{postDeploySentinel}'\n");
+            File.WriteAllText(Path.Combine(tempDir, "DeployFailed.sh"), $"echo ran > '{deployFailedSentinel}'\n");
+
+            await Should.ThrowAsync<InvalidOperationException>(() => new RunScriptCommand().ExecuteWithResultAsync(
+                Path.Combine(tempDir, "command.sh"), Path.Combine(tempDir, "variables.json"), null, null, CancellationToken.None));
+
+            File.Exists(postDeploySentinel).ShouldBeFalse(
+                customMessage: "PostDeploy ran after PreDeploy threw — a smoke test must not run when the deploy " +
+                               "aborted before the main script.");
+            File.Exists(deployFailedSentinel).ShouldBeTrue(
+                customMessage: "DeployFailed did NOT run after a PreDeploy exception — the cleanup-phase failure " +
+                               "hook must fire on the throw path too, not only on a non-zero main-script exit.");
+        }
+        finally
+        {
+            if (Directory.Exists(tempDir))
+                Directory.Delete(tempDir, recursive: true);
+        }
+    }
 }