Support runner groups for custom safe jobs

docs/adr/53977-support-runner-groups-custom-safe-jobs.md

@@ -0,0 +1,45 @@
+# ADR-53977: Extend Custom Safe-Job `runs-on` to Support Runner-Group Objects and Remove `runner` Alias
+
+**Date**: 2026-08-20
+**Status**: Draft
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+Custom safe jobs (`safe-outputs.jobs.<job>`) had their own limited `runs-on` parser that only accepted strings and label arrays. Runner-group object form (`{group: ..., labels: [...]}`) was not supported, leaving custom safe jobs unable to run on self-hosted runner groups. All other `runs-on` configuration surfaces in the framework (top-level `runs-on`, `safe-outputs.runs-on`, `safe-outputs.threat-detection.runs-on`) already accepted all three forms via the shared `extractCustomJobRunsOn` parser. The legacy `runner` key was a deprecated alias for `runs-on` that duplicated the configuration surface and required a separate code path.
+
+### Decision
+
+We will reuse the shared `extractCustomJobRunsOn` parser for custom safe jobs, making their `runs-on` field accept the same three forms (string, label array, runner-group object) as every other runner configuration surface. The deprecated `runner` alias will be removed as a breaking change (major version bump), and a `gh aw fix` codemod (`safe-job-runner-to-runs-on`) will be provided to automatically migrate existing workflows.
+
+### Alternatives Considered
+
+#### Alternative 1: Extend the Custom Safe-Job Parser In-Place
+
+Extend the existing `toRunsOnValue`/`isRunsOnArrayValue` helpers to also handle `map[string]any` (runner-group objects) without delegating to the shared parser. This avoids a code-sharing dependency, but duplicates validation logic (macOS label rejection, empty-object rejection, unknown key rejection) that is already tested and maintained in `extractCustomJobRunsOn`. Any future change to runner-group validation would need to be applied in two places.
+
+#### Alternative 2: Keep the `runner` Alias as a Deprecated No-Op
+
+Retain `runner` as a tolerated (but warned) alias rather than removing it outright, making the change non-breaking. This avoids the need for a migration codemod and a major version bump. However, it perpetuates two parallel configuration keys for the same concept and increases schema surface area indefinitely. Given that `gh aw fix` can automate the rename, the migration cost is low enough to justify a clean removal.
+
+### Consequences
+
+#### Positive
+- Custom safe jobs now have full parity with all other runner configuration surfaces, enabling use of runner groups.
+- Validation logic (macOS rejection, empty-object rejection, unknown-key rejection) is exercised from a single code path, reducing the risk of inconsistencies.
+- The schema is simplified: one canonical key (`runs-on`) replaces two (`runs-on` and `runner`).
+- The automated codemod minimizes user effort for migration.
+
+#### Negative
+- This is a breaking change: workflows using `safe-outputs.jobs.<job>.runner` will fail validation until migrated. Users must run `gh aw fix` or manually rename the key.
+- The major version bump signals a broader API break even though only one deprecated alias is removed, which may cause friction for teams managing dependency pins.
+
+#### Neutral
+- The `SafeJobConfig.RunsOn` field type changes from `RunsOnValue` (a `[]string`) to `string` (the pre-serialized YAML snippet), aligning with how the shared compiler represents runner configuration internally.
+- Two helper functions (`toRunsOnValue`, `isRunsOnArrayValue`) are deleted from `runs_on_snippet.go` as they are now unused.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

docs/src/content/docs/reference/safe-outputs.md

@@ -1895,6 +1895,24 @@ safe-outputs:
 
 `safe-outputs.runs-on` overrides `runs-on-slim:` for safe-output jobs specifically. To override the runner for all framework jobs at once, use the top-level [`runs-on-slim:`](/gh-aw/reference/self-hosted-runners/#configuring-the-framework-job-runner) field instead.
 
+Custom safe-jobs can select their own runner with `safe-outputs.jobs.<job>.runs-on`. This field supports runner labels, label arrays, and runner-group objects:
+
+```aw
+---
+safe-outputs:
+  jobs:
+    notify:
+      runs-on:
+        group: safe-job-runners
+        labels: [linux]
+      inputs:
+        message:
+          description: Notification message
+      steps:
+        - run: echo "Notify"
+---
+```
+
 ### Safe Outputs Job Concurrency (`concurrency-group:`)
 
 Control concurrency for the compiled `safe_outputs` job. When set, the job uses this group with `cancel-in-progress: false` (queuing semantics — in-progress runs are never cancelled).

docs/src/content/docs/reference/self-hosted-runners.md

@@ -79,6 +79,8 @@ runs-on:
 ---
 ```
 
+The string, array, and object forms are supported by the top-level `runs-on`, `runs-on-slim`, `safe-outputs.runs-on`, `safe-outputs.threat-detection.runs-on`, and custom `safe-outputs.jobs.<job>.runs-on` fields.
+
 ## Sharing configuration via imports
 
 `runs-on` must be set in each workflow — it is not merged from imports. Other settings like `network` and `tools` can be shared:

pkg/cli/codemod_safe_job_runner.go

@@ -0,0 +1,147 @@
+package cli
+
+import (
+	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+var safeJobRunnerCodemodLog = logger.New("cli:codemod_safe_job_runner")
+
+func getSafeJobRunnerCodemod() Codemod {
+	return Codemod{
+		ID:           "safe-job-runner-to-runs-on",
+		Name:         "Rename safe-outputs.jobs runner to runs-on",
+		Description:  "Renames deprecated safe-outputs.jobs.<job>.runner fields to runs-on.",
+		IntroducedIn: "1.5.0",
+		Apply: func(content string, _ map[string]any) (string, bool, error) {
+			newContent, applied, err := applyFrontmatterLineTransform(content, renameSafeJobRunnerKeys)
+			if applied {
+				safeJobRunnerCodemodLog.Print("Renamed safe-job runner fields to runs-on")
+			}
+			return newContent, applied, err
+		},
+	}
+}
+
+func renameSafeJobRunnerKeys(lines []string) ([]string, bool) {
+	result := append([]string(nil), lines...)
+	modified := false
+
+	for i := range lines {
+		if !hasYAMLKey(strings.TrimSpace(lines[i]), "safe-outputs") {
+			continue
+		}
+
+		safeOutputsIndent := len(getIndentation(lines[i]))
+		childIndent := -1
+		for j := i + 1; j < len(lines); j++ {
+			trimmed := strings.TrimSpace(lines[j])
+			if trimmed == "" || strings.HasPrefix(trimmed, "#") {
+				continue
+			}
+
+			indent := len(getIndentation(lines[j]))
+			if indent <= safeOutputsIndent {
+				break
+			}
+			if childIndent == -1 {
+				childIndent = indent
+			}
+			if indent != childIndent || !hasYAMLKey(trimmed, "jobs") {
+				continue
+			}
+
+			if renameSafeJobRunnerKeysInJobsBlock(result, lines, j) {
+				modified = true
+			}
+			break
+		}
+	}
+
+	return result, modified
+}
+
+func renameSafeJobRunnerKeysInJobsBlock(result, lines []string, jobsLine int) bool {
+	jobsIndent := len(getIndentation(lines[jobsLine]))
+	jobIndent := -1
+	jobStarts := []int{}
+	blockEnd := jobsLine + 1
+
+	for i := jobsLine + 1; i < len(lines); i++ {
+		trimmed := strings.TrimSpace(lines[i])
+		if trimmed == "" || strings.HasPrefix(trimmed, "#") {
+			blockEnd = i + 1
+			continue
+		}
+
+		indent := len(getIndentation(lines[i]))
+		if indent <= jobsIndent {
+			blockEnd = i
+			break
+		}
+		blockEnd = i + 1
+		if jobIndent == -1 {
+			jobIndent = indent
+		}
+		if indent == jobIndent {
+			jobStarts = append(jobStarts, i)
+		}
+	}
+
+	modified := false
+	for i, start := range jobStarts {
+		end := blockEnd
+		if i+1 < len(jobStarts) {
+			end = jobStarts[i+1]
+		}
+		if renameSafeJobRunnerKeyInJob(result, lines, start, end) {
+			modified = true
+		}
+	}
+	return modified
+}
+
+func renameSafeJobRunnerKeyInJob(result, lines []string, start, end int) bool {
+	jobIndent := len(getIndentation(lines[start]))
+	fieldIndent := -1
+	runnerLine := -1
+	hasRunsOn := false
+
+	for i := start + 1; i < end; i++ {
+		trimmed := strings.TrimSpace(lines[i])
+		if trimmed == "" || strings.HasPrefix(trimmed, "#") {
+			continue
+		}
+
+		indent := len(getIndentation(lines[i]))
+		if indent <= jobIndent {
+			break
+		}
+		if fieldIndent == -1 {
+			fieldIndent = indent
+		}
+		if indent != fieldIndent {
+			continue
+		}
+		if hasYAMLKey(trimmed, "runs-on") {
+			hasRunsOn = true
+		}
+		if hasYAMLKey(trimmed, "runner") {
+			runnerLine = i
+		}
+	}
+
+	if runnerLine == -1 || hasRunsOn {
+		return false
+	}
+	replacement, replaced := findAndReplaceInLine(lines[runnerLine], "runner", "runs-on")
+	if replaced {
+		result[runnerLine] = replacement
+	}
+	return replaced
+}
+
+func hasYAMLKey(line, key string) bool {
+	return strings.HasPrefix(line, key+":")
+}

pkg/cli/codemod_safe_job_runner_test.go

@@ -0,0 +1,137 @@
+//go:build !integration
+
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSafeJobRunnerCodemod(t *testing.T) {
+	codemod := getSafeJobRunnerCodemod()
+
+	t.Run("metadata", func(t *testing.T) {
+		assert.Equal(t, "safe-job-runner-to-runs-on", codemod.ID)
+		assert.Equal(t, "Rename safe-outputs.jobs runner to runs-on", codemod.Name)
+		assert.Equal(t, "Renames deprecated safe-outputs.jobs.<job>.runner fields to runs-on.", codemod.Description)
+		assert.Equal(t, "1.5.0", codemod.IntroducedIn)
+		require.NotNil(t, codemod.Apply)
+	})
+
+	tests := []struct {
+		name        string
+		content     string
+		want        string
+		wantApplied bool
+	}{
+		{
+			name: "renames scalar runner",
+			content: `---
+safe-outputs:
+  jobs:
+    notify:
+      runner: ubuntu-latest
+      steps:
+        - run: echo hi
+---`,
+			want: `---
+safe-outputs:
+  jobs:
+    notify:
+      runs-on: ubuntu-latest
+      steps:
+        - run: echo hi
+---`,
+			wantApplied: true,
+		},
+		{
+			name: "preserves runner group block",
+			content: `---
+safe-outputs:
+  jobs:
+    notify:
+      runner: # runner group
+        group: larger-runners
+        labels: [linux]
+---`,
+			want: `---
+safe-outputs:
+  jobs:
+    notify:
+      runs-on: # runner group
+        group: larger-runners
+        labels: [linux]
+---`,
+			wantApplied: true,
+		},
+		{
+			name: "matches keys with trailing comments",
+			content: `---
+safe-outputs: # security settings
+  jobs: # custom output jobs
+    notify:
+      runner: ubuntu-latest # legacy field
+---`,
+			want: `---
+safe-outputs: # security settings
+  jobs: # custom output jobs
+    notify:
+      runs-on: ubuntu-latest # legacy field
+---`,
+			wantApplied: true,
+		},
+		{
+			name: "skips job with canonical field",
+			content: `---
+safe-outputs:
+  jobs:
+    notify:
+      runner: old-runner
+      runs-on: ubuntu-latest
+---
+`,
+			want: `---
+safe-outputs:
+  jobs:
+    notify:
+      runner: old-runner
+      runs-on: ubuntu-latest
+---
+`,
+			wantApplied: false,
+		},
+		{
+			name: "ignores runner outside safe jobs",
+			content: `---
+runner: top-level
+safe-outputs:
+  create-issue: {}
+jobs:
+  build:
+    runner: custom
+---
+`,
+			want: `---
+runner: top-level
+safe-outputs:
+  create-issue: {}
+jobs:
+  build:
+    runner: custom
+---
+`,
+			wantApplied: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, applied, err := codemod.Apply(tt.content, map[string]any{})
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantApplied, applied)
+			assert.Equal(t, tt.want, result)
+		})
+	}
+}

pkg/cli/fix_codemods.go

@@ -91,6 +91,7 @@ func GetAllCodemods() []Codemod {
 		getSafeOutputMergePRConstraintsCodemod(),                   // Rename deprecated merge-pull-request allowed-labels/allowed-branches
 		getSafeOutputAddReviewerAllowlistsCodemod(),                // Rename deprecated add-reviewer reviewers/team-reviewers
 		getSafeOutputDispatchRepositoryKeyCodemod(),                // Rename deprecated safe-outputs.dispatch_repository key
+		getSafeJobRunnerCodemod(),                                  // Rename deprecated safe-outputs.jobs runner fields
 		getSafeInputsToMCPScriptsCodemod(),                         // Rename safe-inputs to mcp-scripts
 		getRateLimitToUserRateLimitCodemod(),                       // Rename rate-limit to user-rate-limit with max key migration
 		getEffectiveTokensToAICreditsCodemod(),                     // Migrate obsolete effective-token budget keys to AI credits keys

pkg/cli/fix_codemods_test.go

@@ -99,6 +99,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) {
 		"safe-output-merge-pr-constraints",
 		"safe-output-add-reviewer-allowlists",
 		"safe-output-dispatch-repository-key",
+		"safe-job-runner-to-runs-on",
 		"safe-inputs-to-mcp-scripts",
 		"rate-limit-to-user-rate-limit",
 		"effective-tokens-to-ai-credits",
@@ -228,6 +229,7 @@ func expectedCodemodOrder() []string {
 		"safe-output-merge-pr-constraints",
 		"safe-output-add-reviewer-allowlists",
 		"safe-output-dispatch-repository-key",
+		"safe-job-runner-to-runs-on",
 		"safe-inputs-to-mcp-scripts",
 		"rate-limit-to-user-rate-limit",
 		"effective-tokens-to-ai-credits",