Allow runs-on-slim to use runner label arrays#38965
Conversation
Co-authored-by: zarenner <13670625+zarenner@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR expands runs-on-slim to accept the same runner-selection shapes as GitHub Actions runs-on (string, label array, or { group, labels } object), and updates compilation output so framework-generated jobs render those forms correctly. It also updates schema validation, docs, and editor autocomplete metadata to match the new syntax.
Changes:
- Reuse the shared
runs-onschema/validation forruns-on-slimand parse it as string/array/object. - Render complex
runs-on-slimYAML correctly for framework/generated jobs while preservingsafe-outputs.runs-onprecedence. - Update tests, reference docs, workflow guidance, and editor autocomplete data for the expanded forms.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/workflow_builder.go | Extracts runs-on-slim via shared YAML extraction so arrays/objects compile. |
| pkg/workflow/safe_outputs_runtime.go | Renders runs-on-slim as a multi-line YAML snippet suitable for job insertion. |
| pkg/workflow/safe_outputs_runs_on_test.go | Adds coverage for runs-on-slim array/object forms and updated snippet expectations. |
| pkg/workflow/frontmatter_types.go | Changes RunsOnSlim type to any to support string/array/object forms. |
| pkg/workflow/frontmatter_serialization.go | Serializes runs-on-slim from FrontmatterConfig when present. |
| pkg/workflow/frontmatter_parsing.go | Validates runs-on-slim using the shared runs-on shape validator. |
| pkg/workflow/compiler_types.go | Clarifies WorkflowData.RunsOnSlim now holds a rendered runs-on snippet. |
| pkg/workflow/central_slash_command_workflow.go | Ensures centralized workflow runs-on: rendering works with runs-on snippets. |
| pkg/workflow/central_slash_command_workflow_test.go | Adds unit tests for formatting runs-on snippets for inline YAML usage. |
| pkg/parser/schemas/main_workflow_schema.json | Switches runs-on-slim schema to $defs/github_actions_runs_on and updates examples. |
| pkg/parser/schema_location_test.go | Adds schema-location tests to accept runs-on-slim array/object forms. |
| docs/src/content/docs/reference/self-hosted-runners.md | Documents that runs-on-slim supports string/array/object forms. |
| docs/src/content/docs/reference/frontmatter.md | Updates frontmatter reference to describe expanded runs-on-slim syntax. |
| docs/public/editor/autocomplete-data.json | Updates autocomplete type metadata for runs-on-slim. |
| .github/aw/workflow-constraints.md | Updates constraints guidance to reflect new runs-on-slim shapes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 15/15 changed files
- Comments generated: 2
| workflowData.RunsOn = c.extractTopLevelYAMLSection(frontmatter, "runs-on") | ||
| // Extract runs-on-slim as a plain string (no YAML formatting needed) | ||
| if v, ok := frontmatter["runs-on-slim"]; ok { | ||
| if s, ok := v.(string); ok { | ||
| workflowData.RunsOnSlim = s | ||
| } | ||
| workflowData.RunsOnSlim = c.extractTopLevelYAMLSection(map[string]any{"runs-on": v}, "runs-on") | ||
| } |
| if !isNilValue(fc.RunsOn) { | ||
| result["runs-on"] = fc.RunsOn | ||
| } | ||
| if fc.RunsOnSlim != "" { | ||
| if !isNilValue(fc.RunsOnSlim) { | ||
| result["runs-on-slim"] = fc.RunsOnSlim | ||
| } |
|
@copilot review all comments and address the unresolved review feedback on this PR. Then re-run checks and summarize any remaining blockers.
|
|
|
|
❌ Test Quality Sentinel failed during test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (>100 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs MatterADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you for capturing the decision to reuse the shared 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in 🔒 This gate is blocking: link the ADR in the PR body to proceed.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: runs-on-slim multi-form support
No correctness bugs or data-loss risks found. Two non-blocking comments were posted inline; one additional observation below.
🔍 Findings detail
1. formatRunsOnSnippetForInlineValue — undocumented indentation coupling (inline, medium)
The 2-space TrimPrefix and 6-space re-indent are hard-wired to two invariants that live elsewhere: DefaultMarshalOptions using 2-space map indentation, and the central slash command template placing runs-on: at 4-space indent. Neither is stated in the function. See inline comment.
2. TestGenerateCentralSlashCommandWorkflow_UsesCentralizedRunsOnResolution — stale test fixture (medium)
pkg/workflow/central_slash_command_workflow_test.go line 302 still sets RunsOnSlim: "ubuntu-latest" using the old raw-string format. After this PR the production path always stores a rendered snippet ("runs-on: ubuntu-latest"). The test continues to pass only because formatRunsOnSnippetForInlineValue has a defensive fallback for values without a "runs-on:" prefix. This means the test exercises the fallback, not the production path — a future removal of that defensive branch would break the test unexpectedly.
Suggested fix: update the fixture to RunsOnSlim: "runs-on: ubuntu-latest" to match the semantics documented on the field.
3. TestFormatFrameworkJobRunsOn — misleading test name (inline, low)
See inline comment.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
proxy.golang.org
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "proxy.golang.org"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · 957.7 AIC · ⌖ 13.4 AIC · ⊞ 17.3K
| for i, line := range lines { | ||
| line = strings.TrimPrefix(line, " ") | ||
| lines[i] = " " + line |
There was a problem hiding this comment.
Fragile magic-number indentation coupling: the " " strip and " " re-indent are silently coupled to two external invariants — DefaultMarshalOptions producing 2-space map-field indent, and runs-on: sitting at exactly 4-space indent in the central slash command template. No comment explains these assumptions, so a future indent-style or template change would corrupt the generated YAML silently.
💡 Suggested fix
Document the coupling explicitly:
// The 2-space strip matches DefaultMarshalOptions map indentation.
// The 6-space re-indent aligns with the central slash command template,
// where runs-on: lives at 4-space job-level indent (4 + 2 = 6).
line = strings.TrimPrefix(line, " ")
lines[i] = " " + lineAlternatively, pass the target indent as a parameter so the function is not implicitly tied to one specific template.
| expectedRunsOn: "runs-on: " + constants.DefaultActivationJobRunnerImage, | ||
| }, | ||
| { | ||
| name: "runs-on-slim array snippet returned as-is", |
There was a problem hiding this comment.
Misleading test name: "runs-on-slim array snippet returned as-is" implies the value is passed through unchanged, but formatFrameworkJobRunsOn calls c.indentYAMLLines(data.RunsOnSlim, " ") which explicitly adds 4-space indentation to every continuation line. A reader relying on this name to understand the contract will be misled.
💡 Suggested fix
Rename to something that describes the actual transformation, for example:
name: "runs-on-slim array snippet has continuation lines indented",or more precisely:
name: "runs-on-slim array snippet indents continuation lines by 4 spaces",
|
@copilot run pr-finisher review |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Completed a PR-finisher pass in 277b0d8.
Remaining blocker: the ADR gate still needs the ADR linked in the PR body; I can’t update the PR body from this environment. |
runs-on-slimwas schema-validated and parsed as a string only, so workflows using the same runner selection forms supported byruns-onfailed to compile.Schema and parsing
runs-onschema forruns-on-slim.runs-on-slimas string, label array, or{ group, labels }object.Compiler output
runs-on-slimvalues correctly for framework-generated jobs.safe-outputs.runs-onprecedence.Docs and editor metadata
Example now accepted: