fix(generate): include a dependency's retry shims in a dependent's needs

docs/src/content/docs/internals/coverage-matrix.md

@@ -108,7 +108,7 @@ only under real installation tokens on the fleet, never in the token-free harnes
 | Least-privilege permissions | `orchestrate/least-privilege-permissions`, `09-single-env-repo` | least-priv posture (callbacks); gen-time wiring (3env); single-env release posture (single-env) | `internal/generate` | Permissions are scoped to the job that needs them, not the top level |
 | OIDC id-token propagation | `orchestrate/callback-permissions-oidc` | OIDC posture at callee (callbacks); gen-time `id-token: write` scoped (3env) | `internal/generate` | `id-token: write` propagates to the caller job without leaking workflow-wide |
 | Callback dependency ordering (`depends_on`) | `11-job-timeouts-and-optional-deps` | needs ordering (callbacks); base to app order (3env gen-time) | `internal/generate` | A dependent callback starts only after its prerequisite concludes |
-| Callback retry wrapper | | retry-wrapper jobs present (callbacks); retry shim jobs (3env gen-time) | `internal/generate` | The retry jobs are emitted and wired for `retries: N` |
+| Callback retry wrapper | `73-deploy-retries` | retry-wrapper jobs present (callbacks); retry shim jobs (3env gen-time) | `internal/generate` | The retry jobs are emitted and wired for `retries: N`, including a dependent deploy's `needs:` carrying its dependency's retry shims |
 | Signed auto-commit identity (`auto_commits`) | `03-three-env-repo` | auto_commits author and message (3env) | `internal/promote/auto_commit_sha*.go` | The state commit carries the configured author and message |
 
 See [the `auto_commits` field](/cascade/reference/manifest/) in the manifest reference for what a callback must do to trigger this capture.

e2e/scenarios/73-deploy-retries.yaml

@@ -14,11 +14,20 @@ description: |
 
  Step 1 (emitted): the shims exist, there are exactly two of them, and each is
  gated on its PREDECESSOR's failure rather than all hanging off the original
- job. That is a text assertion and is labeled as such.
+ job. That is a text assertion and is labeled as such. The same step also
+ covers a DEPENDENT deploy (notify, depends_on: web): its needs: list must
+ carry the base dependency plus every retry shim, matching the shims its own
+ if: gate already reads. A shim referenced by if: but missing from needs: is
+ both an actionlint parse error and, were it ever accepted, an always-empty
+ read at runtime, silently skipping the dependent on a rescue.
 
  Step 2 (executing): the staged deploy callback always exits non-zero, so
  deploy-web genuinely fails at runtime, and expect_log proves the callback BODY
- really ran rather than a job object merely existing.
+ really ran rather than a job object merely existing. Since no attempt in the
+ ladder ever succeeds, deploy-notify's default run_policy (require success)
+ correctly skips it: this is the non-rescue path, and it is unchanged by the
+ needs:-list fix (a dependent job that is going to skip regardless is not where
+ the defect was observable).
 
  What is NOT asserted, and why: the individual retry shims are not observable
  through expect.jobs. The harness keys jobs by the reusable workflow's INNER
@@ -31,7 +40,7 @@ description: |
 
  WHY base-fail-then-retry-SUCCEED IS NOT EXERCISED HERE, for the next author:
  the case that matters most (the base attempt fails, a shim re-runs and
- SUCCEEDS, so the environment really is deployed) cannot be expressed by this
+ SUCCEEDS, so the dependent deploy actually RUNS) cannot be expressed by this
  harness, and the gap is structural rather than an omission:
 
  1. Each shim is a separate GitHub Actions job, so it gets its own container
@@ -47,11 +56,17 @@ description: |
 
  A callback that fails once and then succeeds therefore has no way to keep state
  across attempts here. Rather than fabricate a scenario that cannot prove the
