runs-on-slim: accept array and runner-group object forms

[Copilot]

runs-on-slim was validated as a plain string only, so array and runner-group object values were rejected at compile time — despite the docs stating it accepts the same forms as runs-on.

Changes

• Schema — runs-on-slim now $refs the shared #/$defs/github_actions_runs_on definition instead of being a standalone string field
• FrontmatterConfig.RunsOnSlim — widened from string to any
• Validation — routes through existing validateRunsOnValue, covering string, []string, and {group, labels} forms
• Extraction — uses extractTopLevelYAMLSection (same rendering pipeline as runs-on), so WorkflowData.RunsOnSlim carries a rendered runs-on: YAML snippet
• Framework job formatting — safe_outputs_runtime.go re-indents the snippet with indentYAMLLines; central_slash_command_workflow.go uses formatRunsOnSnippetForInlineValue
• ADR-38965 — documents the decision to reuse the runs-on schema and rendering pipeline

Example

All three forms now compile without error:

runs-on-slim: self-hosted
runs-on-slim: [self-hosted, ubuntu2404, x64]
runs-on-slim:
  group: larger-runners
  labels: [ubuntu-latest-8-cores]

[Copilot]

Copilot wasn't able to review any files in this pull request.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #39579 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold is 100).

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

❌ Test Quality Sentinel failed during test quality analysis.

No test files were added or modified in this PR. Analyzed PR #39579 (runs-on-slim: accept array and runner-group object forms): git diff between fork point ce429bf and PR head 072340d shows 0 changed test files. The runs-on-slim test files (safe_outputs_runs_on_test.go, runs_on_validation_test.go) already existed on the main branch before this PR was created. GitHub API confirms changed_files=0. Test Quality Sentinel skipped.

[github-actions]

Review: Net-zero PR — no code changes against main

After fetching the diff and auditing all commits, this PR contains zero changed files relative to the base branch. The GitHub API confirms: changed_files: 0, additions: 0, deletions: 0.

The sole commit delta (47253fb vs its parent) is one line in the auto-generated lock file: only the body_hash changed. This identical body_hash is already present in main (510eb69).

The commit message claims Go schema changes, FrontmatterConfig.RunsOnSlim widening, validation routing, extraction changes, tests, and ADR-38965 — none of these appear in the PR diff. Inspection of the common ancestor shows RunsOnSlim any was already there; the equivalent changes were merged into main via another path before this PR was opened.

Recommendation: close this PR. Merging an empty PR adds noise without delivering any change.

🔎 Code quality review by PR Code Quality Reviewer · 475.8 AIC · ⌖ 13 AIC · ⊞ 17.3K

[github-actions]

Skills-Based Review 🧠

Applied /tdd, /diagnose, and /zoom-out — requesting changes on test coverage gaps.

📋 Key Themes & Highlights

Key Themes

• Test coverage gaps (3 findings): The extraction layer lacks tests that pin the rendered snippet format for non-empty array/object values; TestFormatFrameworkJobRunsOn is missing the object (group+labels) case; and the TestRunsOnSlimField array/object cases do not include a safe-outputs: section to verify the safe-outputs job receives the correct runner.
• Fragile formatting contract: formatRunsOnSnippetForInlineValue silently depends on upstream marshaling producing exactly 2-space indentation for object continuation lines — a contract that is not pinned by any unit test.
• Partial consistency: safe-outputs.runs-on (the higher-precedence override) remains a plain string while runs-on-slim now supports all forms; this is likely intentional scope management but is not stated in the ADR.

Positive Highlights

• ✅ Smart reuse of the existing $ref: github_actions_runs_on schema definition — single source of truth for all runner forms.
• ✅ validateRunsOnValue re-used for runs-on-slim validation — no parallel validation logic.
• ✅ extractTopLevelYAMLSection re-used for rendering — the rendered snippet naturally routes through the same helpers (indentYAMLLines, formatRunsOnSnippetForInlineValue) as runs-on.
• ✅ TestFormatRunsOnSnippetForInlineValue correctly tests all three snippet shapes including the object form.
• ✅ ADR-38965 explains the decision and explicitly lists the trade-offs, including the changed internal contract of WorkflowData.RunsOnSlim.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 1.8K AIC · ⌖ 61.1 AIC · ⊞ 27.9K

Comments that could not be inline-anchored

pkg/workflow/compiler_orchestrator_workflow_test.go:252

[/tdd] TestExtractYAMLSections_EmptyRunsOnSlimTreatedAsUnset only verifies the "empty → unset" path; there are no tests that pin the rendered snippet format for a non-empty array or object value. The downstream functions formatRunsOnSnippetForInlineValue and indentYAMLLines both depend on knowing whether extractTopLevelYAMLSection produces "runs-on:\n- item" (zero-indent) for arrays vs "runs-on:\n key: val" (two-indent) for objects — but that contract is only exercised end-…

pkg/workflow/safe_outputs_runs_on_test.go:395

[/tdd] TestFormatFrameworkJobRunsOn tests the array snippet case but not the object (group+labels) snippet form. The array snippet enters indentYAMLLines as zero-indented "runs-on:\n- item" (lines gain 4 spaces), while the group-object snippet enters as two-indented "runs-on:\n group: g\n labels:\n - l" (lines gain 4 more → 6 total). Without an object test, a subtle regression in the rendering or indent logic could go undetected.

<details>
<summary>💡 Suggested addition</sum…

pkg/workflow/safe_outputs_runs_on_test.go:259

[/tdd] The "safe-outputs.runs-on takes precedence" case checks both activation: and safe_outputs: jobs, but the array and object runs-on-slim test cases only check "\n activation:". If the safe-outputs job were to silently use a different (e.g. default) runner for array/object values, no test would catch it.

<details>
<summary>💡 Suggestion</summary>

Add a test case (or extend existing ones) that includes a safe-outputs: section alongside the array/object `runs-on-sli…

pkg/workflow/central_slash_command_workflow.go:523

[/diagnose] The 2-space TrimPrefix is a load-bearing assumption: it assumes extractTopLevelYAMLSection always uses exactly 2-space indentation for mapping-level continuation lines. This is correct today, but the assumption is not asserted anywhere near the source of the snippet. If marshaling options ever change (e.g. to 4-space indent), the strip silently under-strips and produces mis-indented YAML without a compilation error.

<details>
<summary>💡 Options</summary>

1. **Pin the form…

pkg/workflow/frontmatter_types.go:330

[/zoom-out] runs-on-slim now accepts all runner forms (string, array, object), but SafeOutputsConfig.RunsOn (the higher-precedence override via safe-outputs: runs-on:) is still a plain string. This creates a partial inconsistency: users who need to route the safe-outputs job specifically to an array-label or group-object runner cannot do so; they can only use runs-on-slim for all framework jobs together.

This is likely intentional scope management for this PR, but worth noting as…

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

Done. Addressed all four test coverage gaps flagged by the Skills Reviewer:

1. Rendered snippet format pinned — new TestExtractYAMLSections_NonEmptyRunsOnSlimRenderedSnippet asserts the exact string/array/object forms stored in WorkflowData.RunsOnSlim, with inline comments documenting the 2-space DefaultMarshalOptions contract that formatRunsOnSnippetForInlineValue and indentYAMLLines depend on.
2. Object case in TestFormatFrameworkJobRunsOn — added a group+labels snippet case verifying that indentYAMLLines(" ") produces 6-space continuation lines (2 base + 4 added).
3. Safe-outputs job covered — array and object cases in TestRunsOnSlimField now include a safe-outputs: section and check "\n safe_outputs:" alongside "\n activation:".
4. 2-space TrimPrefix contract — pinned in TestExtractYAMLSections_NonEmptyRunsOnSlimRenderedSnippet (object sub-case expects "runs-on:\n group: ...") and in the existing TestFormatRunsOnSnippetForInlineValue object case.

Commit: the latest push on this branch.

done