fix: three generator-validity hazards (timeout on reusable callbacks, env-less rollback needs, unsafe job-ID names)

e2e/scenarios/06-callback-timeout.yaml

@@ -1,20 +1,22 @@
 name: "Callback Timeout"
 description: |
-  Verifies the timeout_minutes field on validate/builds/deploys propagates
-  into the generated orchestrate.yaml as a job-level timeout-minutes (#97).
+  Verifies the timeout_minutes field on an inline run: callback propagates into
+  the generated orchestrate.yaml as a job-level timeout-minutes (#97).
 
-  This is a generator-output verification scenario. Assertion runs on the
-  staged repo after StageRepoFromConfig generates workflows but before any
-  orchestrate runs. We don't run orchestrate because act doesn't honor
-  timeout-minutes on `uses:` reusable workflow callers and would fail the
-  workflow execution. Real timeout-firing belongs to real-GHA PoC validation.
+  timeout-minutes is only valid on a steps job, so the callback uses an inline
+  run:. GitHub rejects timeout-minutes on a reusable-workflow (uses:) caller job,
+  so for those callbacks the timeout must live inside the called workflow.
+
+  This is a generator-output verification scenario. Assertion runs on the staged
+  repo after StageRepoFromConfig generates workflows but before any orchestrate
+  runs. Real timeout-firing belongs to real-GHA PoC validation.
 
 config:
   trunk_branch: main
   environments: []
   builds:
     - name: app
-      workflow: build.yaml
+      run: "make build"
       triggers: ["src/**"]
       timeout_minutes: 30
   deploys: []

e2e/scenarios/11-job-timeouts-and-optional-deps.yaml

@@ -56,3 +56,10 @@ steps:
           not_contains:
             # #18: optional dep must NOT contribute a skip-gate condition.
             - "needs.build-migrations.result == 'success'"
+        # With environments: [] no deploy jobs are emitted in promote.yaml, so
+        # rollback jobs and finalize must not reference nonexistent deploy jobs.
+        - path: ".github/workflows/promote.yaml"
+          not_contains:
+            - "rollback-deploy-app:"
+            - "needs.deploy-deploy-app"
+            - "deploy-deploy-app-prod"

internal/config/parse.go

@@ -159,8 +159,11 @@ func Validate(cfg *TrunkConfig) []string {
 	// Empty environments is valid - means pre-release -> release only (no deployments)
 
 	envSet := make(map[string]bool)
-	for _, env := range cfg.Environments {
+	for i, env := range cfg.Environments {
 		envSet[env] = true
+		// Environment names key job IDs and ${{ }} expression references, so they
+		// must be job-ID-safe.
+		errors = append(errors, validateJobIDSafeName(fmt.Sprintf("environments[%d]", i), env)...)
 	}
 
 	// Build name sets for each section (builds and deploys can share names)
@@ -176,6 +179,9 @@ func Validate(cfg *TrunkConfig) []string {
 		} else {
 			buildNames[b.Name] = true
 		}
+		// The name becomes part of the job ID (build-<name>); enforce the job-ID
+		// grammar so generation cannot emit invalid YAML.
+		errors = append(errors, validateJobIDSafeName(fmt.Sprintf("builds[%d].name", i), b.Name)...)
 		// workflow XOR run: exactly one must be set.
 		errors = append(errors, validateWorkflowRunXOR(fmt.Sprintf("builds[%d]", i), b.Workflow, b.Run, b.Shell)...)
 		errors = append(errors, validateLocalCallbackWorkflowPath(fmt.Sprintf("builds[%d]", i), b.Workflow)...)
@@ -233,6 +239,9 @@ func Validate(cfg *TrunkConfig) []string {
 		} else {
 			deployNames[d.Name] = true
 		}
+		// The name becomes part of the job ID (deploy-<name>); enforce the job-ID
+		// grammar so generation cannot emit invalid YAML.
+		errors = append(errors, validateJobIDSafeName(fmt.Sprintf("deploys[%d].name", i), d.Name)...)
 		// workflow XOR run: exactly one must be set.
 		errors = append(errors, validateWorkflowRunXOR(fmt.Sprintf("deploys[%d]", i), d.Workflow, d.Run, d.Shell)...)
 		errors = append(errors, validateLocalCallbackWorkflowPath(fmt.Sprintf("deploys[%d]", i), d.Workflow)...)
@@ -352,6 +361,8 @@ func Validate(cfg *TrunkConfig) []string {
 					errors = append(errors, fmt.Sprintf("external deploy name '%s' conflicts with local deploy name", d.Name))
 				}
 			}
+			// External deploys also key job IDs (deploy-<name>); enforce the grammar.
+			errors = append(errors, validateJobIDSafeName(fmt.Sprintf("external[%d].deploys[%d].name", i, j), d.Name)...)
 			prefix := fmt.Sprintf("external[%d].deploys[%d]", i, j)
 			errors = append(errors, validateExternalDeployWorkflowOnly(prefix, d.Workflow, d.Run, d.Shell)...)
 			// External deploys are always reusable-workflow callbacks.

internal/config/parse_test.go

@@ -853,3 +853,111 @@ func TestValidate_ReleaseTag(t *testing.T) {
 		})
 	}
 }