- claim, the runtime half is left unexercised and the effective-result gates are
- pinned as emitted text above, with the four ladder shapes (no shims, base
- succeeds, a middle shim succeeds, all attempts fail) covered as unit tests in
- internal/generate/effective_result_test.go. Restoring the executing proof needs
- an artifact server in the harness, not a change to this scenario.
+ claim, the rescue-runtime half is left unexercised and instead pinned as unit
+ tests: internal/generate/pass10_silent_output_test.go
+ (TestGM7_DependentDeploy_NeedsIncludesRetryShims proves the needs:/if:
+ agreement this scenario also checks in emitted text;
+ TestGM5_DependentDeploy_JudgesEffectiveResult proves the if: gate itself reads
+ the ladder's effective result). The actionlint feature-matrix guard
+ (internal/generate/actionlint_feature_matrix_test.go, case
+ retries_dependent_deploy) additionally proves real GitHub would accept the
+ emitted needs:/if: pair at parse for this exact combination, permanently.
+ Restoring the executing rescue proof needs an artifact server in the harness,
+ not a change to this scenario.
 
  INNER-JOB-ID NAMESPACE TRAP, for the next author: expect.jobs entries are
  resolved by findJob (assert.go), which strips a "build-"/"deploy-" prefix and
@@ -61,7 +76,8 @@ description: |
  silently return the BUILD's result. That is why the build here is "app" and the
  deploy is "web", and why the staged callback's inner job id is "web" to match
  the deploy name. Keep build and deploy names distinct in any scenario that
- asserts expect.jobs on them.
+ asserts expect.jobs on them. notify uses its own stub file (notify.yaml) with
+ inner job id "notify" for the same reason.
 
 config:
  trunk_branch: main
@@ -75,6 +91,10 @@ config:
  workflow: deploy.yaml
  triggers: ["src/**"]
  retries: 2
+ - name: notify
+ workflow: notify.yaml
+ triggers: ["src/**"]
+ depends_on: ["deploy:web"]
 
 # The deploy callback always fails, so the original job and both retry shims each
 # re-invoke it and conclude in failure. Its inner job id is "web" so it matches
@@ -98,6 +118,25 @@ setup_workflows:
  - run: |
  echo "cascade-e2e-retry-probe: deploy callback invoked, failing on purpose"
  exit 1
+ # notify's dependency (web) never succeeds in this scenario, so notify never
+ # runs; its body only needs to exist for the workflow set to be valid.
+ ".github/workflows/notify.yaml": |
+ name: notify
+ on:
+ workflow_call:
+ inputs:
+ environment:
+ required: false
+ type: string
+ sha:
+ required: false
+ type: string
+ jobs:
+ notify:
+ runs-on: ubuntu-latest
+ steps:
+ - run: |
+ echo "cascade-e2e-retry-probe: notify callback invoked"
 
 steps:
  - name: "Seed source; assert the retry shims are emitted and chained"
@@ -128,6 +167,11 @@ steps:
  # the note below), so the expressions are pinned as emitted text.
  - "WEB_RESULT: ${{ (needs.deploy-web.result == 'success' || needs.deploy-web-retry-1.result == 'success' || needs.deploy-web-retry-2.result == 'success') && 'success' || 'failure' }}"
  - "!(needs.deploy-web-retry-1.result == 'success' || needs.deploy-web-retry-2.result == 'success')"
+ # notify (deploys[].depends_on: web) must list every one of web's
+ # retry shims in its own needs:, not only the base job, so the shims
+ # its if: gate reads below are actually resolvable references.
+ - "needs: [setup, deploy-web, deploy-web-retry-1, deploy-web-retry-2]"
+ - "(needs.deploy-web.result == 'success' || needs.deploy-web-retry-1.result == 'success' || needs.deploy-web-retry-2.result == 'success')"
  not_contains:
  # retries: 2 emits exactly two shims.
  - "deploy-web-retry-3:"
@@ -145,5 +189,11 @@ steps:
  # so neither reads the other's result.
  build-app: success
  deploy-web: failure
+ # web never succeeds in this scenario (base nor either shim), so notify's
+ # default run_policy (require success) correctly skips it. This is the
+ # non-rescue path; see the description above for why the rescue path
+ # (which is what the needs:-list fix actually protects) cannot be
+ # exercised in this harness.
+ deploy-notify: skipped
  # Proves the callback BODY executed, not merely that a job object exists.
  expect_log: "cascade-e2e-retry-probe: deploy callback invoked"

internal/generate/actionlint_feature_matrix_test.go

@@ -455,6 +455,26 @@ deploys:
  - name: cdk
  workflow: deploy.yaml
  triggers: ["cdk/**"]
+`),
+ stubs: contractStubs(),
+ },
+ {
+ // Coverage-gap closer: the "retries" case above never exercises a
+ // dependent deploy of a retried job, so it could not have caught the
+ // defect where a dependent's needs: omitted the retry shims its own
+ // if: gate referenced (a shim outside needs: is unresolvable, both at
+ // actionlint parse time and at GHA runtime). This case pairs retries
+ // with depends_on so that combination is under permanent guard.
+ name: "retries_dependent_deploy",
+ manifest: wrap(base + `deploys:
+ - name: app
+ workflow: deploy.yaml
+ triggers: ["src/**"]
+ retries: 1
+ - name: notify
+ workflow: deploy.yaml
+ triggers: ["notify/**"]
+ depends_on: ["deploy:app"]
 `),
  stubs: contractStubs(),
  },

internal/generate/correctness_census_map_test.go

@@ -72,7 +72,7 @@ var correctnessCensus = map[string]correctnessCoverage{
 	"deploys[].secrets.map.*": {marker: markerStructural, note: "shared writeSecretsBlock, pinned by TestGenCorrectness_SecretsMap_PropagatesSourceToCallee"},
 	"deploys[].triggers[]": {marker: markerValidityOnly, note: "paths-filter glob; validity + round-trip is the contract"},
 	"deploys[].workflow": {marker: markerValidityOnly, note: "callback path spliced into uses:; validity + round-trip is the contract"},
-	"deploys[].depends_on[]": {assertion: "TestGM5_DependentDeploy_JudgesEffectiveResult"},
+	"deploys[].depends_on[]": {assertion: "TestGM7_DependentDeploy_NeedsIncludesRetryShims"},
 	"deploys[].optional_depends_on[]": {assertion: "TestGenCorrectness_DependsOn_SequencesButOnlyRequiredGates"},
 	"deploys[].env_inputs[key]": {marker: markerValidityOnly, note: "environment reference key; validity + round-trip is the contract"},
 	"deploys[].env_inputs.*": {assertion: "TestPromoteGenerator_UnresolvedEnvStateRefStaysVisible"},

internal/generate/generator.go

@@ -1157,9 +1157,21 @@ func (g *Generator) writeCallbackJob(sb *strings.Builder, info CallbackInfo, wor
 	// IDs). optional_depends_on adds ordering-only edges: they go into needs: so
 	// this job waits for them, but they are excluded from the if: skip-gate below
 	// (#18) so a skipped optional dep does not skip this job.
+	//
+	// A hard dependency that declares retries also needs its retry shim job IDs
+	// appended here: the if: gate below (via effectiveDepSuccessGate /
+	// retrySucceededCond) reads needs.<dep>-retry-N.result to judge the
+	// dependency's effective (ladder-wide) result, and GitHub Actions can only
+	// resolve a needs.<job> reference for a job actually listed in needs:. A
+	// shim missing from needs: is both an actionlint parse error and, were it
+	// somehow accepted, an always-empty read at runtime, so a retry-rescued
+	// dependency would never unblock this job.
 	hardDeps := g.graph.GetDirectDependencies(info.JobID)
 	needs := []string{"setup"}
-	needs = append(needs, hardDeps...)
+	for _, dep := range hardDeps {
+ needs = append(needs, dep)
+ needs = append(needs, retryShimJobIDs(dep, g.graph.Nodes[dep].Retries)...)
+	}
 	needs = append(needs, g.graph.GetOptionalDependencies(info.JobID)...)
 	// When a pre-download job was emitted, make the callback depend on it so the
 	// downloaded artifacts are available in the runner's workspace.
@@ -1606,9 +1618,7 @@ func (g *Generator) writeFinalizeJob(sb *strings.Builder, sorted []string) {
 	for _, jobID := range sorted {
  info := g.graph.Nodes[jobID]
  allJobs = append(allJobs, jobID)
- for i := 1; i <= info.Retries; i++ {
- allJobs = append(allJobs, fmt.Sprintf("%s-retry-%d", jobID, i))
- }
+ allJobs = append(allJobs, retryShimJobIDs(jobID, info.Retries)...)
 	}
 	// The custom changelog runs as its own job; finalize consumes its output,
 	// so it must be in finalize's needs:.
@@ -2099,17 +2109,36 @@ func failureOrCancelledCond(jobName string) string {
 	return fmt.Sprintf("contains(fromJSON('[\"failure\", \"cancelled\"]'), needs.%s.result)", jobName)
 }
 
+// retryShimJobIDs returns a job's retry shim job IDs in ladder order
+// (<jobID>-retry-1 .. <jobID>-retry-N), or nil when retries is zero. It is the
+// single source of truth for a job's shim IDs: every place that either
+// references a shim's result (retrySucceededCond) or must list a shim in a
+// needs: block (writeCallbackJob, writeFinalizeJob) derives from this function
+// so the two can never drift apart again, the way they did before this fix
+// (the if: gate referenced a shim that needs: never listed).
+func retryShimJobIDs(jobID string, retries int) []string {
+	if retries <= 0 {
+ return nil
+	}
+	shims := make([]string, 0, retries)
+	for i := 1; i <= retries; i++ {
+ shims = append(shims, fmt.Sprintf("%s-retry-%d", jobID, i))
+	}
+	return shims
+}
+
 // retrySucceededCond builds the "some shim rescued it" half of a ladder's
 // effective result: a disjunction over each retry shim's success. It returns
 // the empty string when the callback declares no retries, which is what lets
 // the effective-result helpers collapse to their pre-retry form.
 func retrySucceededCond(jobName string, retries int) string {
-	if retries <= 0 {
+	shims := retryShimJobIDs(jobName, retries)
+	if len(shims) == 0 {
  return ""
 	}
-	conds := make([]string, 0, retries)
-	for i := 1; i <= retries; i++ {
- conds = append(conds, fmt.Sprintf("needs.%s-retry-%d.result == 'success'", jobName, i))
+	conds := make([]string, 0, len(shims))
+	for _, shim := range shims {
+ conds = append(conds, fmt.Sprintf("needs.%s.result == 'success'", shim))
 	}
 	return strings.Join(conds, " || ")
 }

internal/generate/pass10_silent_output_test.go

@@ -231,6 +231,58 @@ func TestGM5_DependentDeploy_NoRetries_SingleClause(t *testing.T) {
 	assert.NotContains(t, block, "retry", "no retry shims exist for a zero-retry dependency")
 }
 
+// TestGM7_DependentDeploy_NeedsIncludesRetryShims proves a dependent deploy's
+// needs: list carries every retry shim job ID its dependency declares, not just
+// the base dependency's job ID.
+//
+// The if: gate (effectiveDepSuccessGate, proven by TestGM5 above) already
+// references needs.deploy-web-retry-1 / -2 so a retry-rescued dependency does
+// not skip its dependents. But GitHub Actions can only resolve a needs.<job>
+// reference for a job actually listed in that job's needs:; a reference to a
+// job outside needs: is rejected by actionlint at parse and, if it somehow ran,
+// would resolve to an ever-empty value at runtime. Before this fix needs: was
+// built from GetDirectDependencies alone (the base job ID only), so the if:
+// gate and the needs: list silently disagreed.
+func TestGM7_DependentDeploy_NeedsIncludesRetryShims(t *testing.T) {
+	dir := pass10Fixture(t, pass10DeployWorkflow)
+	cfg := &config.TrunkConfig{
+ TrunkBranch: "main",
+ Environments: config.EnvNames("dev"),
+ Deploys: []config.DeployConfig{
+ {Name: "web", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, Retries: 2},
+ {Name: "api", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, DependsOn: []string{"web"}},
+ },
+	}
+	out, err := NewGenerator(cfg, dir).Generate()
+	require.NoError(t, err)
+
+	block := pass10JobBlock(t, out, "deploy-api")
+	assert.Contains(t, block, "needs: [setup, deploy-web, deploy-web-retry-1, deploy-web-retry-2]",
+ "needs: must list the base dependency plus every retry shim it declares, "+
+ "in ladder order, so the if: gate referencing those shims is well-formed")
+}
+
+// TestGM7_DependentDeploy_NoRetries_NeedsByteIdentical proves the no-retries
+// path is unchanged: a dependent of a deploy with no retries emits the bare
+// needs: list with no phantom shim references.
+func TestGM7_DependentDeploy_NoRetries_NeedsByteIdentical(t *testing.T) {
+	dir := pass10Fixture(t, pass10DeployWorkflow)
+	cfg := &config.TrunkConfig{
+ TrunkBranch: "main",
+ Environments: config.EnvNames("dev"),
+ Deploys: []config.DeployConfig{
+ {Name: "web", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}},
+ {Name: "api", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, DependsOn: []string{"web"}},
+ },
+	}
+	out, err := NewGenerator(cfg, dir).Generate()
+	require.NoError(t, err)
+	block := pass10JobBlock(t, out, "deploy-api")
+	assert.Contains(t, block, "needs: [setup, deploy-web]",
+ "a no-retry dependency must emit the bare needs: list with no shim references")
+	assert.NotContains(t, block, "retry", "no retry shims exist for a zero-retry dependency")
+}
+
 // TestGM6_PromoteNativeDeployment_GuardsDryRunAndCountsSkips proves a dry-run
 // promote does not create a real GitHub Deployment, and that the terminal status
 // counts a legitimately skipped deploy as success and includes the prod deploy.