Merge pull request #58 from github/nodeselector/improve-self-hosted-runner-errors

Improve self-hosted-runner errors: show context, deduplicate, add --allow-runners

Author: nodeselector

cmd/gh-actions-lock/check.go

@@ -24,6 +24,7 @@ import (
 	"github.com/github/gh-actions-lock/internal/resolve"
 	"github.com/github/gh-actions-lock/internal/tag"
 	"github.com/github/gh-actions-lock/internal/ui"
+	"github.com/github/gh-actions-lock/internal/workflowfile"
 	"github.com/spf13/cobra"
 )
 
@@ -46,6 +47,10 @@ type checkOptions struct {
 	// are kept as-is in the lock comment instead of being resolved to
 	// the full patch tag (e.g. "v4.2.1").
 	noNarrow bool
+	// allowRunners lists additional runner labels to treat as hosted.
+	// Use when org-provisioned larger runners (e.g. ubuntu-latest-xl)
+	// are flagged as self-hosted but you know they are GitHub-hosted.
+	allowRunners []string
 }
 
 func newCheckCmd(newResolver resolverFunc) *cobra.Command {
@@ -103,6 +108,9 @@ func newCheckCmd(newResolver resolverFunc) *cobra.Command {
 			# Read-only check for CI (writes nothing, exits 1 if invalid)
 			$ gh actions-lock check --no-fix --json=valid,findings
 
+			# Treat org larger runners as hosted
+			$ gh actions-lock --allow-runners ubuntu-latest-xl,ubuntu-latest-2xl
+
 			# All fields as JSON
 			$ gh actions-lock check --json
 		`),
@@ -131,6 +139,7 @@ func bindCheckFlags(cmd *cobra.Command, opts *checkOptions) {
 	cmd.Flags().BoolVar(&opts.rescan, "rescan", false, "Re-verify reachability for every recorded pin (bypasses the lockfile fast path)")
 	cmd.Flags().BoolVar(&opts.noFix, "no-fix", false, "Read-only: report findings without modifying workflows or the lockfile")
 	cmd.Flags().BoolVar(&opts.noNarrow, "no-narrow", false, "Keep mutable version refs (e.g. v4) instead of narrowing to full patch tags (e.g. v4.2.1)")
+	cmd.Flags().StringSliceVar(&opts.allowRunners, "allow-runners", nil, "Additional runner `labels` to treat as GitHub-hosted (e.g. ubuntu-latest-xl)")
 	cmd.Flags().StringVar(&opts.profileDir, "profile", "", "Enable profiling: write trace, CPU profile, and HTTP log to `dir`")
 }
 
@@ -172,6 +181,11 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 	// Shared worker pool for all concurrent phases.
 	pool := pinpool.New(0, console) // 0 → DefaultWorkers
 
+	// Register user-supplied runner labels as hosted before parsing.
+	if len(opts.allowRunners) > 0 {
+		workflowfile.RegisterOrgHostedLabels(opts.allowRunners)
+	}
+
 	// If profiling, use a profiled resolver that logs HTTP calls.
 	if prof != nil && newResolver == nil {
 		newResolver = func(hostname string, pool *pinpool.Pool) (*resolve.Resolver, error) {
@@ -197,6 +211,17 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 		r.SeedFromLockfile(store.AllDeps())
 	}
 	endSetup()
+
+	// Fetch org-level hosted runner labels so larger runners aren't flagged
+	// as self-hosted. Best-effort: 403 is silently ignored.
+	if gc := r.GHClient(); gc != nil {
+		if currentRepo, err := repository.Current(); err == nil {
+			if labels, err := gc.OrgHostedRunnerNames(ctx, currentRepo.Owner); err == nil && len(labels) > 0 {
+				workflowfile.RegisterOrgHostedLabels(labels)
+			}
+		}
+	}
+
 	opts.workflowPaths = paths
 
 	// Detailed narration is suppressed from the terminal during the run so the

cmd/gh-actions-lock/format/terminal.go

@@ -2,6 +2,7 @@ package format
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/github/gh-actions-lock/internal/pipeline/checks"
@@ -84,12 +85,22 @@ func renderErrorFindings(out *ui.UI, report *checks.Report, failedCount, checked
 			continue
 		}
 
+		// Self-hosted-runner findings share the same empty dep key;
+		// render them as a deduplicated group showing affected workflows.
+		var selfHostedFindings []checks.Finding
 		for _, f := range dg.findings {
 			if f.Category == checks.NotPinned {
 				continue
 			}
+			if f.Category == checks.SelfHostedRunner {
+				selfHostedFindings = append(selfHostedFindings, f)
+				continue
+			}
 			renderFindingDetail(out, f, dep)
 		}
+		if len(selfHostedFindings) > 0 {
+			renderSelfHostedGroup(out, selfHostedFindings)
+		}
 	}
 
 	parts := []string{}
@@ -124,11 +135,11 @@ func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) {
 	if f.Category == checks.LockfileForgery && f.Dependency != nil {
 		owner, repo := f.Dependency.OwnerRepo()
 		if owner != "" {
-			out.Detail("  → %s", out.Dim(fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo)))
+			out.Detail("  ↳ %s", out.Dim(fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo)))
 		}
 	}
 	if IsAlertedCategory(f.Category) && f.Remediation != "" {
-		out.Detail("  %s %s", out.Bold("⚠"), f.Remediation)
+		out.Detail("  %s %s", ui.IconWarning, f.Remediation)
 	}
 	if f.RecommendedTag != "" {
 		nwo := ""
@@ -139,20 +150,79 @@ func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) {
 		if len(sha) > 7 {
 			sha = sha[:7]
 		}
-		out.Detail("  %s Suggested re-pin: %s@%s (%s) — latest release reachable from a branch",
-			out.Bold("→"), nwo, f.RecommendedTag, sha)
+		out.Detail("  ↳ Suggested re-pin: %s@%s (%s) — latest release reachable from a branch",
+			nwo, f.RecommendedTag, sha)
 	}
 	if f.Category == checks.ImpostorCommit {
-		out.Detail("  %s %s", out.Yellow("!"), pipeline.ImpostorCommitContext)
-		out.Detail("  %s %s", out.Bold("→"), pipeline.PublisherEscalationCopy)
+		out.Detail("  %s %s", ui.IconWarning, pipeline.ImpostorCommitContext)
+		out.Detail("  ↳ %s", pipeline.PublisherEscalationCopy)
 		out.Detail("  see: %s", out.DocLink(pipeline.PublisherTagReleasesDocURL))
 	}
 	if f.DocURL != "" {
 		out.Detail("  see: %s", out.DocLink(f.DocURL))
 	}
 }
 
-// warningGroup deduplicates warnings by dep key across workflows.
+// renderSelfHostedGroup prints a deduplicated block for self-hosted-runner
+// findings, listing each affected workflow and its non-hosted labels.
+func renderSelfHostedGroup(out *ui.UI, findings []checks.Finding) {
+	label := "SELF-HOSTED-RUNNER"
+	icon := "!"
+	if IsAlertedCategory(checks.SelfHostedRunner) {
+		icon = "✗"
+	}
+	out.Detail("%s %s", icon, out.Dim(label))
+	for _, f := range findings {
+		wfName := workflowName(f.WorkflowPath)
+		out.Detail("  %s: %s", out.Bold(wfName), f.Detail)
+	}
+	if IsAlertedCategory(checks.SelfHostedRunner) && findings[0].Remediation != "" {
+		out.Detail("  %s %s", ui.IconWarning, findings[0].Remediation)
+	}
+	labelSet := map[string]bool{}
+	for _, f := range findings {
+		for _, l := range extractBracketedLabels(f.Detail) {
+			labelSet[l] = true
+		}
+	}
+	if len(labelSet) > 0 {
+		var labels []string
+		for l := range labelSet {
+			labels = append(labels, l)
+		}
+		sort.Strings(labels)
+		out.Detail("  ↳ re-run with --allow-runners %s", strings.Join(labels, ","))
+	}
+}
+
+// workflowName extracts the workflow filename from a path like
+// ".github/workflows/ci.yml".
+func workflowName(path string) string {
+	if i := strings.LastIndex(path, "/"); i >= 0 {
+		return path[i+1:]
+	}
+	return path
+}
+
+// extractBracketedLabels pulls comma-separated items from the first
+// [...] group in s. Returns nil if no brackets are found.
+func extractBracketedLabels(s string) []string {
+	start := strings.Index(s, "[")
+	end := strings.Index(s, "]")
+	if start < 0 || end <= start {
+		return nil
+	}
+	inner := s[start+1 : end]
+	var labels []string
+	for _, l := range strings.Split(inner, ",") {
+		l = strings.TrimSpace(l)
+		if l != "" {
+			labels = append(labels, l)
+		}
+	}
+	return labels
+}
+
 type warningGroup struct {
 	finding   checks.Finding
 	count     int
@@ -229,6 +299,28 @@ func renderWarnings(out *ui.UI, report *checks.Report, willRemediate bool) {
 		out.TermCaution("%d %s skipped — non-hosted runner labels are not supported",
 			len(selfHostedRunnerWorkflows),
 			ui.Pluralize(len(selfHostedRunnerWorkflows), "workflow", "workflows"))
+		// Collect distinct labels from findings for the remediation hint.
+		labelSet := map[string]bool{}
+		for _, key := range warnOrder {
+			wg := warnMap[key]
+			if wg.finding.Category != checks.SelfHostedRunner {
+				continue
+			}
+			for _, l := range extractBracketedLabels(wg.finding.Detail) {
+				labelSet[l] = true
+			}
+		}
+		if len(labelSet) > 0 {
+			var labels []string
+			for l := range labelSet {
+				labels = append(labels, l)
+			}
+			sort.Strings(labels)
+			out.TermDetail("↳ if these are org-hosted larger runners, re-run with --allow-runners %s",
+				strings.Join(labels, ","))
+		} else {
+			out.TermDetail("↳ if these are org-hosted larger runners, re-run with --allow-runners <label>")
+		}
 	}
 	if len(unpinnedWorkflows) > 0 {
 		out.TermWarn("%d %s not yet pinned",

internal/ghapi/hosted_runners.go

@@ -0,0 +1,39 @@
+package ghapi
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/url"
+)
+
+// OrgHostedRunnerNames fetches runs-on labels of org-hosted runners.
+// Returns nil on permission errors (401/403) or 404 (user-owned repos)
+// so callers fall back to the static list.
+func (c *Client) OrgHostedRunnerNames(ctx context.Context, org string) ([]string, error) {
+	var names []string
+	page := 1
+	for {
+		path := fmt.Sprintf("orgs/%s/actions/hosted-runners?per_page=100&page=%d", url.PathEscape(org), page)
+		var resp struct {
+			TotalCount int `json:"total_count"`
+			Runners    []struct {
+				Name string `json:"name"`
+			} `json:"runners"`
+		}
+		if err := c.rest.DoWithContext(ctx, http.MethodGet, path, nil, &resp); err != nil {
+			if IsPermissionDenied(err) || IsNotFound(err) {
+				return nil, nil
+			}
+			return nil, fmt.Errorf("listing org hosted runners: %w", err)
+		}
+		for _, r := range resp.Runners {
+			names = append(names, r.Name)
+		}
+		if len(names) >= resp.TotalCount {
+			break
+		}
+		page++
+	}
+	return names, nil
+}

internal/ghapi/hosted_runners_test.go

@@ -0,0 +1,44 @@
+package ghapi
+
+import (
+	"context"
+	"testing"
+
+	"github.com/github/gh-actions-lock/internal/ghapi/httpmock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOrgHostedRunnerNames_Success(t *testing.T) {
+	reg := &httpmock.Registry{}
+	reg.Register(
+		httpmock.REST("GET", "orgs/my-org/actions/hosted-runners"),
+		httpmock.JSONResponse(map[string]any{
+			"total_count": 2,
+			"runners": []map[string]any{
+				{"name": "ubuntu-latest-xl"},
+				{"name": "ubuntu-latest-2xl"},
+			},
+		}),
+	)
+	c, err := New("github.com", WithClientTransport(reg))
+	require.NoError(t, err)
+
+	names, err := c.OrgHostedRunnerNames(context.Background(), "my-org")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"ubuntu-latest-xl", "ubuntu-latest-2xl"}, names)
+}
+
+func TestOrgHostedRunnerNames_403_ReturnsNil(t *testing.T) {
+	reg := &httpmock.Registry{}
+	reg.Register(
+		httpmock.REST("GET", "orgs/my-org/actions/hosted-runners"),
+		httpmock.StatusResponse(403),
+	)
+	c, err := New("github.com", WithClientTransport(reg))
+	require.NoError(t, err)
+
+	names, err := c.OrgHostedRunnerNames(context.Background(), "my-org")
+	assert.NoError(t, err)
+	assert.Nil(t, names)
+}

internal/pipeline/checks/parsed.go

@@ -42,6 +42,9 @@ type ParsedWorkflow struct {
 	// (self-hosted, custom label, runner group, or expression). These
 	// workflows are skipped from onboarding.
 	NonHostedRunner bool
+	// NonHostedLabels holds the specific non-hosted runner labels found
+	// in the workflow (populated only when NonHostedRunner is true).
+	NonHostedLabels []string
 }
 
 // PartitionRefs splits refs into recorded (matching a lockfile entry by

internal/pipeline/diagnose.go

@@ -5,6 +5,7 @@ package pipeline
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/github/gh-actions-lock/internal/dep"
 	"github.com/github/gh-actions-lock/internal/ghapi"
@@ -89,14 +90,15 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
 	}
 
 	if pw.NonHostedRunner {
+		labelList := strings.Join(pw.NonHostedLabels, ", ")
 		wfKey := workflowfile.KeyFromPath(pw.Path)
 		if store != nil && store.HasWorkflow(wfKey) {
 			wr.Findings = append(wr.Findings, checks.Finding{
 				WorkflowPath: pw.Path,
 				Category:     checks.SelfHostedRunner,
 				Severity:     checks.SeverityError,
 				Confidence:   checks.ConfidenceHigh,
-				Detail:       "workflow uses non-hosted runner labels which are not supported; use GitHub-hosted runners to continue using the lockfile",
+				Detail:       fmt.Sprintf("uses non-hosted runner labels [%s]; use GitHub-hosted runners to continue using the lockfile", labelList),
 				Remediation:  "switch to GitHub-hosted runner labels or move self-hosted jobs to a separate workflow",
 			})
 		} else {
@@ -105,7 +107,7 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
 				Category:     checks.SelfHostedRunner,
 				Severity:     checks.SeverityWarning,
 				Confidence:   checks.ConfidenceHigh,
-				Detail:       "workflow uses non-hosted runner labels; lockfile onboarding is not supported",
+				Detail:       fmt.Sprintf("uses non-hosted runner labels [%s]; lockfile onboarding is not supported", labelList),
 			})
 		}
 		return wr

internal/pipeline/diagnose_test.go

@@ -52,12 +52,14 @@ func TestDiagnoseOneParsed_SelfHostedRunner_NotOnboarded(t *testing.T) {
 	pw := checks.ParsedWorkflow{
 		Path:            ".github/workflows/ci.yml",
 		NonHostedRunner: true,
+		NonHostedLabels: []string{"self-hosted", "linux"},
 	}
 	wr := diagnoseOneParsed(context.Background(), pw, nil, nil, nil)
 
 	assert.Len(t, wr.Findings, 1)
 	assert.Equal(t, checks.SelfHostedRunner, wr.Findings[0].Category)
 	assert.Equal(t, checks.SeverityWarning, wr.Findings[0].Severity)
+	assert.Contains(t, wr.Findings[0].Detail, "self-hosted, linux")
 }
 
 func TestDiagnoseOneParsed_SelfHostedRunner_AlreadyOnboarded(t *testing.T) {
@@ -71,6 +73,7 @@ func TestDiagnoseOneParsed_SelfHostedRunner_AlreadyOnboarded(t *testing.T) {
 	pw := checks.ParsedWorkflow{
 		Path:            wfKey,
 		NonHostedRunner: true,
+		NonHostedLabels: []string{"self-hosted"},
 	}
 	wr := diagnoseOneParsed(context.Background(), pw, nil, store, nil)
 

internal/pipeline/parse.go

@@ -48,7 +48,8 @@ func ParseAll(paths []string, store *lockfile.State) []checks.ParsedWorkflow {
 			continue
 		}
 		pw.Refs, pw.LocalPaths, pw.ParseWarnings = wf.ExtractActionRefs()
-		pw.NonHostedRunner = wf.HasNonHostedRunnerLabels()
+		pw.NonHostedLabels = wf.NonHostedRunnerLabels()
+		pw.NonHostedRunner = len(pw.NonHostedLabels) > 0
 		if len(pw.Refs) > 0 {
 			wfKey := workflowfile.KeyFromPath(path)
 			deps, depsErr := store.Get(wfKey)