+
+// TestValidate_JobIDSafeNames asserts that build, deploy, external-deploy, and
+// environment names which would become part of a GitHub Actions job ID
+// (build-<name>, deploy-<name>, and env-keyed identifiers) are rejected at
+// config validation when they contain characters outside the job-ID grammar.
+//
+// A GitHub job ID must start with a letter or _ and contain only [A-Za-z0-9_-].
+// Because the name is used as a suffix after build-/deploy-, a leading digit,
+// uppercase letters, and hyphens are all fine; only characters outside the
+// allowed set (such as ., spaces, and /) are rejected. Sanitizing is avoided
+// deliberately: two distinct names could collapse to one job ID.
+func TestValidate_JobIDSafeNames(t *testing.T) {
+	tests := []struct {
+		name     string
+		config   TrunkConfig
+		wantErr  string // substring that must appear; "" means expect no errors
+		wantNone bool
+	}{
+		{
+			name: "build name with dot rejected",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"dev"},
+				Builds:       []BuildConfig{{Name: "app.web", Workflow: ".github/workflows/build.yaml"}},
+			},
+			wantErr: `builds[0].name "app.web"`,
+		},
+		{
+			name: "build name with space rejected",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"dev"},
+				Builds:       []BuildConfig{{Name: "my app", Workflow: ".github/workflows/build.yaml"}},
+			},
+			wantErr: `builds[0].name "my app"`,
+		},
+		{
+			name: "deploy name with slash rejected",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"dev"},
+				Deploys:      []DeployConfig{{Name: "svc/api", Workflow: ".github/workflows/deploy.yaml"}},
+			},
+			wantErr: `deploys[0].name "svc/api"`,
+		},
+		{
+			name: "external deploy name with dot rejected",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"dev"},
+				External: []ExternalRepoConfig{{
+					Repo:    "owner/repo",
+					Deploys: []ExternalDeployConfig{{Name: "svc.api", Workflow: ".github/workflows/deploy.yaml"}},
+				}},
+			},
+			wantErr: `external[0].deploys[0].name "svc.api"`,
+		},
+		{
+			name: "environment name with dot rejected",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"us.east"},
+			},
+			wantErr: `environments[0] "us.east"`,
+		},
+		{
+			name: "valid names: hyphen, uppercase, leading digit, underscore",
+			config: TrunkConfig{
+				TrunkBranch:  "main",
+				Environments: []string{"dev-1", "Prod", "2nd", "us_west"},
+				Builds: []BuildConfig{
+					{Name: "my-app", Workflow: ".github/workflows/build.yaml"},
+					{Name: "MyApp", Workflow: ".github/workflows/build.yaml"},
+					{Name: "_internal", Workflow: ".github/workflows/build.yaml"},
+				},
+				Deploys: []DeployConfig{
+					{Name: "1svc", Workflow: ".github/workflows/deploy.yaml"},
+				},
+			},
+			wantNone: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := tt.config
+			errs := Validate(&cfg)
+			if tt.wantNone {
+				for _, e := range errs {
+					if strings.Contains(e, "must contain only") {
+						t.Errorf("Validate() unexpectedly rejected a valid name: %q", e)
+					}
+				}
+				return
+			}
+			found := false
+			for _, e := range errs {
+				if strings.Contains(e, tt.wantErr) && strings.Contains(e, "must contain only") {
+					found = true
+					break
+				}
+			}
+			if !found {
+				t.Errorf("Validate() missing job-ID-safe error containing %q, got: %v", tt.wantErr, errs)
+			}
+		})
+	}
+}

