Give missing refs and invalid workflows their own JSON categories; report post-fix state in fix-mode JSON (#232)

* Give missing refs and invalid workflows their own JSON categories; report post-fix state in fix-mode JSON

Missing uses: refs are now ref-missing and unloadable workflows
invalid-workflow, instead of not-pinned. A successful fix run's --json
reports what the commit left: errors it fixed are dropped and valid is
recomputed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Expect ref-missing in the impostor-commit scenario

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: nodeselector

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

@@ -92,8 +92,7 @@ type Workflow struct {
 	Dependencies []Dependency `json:"dependencies,omitempty"`
 }
 
-// Pin is the post-fix outcome for one action from the pin plan. The other
-// fields describe the pre-fix diagnosis.
+// Pin is the post-fix outcome for one action from the pin plan.
 type Pin struct {
 	Hostname     string `json:"hostname,omitempty"`
 	NWO          string `json:"nwo"`

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

@@ -65,7 +65,7 @@ func PresentResults(out *ui.UI, report *checks.Report, valid bool, willRemediate
 
 func findingExcluded(f checks.Finding, exclude map[checks.Category]bool, willRemediate bool) bool {
 	if f.Category == checks.NotPinned && !f.IsRemediableNotPinned() {
-		// Workflow-level NotPinned is a load failure, not a pin finding.
+		// A non-remediable NotPinned can't be fixed by pinning.
 		return false
 	}
 	return exclude[f.Category] || willRemediate && f.Category == checks.NotPinned
@@ -282,6 +282,10 @@ func categoryLabel(c checks.Category) string {
 	switch c {
 	case checks.NotPinned:
 		return "Not pinned"
+	case checks.MissingRef:
+		return "Ref does not exist"
+	case checks.InvalidWorkflow:
+		return "Invalid workflow"
 	case checks.RefChanged:
 		return "Ref changed"
 	case checks.MisleadingSHA:
@@ -397,7 +401,7 @@ func renderErrorFindings(out *ui.UI, report *checks.Report, failedCount, checked
 	parts := []string{}
 	for _, cat := range []checks.Category{
 		checks.RepoReplaced, checks.RepoUnavailable, checks.RepoRenamed, checks.UnreachablePin, checks.TagCommitMismatch,
-		checks.RefChanged, checks.NotPinned, checks.OnboardingRequired,
+		checks.RefChanged, checks.NotPinned, checks.MissingRef, checks.InvalidWorkflow, checks.OnboardingRequired,
 		checks.LocalAction, checks.InvalidSelfRepositoryRef,
 		checks.Stale, checks.MisleadingSHA, checks.IncompleteLockfile,
 	} {

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

@@ -445,7 +445,7 @@ func TestPresentResults_ExcludedNotPinnedKeepsWorkflowLoadErrors(t *testing.T) {
 			{
 				Path: ".github/workflows/broken.yml",
 				Findings: []checks.Finding{{
-					Category: checks.NotPinned,
+					Category: checks.InvalidWorkflow,
 					Severity: checks.SeverityError,
 					Detail:   "failed to load workflow",
 				}},
@@ -456,7 +456,7 @@ func TestPresentResults_ExcludedNotPinnedKeepsWorkflowLoadErrors(t *testing.T) {
 	PresentResults(ui.NewPlain(&buf), report, false, false, checks.NotPinned)
 
 	got := buf.String()
-	if !strings.Contains(got, "1 of 2 workflows failed: 1 not-pinned") {
+	if !strings.Contains(got, "1 of 2 workflows failed: 1 invalid-workflow") {
 		t.Errorf("workflow load error should remain after remediation:\n%s", got)
 	}
 	if !strings.Contains(got, ".github/workflows/broken.yml") ||

cmd/gh-actions-lock/missing_ref_test.go

@@ -78,7 +78,7 @@ jobs:
 	require.NoError(t, json.Unmarshal([]byte(stdout), &out), stdout)
 	found := false
 	for _, f := range out.Findings {
-		if f.Category == "not-pinned" && f.Severity == "error" {
+		if f.Category == "ref-missing" && f.Severity == "error" {
 			found = true
 			assert.Equal(t, detail, f.Detail)
 			assert.Equal(t, remedy, f.Remediation)

cmd/gh-actions-lock/pin_summary.go

@@ -29,7 +29,7 @@ func reportHasUnfixableErrors(report *checks.Report, acceptMoved bool) bool {
 				continue
 			}
 			switch f.Category {
-			case checks.LocalAction, checks.InvalidSelfRepositoryRef, checks.RepoRenamed, checks.RepoReplaced, checks.RepoUnavailable, checks.ReachabilityUnknown, checks.AncestryUnknown:
+			case checks.LocalAction, checks.InvalidSelfRepositoryRef, checks.MissingRef, checks.InvalidWorkflow, checks.RepoRenamed, checks.RepoReplaced, checks.RepoUnavailable, checks.ReachabilityUnknown, checks.AncestryUnknown:
 				return true
 			case checks.NotPinned:
 				if !f.IsRemediableNotPinned() {
@@ -86,6 +86,8 @@ func reportHasNonInvestigatedUnfixableErrors(report *checks.Report) bool {
 			}
 			if f.Category == checks.LocalAction ||
 				f.Category == checks.InvalidSelfRepositoryRef ||
+				f.Category == checks.MissingRef ||
+				f.Category == checks.InvalidWorkflow ||
 				f.Category == checks.RepoRenamed ||
 				f.Category == checks.RepoReplaced ||
 				f.Category == checks.RepoUnavailable ||

cmd/gh-actions-lock/pin_summary_test.go

@@ -62,17 +62,20 @@ func TestRenderNarrowedEntries_BranchRepair(t *testing.T) {
 func TestReportHasUnfixableErrors_ClassifiesWorkflowNotPinned(t *testing.T) {
 	for _, tt := range []struct {
 		name        string
+		category    checks.Category
 		remediable  bool
 		wantBlocked bool
 	}{
-		{name: "workflow load error", wantBlocked: true},
-		{name: "dependency inventory error", remediable: true},
+		{name: "malformed nested ref", category: checks.NotPinned, wantBlocked: true},
+		{name: "dependency inventory error", category: checks.NotPinned, remediable: true},
+		{name: "workflow load error", category: checks.InvalidWorkflow, wantBlocked: true},
+		{name: "missing ref", category: checks.MissingRef, wantBlocked: true},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			report := &checks.Report{
 				Workflows: []checks.WorkflowReport{{
 					Findings: []checks.Finding{{
-						Category:   checks.NotPinned,
+						Category:   tt.category,
 						Severity:   checks.SeverityError,
 						Remediable: tt.remediable,
 					}},

cmd/gh-actions-lock/run.go

@@ -565,17 +565,37 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
 		injectFreshTagFindings(ctx, report, record, tagger, cooldownCfg)
 	}
 
-	// JSON mode emits the (pre-fix) diagnosis now — after the commit
-	// succeeded — so machine consumers never see findings for a run that
-	// then failed to write.
+	// JSON mode reports the state the commit left, and only after it
+	// succeeded, so machine consumers never see findings for a run that
+	// then failed to write. The run log keeps the pre-fix diagnosis.
 	if opts.jsonFields != "" {
-		return format.WriteJSON(out, report, record, valid, opts.jsonFields, cliVersion(), store.File().Version, r.Hostname())
+		fixed := postFixReport(report)
+		return format.WriteJSON(out, fixed, record, fixed.IsValid(), opts.jsonFields, cliVersion(), store.File().Version, r.Hostname())
 	}
 
 	// Terminal summary.
 	return renderPinSummary(ctx, console, record, report, r, refusedLabels, opts.noNarrow, opts.acceptMoved, store.OriginalVersion(), staleWorkflows)
 }
 
+// postFixReport is report as a committed write left it. A run commits only
+// when it can fix every error it found (see runHasBlockingErrors), so no error
+// stands afterwards; warnings and info findings remain.
+func postFixReport(report *checks.Report) *checks.Report {
+	unfixed := func(ff []checks.Finding) []checks.Finding {
+		return slices.DeleteFunc(slices.Clone(ff), func(f checks.Finding) bool {
+			return f.Severity == checks.SeverityError
+		})
+	}
+	out := *report
+	out.RepoFindings = unfixed(report.RepoFindings)
+	out.Workflows = make([]checks.WorkflowReport, len(report.Workflows))
+	for i, wr := range report.Workflows {
+		wr.Findings = unfixed(wr.Findings)
+		out.Workflows[i] = wr
+	}
+	return &out
+}
+
 // injectVersionRefFindings appends info-severity findings for entries pinned
 // with a non-full-semver ref (v4, v3.1, main, etc.). These surface in --json
 // output so machine consumers can detect imprecise refs.

cmd/gh-actions-lock/run_test.go

@@ -97,11 +97,12 @@ updates:
 						assert.True(t, payload.Valid)
 						assert.Empty(t, payload.Findings)
 					} else {
-						require.Len(t, payload.Findings, 2)
+						// The fix pinned the action; only the warning remains.
+						assert.True(t, payload.Valid)
+						require.Len(t, payload.Findings, 1)
 						assert.Equal(t, "cooldown-config-ignored", payload.Findings[0].Category)
 						assert.Equal(t, "warning", payload.Findings[0].Severity)
 						assert.Equal(t, warning, payload.Findings[0].Detail)
-						assert.Equal(t, "not-pinned", payload.Findings[1].Category)
 					}
 				} else if mode.readOnly {
 					assert.NotContains(t, stdout+stderr, warning)
@@ -918,3 +919,77 @@ func TestRun_LocalActionErrorLeadsTheOutput(t *testing.T) {
 		})
 	}
 }
+
+func TestPostFixReport(t *testing.T) {
+	report := &checks.Report{
+		Workflows: []checks.WorkflowReport{{
+			Path: ".github/workflows/ci.yml",
+			Findings: []checks.Finding{
+				{Category: checks.NotPinned, Severity: checks.SeverityError},
+				{Category: checks.FreshTag, Severity: checks.SeverityInfo},
+				{Category: checks.LocalAction, Severity: checks.SeverityWarning},
+			},
+		}},
+		RepoFindings: []checks.Finding{
+			{Category: checks.Stale, Severity: checks.SeverityError},
+			{Category: checks.StaleWorkflow, Severity: checks.SeverityInfo},
+		},
+	}
+	require.False(t, report.IsValid())
+
+	got := postFixReport(report)
+
+	assert.True(t, got.IsValid())
+	var cats []checks.Category
+	for _, f := range got.Workflows[0].Findings {
+		cats = append(cats, f.Category)
+	}
+	assert.Equal(t, []checks.Category{checks.FreshTag, checks.LocalAction}, cats, "non-blocking findings stay")
+	require.Len(t, got.RepoFindings, 1)
+	assert.Equal(t, checks.StaleWorkflow, got.RepoFindings[0].Category)
+	assert.Len(t, report.Workflows[0].Findings, 3, "the run log keeps the pre-fix report")
+	assert.Len(t, report.RepoFindings, 2)
+}
+
+func TestRun_FixJSONReportsPostFixState(t *testing.T) {
+	const sha = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+	path := writeTempWorkflow(t, `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: example/action@main
+`)
+	t.Setenv("GH_ACTIONS_LOCK_CONFIG", filepath.Join(t.TempDir(), "none.yml"))
+	reg := &httpmock.Registry{}
+	reg.Register(httpmock.GraphQLForRepo("example", "action"), httpmock.JSONResponse(map[string]any{
+		"data": map[string]any{"a0": testRepoResponse("example/action", sha, nodeActionYAML)},
+	}))
+	reg.Register(httpmock.REST("GET", "repos/example/action$"), httpmock.JSONResponse(map[string]any{
+		"id": 2, "owner": map[string]any{"id": 1}, "default_branch": "main",
+	}))
+
+	stdout, stderr, err := runCommandWithHTTP(t, reg, "--no-interactive", "--json=valid,findings,workflows,pins", path)
+	require.NoError(t, err, "stderr:\n%s", stderr)
+
+	var payload struct {
+		Valid     bool              `json:"valid"`
+		Findings  []format.Finding  `json:"findings"`
+		Workflows []format.Workflow `json:"workflows"`
+		Pins      []format.Pin      `json:"pins"`
+	}
+	require.NoError(t, json.Unmarshal([]byte(stdout), &payload), stdout)
+	assert.True(t, payload.Valid, stdout)
+	for _, f := range payload.Findings {
+		assert.NotEqual(t, "error", f.Severity, "%+v", f)
+	}
+	require.Len(t, payload.Workflows, 1)
+	assert.True(t, payload.Workflows[0].Valid)
+	require.Len(t, payload.Pins, 1)
+	assert.Equal(t, "example/action", payload.Pins[0].NWO)
+	assert.Equal(t, sha, payload.Pins[0].SHA)
+	assert.Contains(t, readTempLockfilePins(t), sha)
+	reg.Verify(t)
+}

cmd/gh-actions-lock/verify_test.go

@@ -210,10 +210,15 @@ jobs:
 					stale = append(stale, f)
 				}
 			}
-			require.Len(t, stale, 1, "%+v", p.Findings)
-			assert.Equal(t, "actions/setup-go@v5", stale[0].Dependency)
-			assert.Equal(t, string(checks.SeverityError), stale[0].Severity)
 			assert.Equal(t, !hasError, p.Valid, "valid must agree with severities")
+			if !tc.wantErr {
+				assert.Empty(t, stale, "a fix run reports the state it left")
+				assert.True(t, p.Valid)
+			} else {
+				require.Len(t, stale, 1, "%+v", p.Findings)
+				assert.Equal(t, "actions/setup-go@v5", stale[0].Dependency)
+				assert.Equal(t, string(checks.SeverityError), stale[0].Severity)
+			}
 
 			pins := readTempLockfilePins(t)
 			if tc.wantErr {
@@ -293,7 +298,7 @@ jobs:
 	}{
 		{name: "verify untracked", mode: []string{"--verify"}, otherPins: []string{pin}, wantCategory: checks.NotPinned, wantErr: true},
 		{name: "verify-local untracked", mode: []string{"--verify-local"}, otherPins: []string{pin}, wantCategory: checks.NotPinned, wantErr: true},
-		{name: "fix run adds entry", otherPins: []string{pin}, wantCategory: checks.NotPinned, wantEntry: "'.github/workflows/workflow.yml': []"},
+		{name: "fix run adds entry", otherPins: []string{pin}, wantEntry: "'.github/workflows/workflow.yml': []"},
 		{name: "no-onboard leaves it", mode: []string{"--no-onboard"}, otherPins: []string{pin}, wantCategory: checks.OnboardingRequired},
 		{name: "verify no-onboard opts out", mode: []string{"--verify", "--no-onboard"}, otherPins: []string{pin}, wantCategory: checks.OnboardingRequired},
 		{name: "verify unused pin", mode: []string{"--verify"}, ownPins: []string{pin}, wantCategory: checks.Stale, wantErr: true},
@@ -319,11 +324,7 @@ jobs:
 				hasError = hasError || f.Severity == string(checks.SeverityError)
 				cats = append(cats, f.Category)
 			}
-			// Fix runs report the pre-fix diagnosis, so only the read-only
-			// modes tie errors to the exit code.
-			if len(tc.mode) > 0 {
-				assert.Equal(t, tc.wantErr, hasError, "%+v", p.Findings)
-			}
+			assert.Equal(t, tc.wantErr, hasError, "%+v", p.Findings)
 			assert.Equal(t, !hasError, p.Valid, "valid must agree with severities")
 			if tc.wantCategory != "" {
 				assert.Contains(t, cats, string(tc.wantCategory))

internal/pipeline/checks/category.go

@@ -11,6 +11,12 @@ const (
 	// NotPinned means the workflow has action refs but no
 	// corresponding dependencies entry in the lockfile.
 	NotPinned Category = "not-pinned"
+	// MissingRef means a `uses:` ref (tag, branch, or commit) does not exist
+	// upstream, so there is nothing to pin. Only a source change fixes it.
+	MissingRef Category = "ref-missing"
+	// InvalidWorkflow means a workflow file failed to load, so none of its
+	// actions could be checked.
+	InvalidWorkflow Category = "invalid-workflow"
 	// ShaAsRef means a dependency is pinned to a bare SHA with no
 	// human-readable tag ref alongside it.
 	ShaAsRef Category = "sha-as-ref"