fix(generate): close four silent generated-output defects

CHANGELOG.md

@@ -16,6 +16,30 @@ A `Migration` section is added to any release that bumps `schema_version`.
 
 ### Fixed
 
+- **promote:** A matrix-based promote deploy (a deploy declaring inputs) now
+  threads the per-promotion environment and sha to its reusable-workflow
+  callback, matching orchestrate. The matrix path previously emitted only the
+  declared manifest inputs, so the callback ran with an empty environment while
+  the job name referenced `${{ matrix.environment }}`, a key the matrix builder
+  never set. environment and sha are added to every matrix entry and passed in
+  the `with:` block (sha only when the callback declares it, and neither when the
+  manifest already wires it as an explicit input).
+- **rollback:** The repository_dispatch dry-run guard now treats a JSON boolean
+  `true` from `client_payload` and the string `'true'` from a workflow_dispatch
+  input alike. A bare `!= 'true'` compared a boolean against a string, which
+  GitHub Actions coerces numerically, so a natural `{"dry_run": true}` payload
+  read as not-a-dry-run and a dry-run rollback ran real deploys and wrote
+  rolled-back state. Without the trigger the output is unchanged.
+- **generate:** A dependent deploy is now gated on the base deploy's effective
+  result (the base job or any retry shim succeeded), so a base deploy rescued by
+  a retry no longer skips the deploys that depend on it. The condition was
+  reading the base job's immutable `result`, frozen at `failure` after a rescued
+  attempt, and a deploy-on-deploy dependency also emitted the clause twice.
+- **promote:** A dry-run promote no longer creates a real GitHub Deployment, and
+  the Deployment's terminal status no longer counts a legitimately skipped deploy
+  as a failure or omits the prod deploy result. The native-deployment lifecycle
+  steps are gated on a non-dry-run run, and the status expression judges each
+  deploy (including its prod job) on success-or-skipped.
 - **state:** A single-component (flat) state write on a manifest that carries
   per-component state no longer destroys that state. The flat state map models
   environments only, so parsing a component-scoped manifest lifted

e2e/scenarios/31-native-deployments.yaml

@@ -49,13 +49,15 @@ steps:
           contains:
             - "  deployments: write"
             - "      - name: Create deployment"
-            - "        if: ${{ github.server_url == 'https://github.com' }}"
+            # A dry-run run skips its deploys, so it must not create a real
+            # Deployment: every lifecycle step is also gated on a non-dry-run run.
+            - "        if: ${{ github.server_url == 'https://github.com' && github.event.inputs.dry_run != 'true' }}"
             - "          deployment_id=$(gh api repos/${{ github.repository }}/deployments \\"
             - "            --field auto_inactive=false \\"
             - "      - name: Set deployment in_progress"
             - "            --field state=in_progress"
             - "      - name: Set deployment status"
-            - "        if: ${{ github.server_url == 'https://github.com' && always() }}"
+            - "        if: ${{ github.server_url == 'https://github.com' && github.event.inputs.dry_run != 'true' && always() }}"
             - "            production) environment_url='https://app.example.com' ;;"
 
   - name: "Regenerate and confirm no drift"

e2e/scenarios/32-rollback-repository-dispatch.yaml

@@ -59,6 +59,12 @@ steps:
             # deploy guard and finalize gate coalesce dry_run and deployable.
             - "github.event.inputs.dry_run || github.event.client_payload.dry_run"
             - "github.event.inputs.deployable || github.event.client_payload.deployable"
+            # The dry_run guard matches both a JSON boolean true (client_payload)
+            # and the string 'true' (workflow_dispatch input): a natural
+            # {"dry_run": true} payload sends a boolean, and a bare "!= 'true'"
+            # would read it as not-a-dry-run and run real deploys.
+            - "(github.event.inputs.dry_run || github.event.client_payload.dry_run) != true"
+            - "(github.event.inputs.dry_run || github.event.client_payload.dry_run) != 'true'"
           not_contains:
             # the bare, un-coalesced reads must be gone once the toggle is on.
             - "ENVIRONMENT: ${{ github.event.inputs.environment }}"

e2e/scenarios/43-deploy-rollout-strategy.yaml

@@ -72,3 +72,8 @@ steps:
           contains:
             - "fail-fast: true"
             - "max-parallel: 2"
+            # The matrix deploy must thread the per-promotion environment and sha
+            # to its callback (the way orchestrate does), or the deploy targets an
+            # empty environment while its job name references matrix.environment.
+            - "environment: ${{ matrix.environment }}"
+            - "sha: ${{ matrix.sha }}"

internal/generate/generator.go