internal/config/validate_v1.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"fmt"
+	"regexp"
 	"sort"
 	"strings"
 )
@@ -44,6 +45,32 @@ var validRolloutTypes = map[string]bool{
 	RolloutTypeBlueGreen: true,
 }
 
+// jobIDSafeNameRe matches a name that is safe to use as a component of a GitHub
+// Actions job ID. cascade derives job IDs as build-<name>/deploy-<name> and
+// keys several identifiers and expression references off environment names. A
+// GitHub job ID must start with a letter or _ and contain only [A-Za-z0-9_-].
+// Because the name is a suffix after a build-/deploy- prefix, a leading digit,
+// uppercase letters, and hyphens are all acceptable in the name itself; only
+// characters outside [A-Za-z0-9_-] (such as ".", spaces, and "/") break the job
+// ID and the ${{ }} dereferences that read its outputs.
+var jobIDSafeNameRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
+
+// validateJobIDSafeName rejects a name that would produce an invalid GitHub
+// Actions job ID or break the expression references derived from it. Names are
+// rejected (not sanitized) on purpose: sanitizing distinct names could collapse
+// them to a single job ID and silently merge two callbacks.
+func validateJobIDSafeName(prefix, name string) []string {
+	if name == "" {
+		// Empty names are reported separately by the caller (".name is required").
+		return nil
+	}
+	if jobIDSafeNameRe.MatchString(name) {
+		return nil
+	}
+	return []string{fmt.Sprintf(
+		"%s %q must contain only letters, digits, hyphens, and underscores", prefix, name)}
+}
+
 // validateWorkflowRunXOR enforces that exactly one of workflow:/run: is set, and
 // that shell: is only present alongside run:.
 func validateWorkflowRunXOR(prefix, workflow, run, shell string) []string {

internal/generate/generator.go

@@ -832,8 +832,12 @@ func (g *Generator) writeCallbackJob(sb *strings.Builder, info CallbackInfo, wor
 	g.writeIfCondition(sb, info, needs)
 
 	switch {
-	case info.TimeoutMinutes > 0:
-		// Explicit per-callback timeout always wins.
+	case info.TimeoutMinutes > 0 && info.Run != "":
+		// Explicit per-callback timeout wins, but only on an inline run: callback.
+		// timeout-minutes is forbidden on a reusable-workflow caller job
+		// (jobs.<id>.uses): GitHub rejects the workflow at parse time. For uses:
+		// callbacks the timeout must live inside the called workflow. This mirrors
+		// the info.Run gate on environment: below.
 		fmt.Fprintf(sb, "    timeout-minutes: %d\n", info.TimeoutMinutes)
 	case info.Run != "":
 		// Inline run: callbacks are cascade-owned jobs, so they inherit the

internal/generate/generator_test.go

@@ -158,28 +158,30 @@ on:
 
 // TestGenerator_CallbackTimeoutMinutes asserts the per-callback
 // timeout_minutes field renders into the generated workflow as a job-level
-// `timeout-minutes:` setting (#97). Without this, callbacks default to GHA's
-// 360min timeout, which is too lenient for tight feedback loops and too
-// strict for longer integration jobs.
+// `timeout-minutes:` setting (#97) for inline run: callbacks. Without this,
+// inline callbacks default to GHA's 360min timeout, which is too lenient for
+// tight feedback loops and too strict for longer integration jobs.
+//
+// timeout-minutes is only valid on a steps job, so the callbacks here use inline
+// run:. Reusable-workflow (uses:) callbacks must NOT receive it (GitHub rejects
+// the workflow); that gate is covered by
+// TestGenerator_ExplicitTimeoutNotOnReusableCallback.
 func TestGenerator_CallbackTimeoutMinutes(t *testing.T) {
 	tmpDir := t.TempDir()
 	require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755))
-	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n  workflow_call:\n"), 0644))
-	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/deploy.yaml"), []byte("on:\n  workflow_call:\n"), 0644))
-	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/validate.yaml"), []byte("on:\n  workflow_call:\n"), 0644))
 
 	cfg := &config.TrunkConfig{
 		TrunkBranch:  "main",
 		Environments: []string{"dev"},
 		Validate: &config.ValidateConfig{
-			Workflow:       ".github/workflows/validate.yaml",
+			Run:            "make validate",
 			TimeoutMinutes: 5,
 		},
 		Builds: []config.BuildConfig{
-			{Name: "app", Workflow: ".github/workflows/build.yaml", Triggers: []string{"src/**"}, TimeoutMinutes: 30},
+			{Name: "app", Run: "make build", Triggers: []string{"src/**"}, TimeoutMinutes: 30},
 		},
 		Deploys: []config.DeployConfig{
-			{Name: "svc", Workflow: ".github/workflows/deploy.yaml", DependsOn: []string{"app"}, TimeoutMinutes: 15},
+			{Name: "svc", Run: "make deploy", DependsOn: []string{"app"}, TimeoutMinutes: 15},
 		},
 	}
 

internal/generate/job_control_test.go

@@ -248,3 +248,48 @@ func TestGenerator_BothFieldsUnsetNonBreaking(t *testing.T) {
 	assert.NotContains(t, jobBlock(t, result, "build-app"), "timeout-minutes:")
 	assert.Contains(t, jobBlock(t, result, "setup"), "timeout-minutes: 30")
 }
+
+// TestGenerator_ExplicitTimeoutNotOnReusableCallback asserts that an explicit
+// per-callback timeout_minutes is NOT emitted as a job-level timeout-minutes on
+// a reusable-workflow (uses:) callback. GitHub forbids timeout-minutes on a job
+// that calls a reusable workflow (allowed caller keys: name, uses, with,
+// secrets, needs, if, permissions, strategy, concurrency); the timeout must live
+// inside the called workflow. An inline run: callback with the same setting DOES
+// carry it, since inline jobs are cascade-owned steps jobs.
+func TestGenerator_ExplicitTimeoutNotOnReusableCallback(t *testing.T) {
+	tmpDir := t.TempDir()
+	writeStubWorkflow(t, tmpDir, "build.yaml")
+	writeStubWorkflow(t, tmpDir, "deploy.yaml")
+
+	cfg := &config.TrunkConfig{
+		TrunkBranch:  "main",
+		Environments: []string{"dev"},
+		Builds: []config.BuildConfig{
+			// Reusable-workflow callback with an explicit timeout: must NOT emit it.
+			{Name: "reusable", Workflow: ".github/workflows/build.yaml", Triggers: []string{"src/**"}, TimeoutMinutes: 15},
+			// Inline run: callback with an explicit timeout: DOES emit it.
+			{Name: "inline", Run: "go test ./...", Triggers: []string{"src/**"}, TimeoutMinutes: 15},
+		},
+		Deploys: []config.DeployConfig{
+			// Reusable-workflow deploy callback with an explicit timeout: no emit.
+			{Name: "svc", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, TimeoutMinutes: 15},
+		},
+	}
+
+	result, err := NewGenerator(cfg, tmpDir).Generate()
+	require.NoError(t, err)
+
+	reusableBuild := jobBlock(t, result, "build-reusable")
+	assert.Contains(t, reusableBuild, "uses:", "sanity: reusable callback is a uses: caller")
+	assert.NotContains(t, reusableBuild, "timeout-minutes:",
+		"explicit timeout_minutes must not be emitted on a reusable-workflow caller job")
+
+	reusableDeploy := jobBlock(t, result, "deploy-svc")
+	assert.Contains(t, reusableDeploy, "uses:", "sanity: reusable deploy is a uses: caller")
+	assert.NotContains(t, reusableDeploy, "timeout-minutes:",
+		"explicit timeout_minutes must not be emitted on a reusable-workflow deploy caller job")
+
+	inline := jobBlock(t, result, "build-inline")
+	assert.Contains(t, inline, "timeout-minutes: 15",
+		"explicit timeout_minutes on an inline run: callback is honored")
+}