@@ -1264,6 +1264,9 @@ func (g *Generator) writeStrategyBlock(sb *strings.Builder, m *config.MatrixConf
 func (g *Generator) writeIfCondition(sb *strings.Builder, info CallbackInfo, needs []string) {
 	var conditions []string
 	buildLinkedDeploy := false
+	// Dependencies already emitted by the build-linked path below, so the
+	// general dependency loop does not emit a second, duplicate clause for them.
+	linkedDeps := make(map[string]struct{})
 
 	// For deploys with depends_on build, check if build ran successfully
 	// instead of using setup detection
@@ -1278,14 +1281,20 @@ func (g *Generator) writeIfCondition(sb *strings.Builder, info CallbackInfo, nee
 					if err != nil {
 						continue
 					}
-					// Apply run_policy to build dependency check
+					// Record so the general dependency loop below does not emit
+					// a second, duplicate clause for the same dependency.
+					linkedDeps[depJobID] = struct{}{}
+					depRetries := g.graph.Nodes[depJobID].Retries
+					// Apply run_policy to the dependency check, judged on the
+					// dependency's effective result (base OR any retry shim
+					// succeeded) so a rescued deploy does not skip its dependents.
 					switch info.RunPolicy {
 					case config.RunPolicyAlways:
-						conditions = append(conditions, fmt.Sprintf("(needs.%s.result == 'success' || needs.%s.result == 'skipped')", depJobID, depJobID))
+						conditions = append(conditions, fmt.Sprintf("(%s || needs.%s.result == 'skipped')", effectiveSuccessCond(depJobID, depRetries), depJobID))
 					case config.RunPolicyForce:
 						// No condition needed for force
 					default:
-						conditions = append(conditions, fmt.Sprintf("needs.%s.result == 'success'", depJobID))
+						conditions = append(conditions, effectiveDepSuccessGate(depJobID, depRetries))
 					}
 				}
 				break
@@ -1309,12 +1318,17 @@ func (g *Generator) writeIfCondition(sb *strings.Builder, info CallbackInfo, nee
 		if buildLinkedDeploy && depInfo.Type == config.CallbackTypeBuild {
 			continue
 		}
+		// Skip any dependency the build-linked path already emitted, so a
+		// deploy-on-deploy dependency is not gated by a duplicated clause.
+		if _, done := linkedDeps[depJobID]; done {
+			continue
+		}
 
 		switch info.RunPolicy {
 		case config.RunPolicyDefault, "":
-			conditions = append(conditions, fmt.Sprintf("needs.%s.result == 'success'", depJobID))
+			conditions = append(conditions, effectiveDepSuccessGate(depJobID, depInfo.Retries))
 		case config.RunPolicyAlways:
-			conditions = append(conditions, fmt.Sprintf("(needs.%s.result == 'success' || needs.%s.result == 'skipped')", depJobID, depJobID))
+			conditions = append(conditions, fmt.Sprintf("(%s || needs.%s.result == 'skipped')", effectiveSuccessCond(depJobID, depInfo.Retries), depJobID))
 		case config.RunPolicyForce:
 			// No dependency condition
 		}
@@ -1733,7 +1747,11 @@ func (g *Generator) writeNativeDeploymentSteps(sb *strings.Builder, sorted []str
 		resultExpr = fmt.Sprintf("${{ (%s) && 'success' || 'failure' }}", strings.Join(conds, " && "))
 	}
 
-	writeNativeDeploymentSteps(sb, g.config, envExpr, resultExpr, "      ")
+	// A dry-run orchestrate skips its deploy callbacks, so it must not create a
+	// real GitHub Deployment either. github.event.inputs.dry_run is null-safe on
+	// the non-dispatch triggers (push/schedule/workflow_run), where it renders
+	// empty and reads as not-a-dry-run.
+	writeNativeDeploymentSteps(sb, g.config, envExpr, resultExpr, "github.event.inputs.dry_run != 'true'", "      ")
 }
 
 func (g *Generator) writeSummaryStep(sb *strings.Builder, sorted []string) {
@@ -2118,6 +2136,22 @@ func effectiveSuccessCond(jobName string, retries int) string {
 	return cond
 }
 
+// effectiveDepSuccessGate renders the "dependency satisfied" clause for a
+// downstream job's if: condition, judged on the dependency's effective result so
+// a base deploy that failed but was rescued by a retry shim still lets its
+// dependents run. It reuses effectiveSuccessCond (the shared retry-aware helper)
+// and parenthesizes the disjunction only when a retry ladder is present, since
+// callers join these clauses with " && "; with no retries it collapses to the
+// bare needs.<job>.result == 'success', so a manifest without retries emits
+// byte-identical output.
+func effectiveDepSuccessGate(jobName string, retries int) string {
+	cond := effectiveSuccessCond(jobName, retries)
+	if retries > 0 {
+		return "(" + cond + ")"
+	}
+	return cond
+}
+
 // effectiveResultExpr renders a callback's effective result as a ${{ }}
 // expression evaluating to the string 'success' or 'failure', suitable for an
 // env: value that shell then compares against "success".

internal/generate/native_deployments.go

@@ -31,20 +31,33 @@ func deploymentAutoInactive(cfg *config.TrunkConfig) bool {
 // envExpr is the shell-safe expression that resolves to the target environment
 // name at run time (it differs between the orchestrate and promote seams).
 // resultExpr is the shell expression that evaluates to "success" or "failure"
-// for the deploy outcome. indent is the per-step indent (matching the
-// surrounding generated YAML).
-func writeNativeDeploymentSteps(sb *strings.Builder, cfg *config.TrunkConfig, envExpr, resultExpr, indent string) {
+// for the deploy outcome. dryRunGuard is the expression body (no ${{ }} wrapper)
+// that is true when the run is NOT a dry run; a dry run must never create a real
+// GitHub Deployment, so it is ANDed into every step's if:. An empty dryRunGuard
+// leaves the steps ungated (no dry-run concept for that seam). indent is the
+// per-step indent (matching the surrounding generated YAML).
+func writeNativeDeploymentSteps(sb *strings.Builder, cfg *config.TrunkConfig, envExpr, resultExpr, dryRunGuard, indent string) {
 	if !nativeDeploymentsEnabled(cfg) {
 		return
 	}
 
 	body := indent + "  "
 
+	// Compose the server guard with the non-dry-run guard so a dry run neither
+	// creates the Deployment nor posts status against a Deployment that was never
+	// created. serverGuard is the bare comparison (no ${{ }} wrapper) so it can
+	// be joined with the other clauses inside a single expression.
+	serverGuard := stripTokenExprWrapper(appTokenServerGuard)
+	createGuard := "${{ " + serverGuard + " }}"
+	if dryRunGuard != "" {
+		createGuard = "${{ " + serverGuard + " && " + dryRunGuard + " }}"
+	}
+
 	// Create the Deployment. The environment name is resolved at run time, so the
 	// URL lookup is a shell case over the configured environment_config entries.
 	sb.WriteString(indent + "- name: Create deployment\n")
 	sb.WriteString(body + "id: cascade-deployment\n")
-	sb.WriteString(body + "if: " + appTokenServerGuard + "\n")
+	sb.WriteString(body + "if: " + createGuard + "\n")
 	sb.WriteString(body + "env:\n")
 	sb.WriteString(body + "  GH_TOKEN: ${{ github.token }}\n")
 	sb.WriteString(body + "run: |\n")
@@ -61,7 +74,7 @@ func writeNativeDeploymentSteps(sb *strings.Builder, cfg *config.TrunkConfig, en
 
 	// Mark the deployment in_progress.
 	sb.WriteString(indent + "- name: Set deployment in_progress\n")
-	sb.WriteString(body + "if: " + appTokenServerGuard + "\n")
+	sb.WriteString(body + "if: " + createGuard + "\n")
 	sb.WriteString(body + "env:\n")
 	sb.WriteString(body + "  GH_TOKEN: ${{ github.token }}\n")
 	sb.WriteString(body + "run: |\n")
@@ -72,8 +85,12 @@ func writeNativeDeploymentSteps(sb *strings.Builder, cfg *config.TrunkConfig, en
 	// Report the terminal status. always() lets it run even when a deploy failed,
 	// so the Deployment never sticks at in_progress. The URL is selected by the
 	// runtime environment name from the configured environment_config entries.
+	statusGuard := serverGuard
+	if dryRunGuard != "" {
+		statusGuard += " && " + dryRunGuard
+	}
 	sb.WriteString(indent + "- name: Set deployment status\n")
-	sb.WriteString(body + "if: ${{ " + stripTokenExprWrapper(appTokenServerGuard) + " && always() }}\n")
+	sb.WriteString(body + "if: ${{ " + statusGuard + " && always() }}\n")
 	sb.WriteString(body + "env:\n")
 	sb.WriteString(body + "  GH_TOKEN: ${{ github.token }}\n")
 	sb.WriteString(body + "run: |\n")

internal/generate/native_deployments_test.go

@@ -32,7 +32,7 @@ on:
 	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/deploy.yaml"), []byte(deployWorkflow), 0o644))
 
 	cfg := &config.TrunkConfig{
-		TrunkBranch:  "main",
+		TrunkBranch: "main",
 		Environments: []config.EnvironmentEntry{
 			{Name: "production", EnvironmentConfig: config.EnvironmentConfig{EnvironmentURL: "https://app.example.com"}},
 		},
@@ -63,8 +63,12 @@ func TestNativeDeployments_Enabled(t *testing.T) {
 		"must POST to the deployment statuses collection")
 	assert.Contains(t, out, "https://app.example.com",
 		"the configured environment_url must be wired into the status update")
-	assert.Contains(t, out, appTokenServerGuard,
+	assert.Contains(t, out, "github.server_url == 'https://github.com'",
 		"deployment steps must be guarded to real GitHub via the server_url guard")
+	// A dry-run orchestrate skips its deploys, so it must not create a real
+	// Deployment: every lifecycle step is also gated on a non-dry-run run.
+	assert.Contains(t, out, "github.server_url == 'https://github.com' && github.event.inputs.dry_run != 'true'",
+		"deployment steps must not run on a dry-run orchestrate")
 	// The terminal status reflects the deploy job result, not a hardcoded value.
 	assert.Contains(t, out, "needs.deploy-app.result",
 		"terminal status must derive from the deploy job result")