Skip to content

Commit 0656836

Browse files
Record dependency-free workflows in the lockfile (#124)
* Record dependency-free workflows Dependency resolution failures and workflows without dependencies can both produce empty dependency slices. Preserve resolver failure state through planning and pass the existing no-onboard policy to Commit so it can distinguish these cases before writing new lockfile entries. This allows dependency-free workflows to receive empty lockfile entries without adding entries for unresolved or skipped workflows. * Fix plan test compilation Remove a duplicate lockfile import and update the stale newSlowPathFixtures call to match its current signature. * Add dependency-free workflow tests Verify that ResolveAllRecursive errors remain available in workflow plans and that successful partial results are committed. Cover recording a dependency-free workflow while skipping unresolved workflows and new workflow entries disabled by the caller. * Skip rewrites for new workflows Filter workflows with no existing lockfile entry before the Commit phase rewrites workflow or self-action files when new entries are disabled. Add a regression test that verifies both source files and the lockfile remain unchanged. * Test blocking resolver error exclusion Verify that planning excludes workflow reports marked with a blocking resolver error. --------- Co-authored-by: Jeff Martin <nodeselector@github.com>
1 parent 152cf29 commit 0656836

7 files changed

Lines changed: 110 additions & 5 deletions

File tree

‎cmd/gh-actions-lock/run.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
433433
// Commit: write all changes to disk atomically (fast local I/O, no
434434
// spinner label — it finishes before the user could read one).
435435
endCommit := prof.Phase("pin.Commit (disk writes)")
436-
if err := pin.Commit(ctx, record, store, nil); err != nil {
436+
if err := pin.Commit(ctx, record, store, &pin.CommitOptions{SkipNewWorkflowEntries: noOnboardFlag(cmd)}); err != nil {
437437
console.StopProgress()
438438
return fmt.Errorf("committing pins: %w", err)
439439
}

‎internal/pin/commit.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
type CommitOptions struct {
1919
// OnProgress is called at each phase boundary. Nil means no progress.
2020
OnProgress func(phase string)
21+
// SkipNewWorkflowEntries forces the Commit phase to skip workflows with no existing lockfile entry.
22+
SkipNewWorkflowEntries bool
2123
}
2224

2325
// Commit writes a planned Record to disk: rewrites workflow files and
@@ -29,6 +31,15 @@ func Commit(ctx context.Context, rec *Record, store *lockfile.State, copts *Comm
2931
if copts != nil && copts.OnProgress != nil {
3032
progress = copts.OnProgress
3133
}
34+
if copts != nil && copts.SkipNewWorkflowEntries {
35+
workflows := rec.Workflows[:0]
36+
for _, wp := range rec.Workflows {
37+
if store.HasWorkflow(workflowfile.KeyFromPath(wp.Path)) {
38+
workflows = append(workflows, wp)
39+
}
40+
}
41+
rec.Workflows = workflows
42+
}
3243

3344
// Phase 1: Rewrite workflow files (uses: line changes).
3445
if len(rec.Workflows) > 0 {
@@ -67,9 +78,6 @@ func Commit(ctx context.Context, rec *Record, store *lockfile.State, copts *Comm
6778
wfPath := wp.Path
6879
wfKey := workflowfile.KeyFromPath(wfPath)
6980
deps := pinnedByWorkflow[wfPath]
70-
if len(deps) == 0 && !store.HasWorkflow(wfKey) {
71-
continue
72-
}
7381
parentMap := buildParentMap(rec, wfPath)
7482
directKeys := buildDirectKeys(rec, wfPath)
7583
deps = retainUnresolvablePins(rec, store, wfPath, deps, directKeys)

‎internal/pin/commit_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,88 @@ func TestBuildDirectKeys(t *testing.T) {
109109
assert.NotContains(t, keys, "g/h@v4", "investigate should be excluded")
110110
}
111111

112+
func TestCommitDependencyFreeWorkflow(t *testing.T) {
113+
tests := []struct {
114+
name string
115+
skipNewWorkflowEntries bool
116+
wantEntry bool
117+
}{
118+
{"records empty entry", false, true},
119+
{"skips onboarding", true, false},
120+
}
121+
for _, tt := range tests {
122+
t.Run(tt.name, func(t *testing.T) {
123+
dir := t.TempDir()
124+
workflowPath := filepath.Join(".github", "workflows", "ci.yml")
125+
require.NoError(t, os.MkdirAll(filepath.Join(dir, filepath.Dir(workflowPath)), 0o755))
126+
require.NoError(t, os.WriteFile(filepath.Join(dir, workflowPath), []byte("on: push\n"), 0o644))
127+
t.Chdir(dir)
128+
129+
store, err := lockfile.LoadState(dir, fakeMeta{})
130+
require.NoError(t, err)
131+
rec := &Record{Workflows: []WorkflowPlan{{Path: workflowPath}}}
132+
133+
require.NoError(t, Commit(context.Background(), rec, store, &CommitOptions{SkipNewWorkflowEntries: tt.skipNewWorkflowEntries}))
134+
assert.Equal(t, tt.wantEntry, store.HasWorkflow(workflowPath))
135+
})
136+
}
137+
}
138+
139+
func TestCommitPartialResolution(t *testing.T) {
140+
dir := t.TempDir()
141+
workflowPath := filepath.Join(".github", "workflows", "ci.yml")
142+
require.NoError(t, os.MkdirAll(filepath.Join(dir, filepath.Dir(workflowPath)), 0o755))
143+
require.NoError(t, os.WriteFile(filepath.Join(dir, workflowPath), []byte("on: push\n"), 0o644))
144+
t.Chdir(dir)
145+
146+
store, err := lockfile.LoadState(dir, fakeMeta{})
147+
require.NoError(t, err)
148+
rec := &Record{
149+
Entries: []Entry{{
150+
NWO: "actions/checkout", Ref: "v4", SHA: strings.Repeat("a", 40),
151+
Resolution: Pinned, Workflows: []string{workflowPath}, Direct: true,
152+
}},
153+
Workflows: []WorkflowPlan{{Path: workflowPath}},
154+
}
155+
156+
require.NoError(t, Commit(context.Background(), rec, store, nil))
157+
deps, err := store.Get(workflowPath)
158+
require.NoError(t, err)
159+
assert.Len(t, deps, 1)
160+
}
161+
162+
func TestCommitSkipNewWorkflowEntriesPreventsRewrites(t *testing.T) {
163+
dir := t.TempDir()
164+
workflowPath := filepath.Join(".github", "workflows", "ci.yml")
165+
actionPath := filepath.Join(".github", "actions", "local", "action.yml")
166+
oldUses := "actions/checkout@" + strings.Repeat("a", 40)
167+
newUses := "actions/checkout@v4.2.0"
168+
workflowContent := []byte("on: push\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - uses: " + oldUses + "\n")
169+
actionContent := []byte("name: local\nruns:\n using: composite\n steps:\n - uses: " + oldUses + "\n")
170+
for path, content := range map[string][]byte{workflowPath: workflowContent, actionPath: actionContent} {
171+
require.NoError(t, os.MkdirAll(filepath.Join(dir, filepath.Dir(path)), 0o755))
172+
require.NoError(t, os.WriteFile(filepath.Join(dir, path), content, 0o644))
173+
}
174+
t.Chdir(dir)
175+
176+
store, err := lockfile.LoadState(dir, fakeMeta{})
177+
require.NoError(t, err)
178+
rec := &Record{Workflows: []WorkflowPlan{{
179+
Path: workflowPath,
180+
Rewrites: map[string]string{oldUses: newUses},
181+
SelfActionFiles: []string{actionPath},
182+
}}}
183+
184+
require.NoError(t, Commit(context.Background(), rec, store, &CommitOptions{SkipNewWorkflowEntries: true}))
185+
workflowAfter, err := os.ReadFile(workflowPath)
186+
require.NoError(t, err)
187+
actionAfter, err := os.ReadFile(actionPath)
188+
require.NoError(t, err)
189+
assert.Equal(t, workflowContent, workflowAfter)
190+
assert.Equal(t, actionContent, actionAfter)
191+
assert.False(t, store.HasWorkflow(workflowPath))
192+
}
193+
112194
func TestCommitRemovesDependenciesDroppedFromWorkflow(t *testing.T) {
113195
dir := t.TempDir()
114196
workflowPath := filepath.Join(".github", "workflows", "ci.yml")

‎internal/pin/plan.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ type planResult struct {
135135
func planWorkflow(ctx context.Context, wr checks.WorkflowReport, opts PlanOptions, status func(string)) (planResult, error) {
136136
var entries []Entry
137137
var wplans []WorkflowPlan
138-
if wr.SkipCommit {
138+
if wr.SkipCommit || wr.BlockingResolverError {
139139
return planResult{entries: verifiedEntries(wr.Inventory, wr.Path)}, nil
140140
}
141141
for _, finding := range wr.Findings {

‎internal/pin/plan_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,18 @@ func TestPlanExcludesLoadFailuresFromCommit(t *testing.T) {
592592
assert.Equal(t, blocked.Path, record.Entries[0].Workflows[0])
593593
}
594594

595+
func TestPlanExcludesBlockingResolverErrorsFromCommit(t *testing.T) {
596+
record, err := Plan(context.Background(), &checks.Report{
597+
Workflows: []checks.WorkflowReport{{
598+
Path: ".github/workflows/ci.yml",
599+
BlockingResolverError: true,
600+
}},
601+
}, PlanOptions{Pool: pinpool.New(2, nil)})
602+
require.NoError(t, err)
603+
604+
assert.Empty(t, record.Workflows)
605+
}
606+
595607
func TestPlanWorkflow_SelfRepositoryDependencyIsNotRewrittenOnFastPath(t *testing.T) {
596608
const sha = "abc1230000000000000000000000000000000000"
597609

‎internal/pipeline/checks/finding.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ type WorkflowReport struct {
6565
Findings []Finding
6666
// SkipCommit prevents terminal parse failures from entering the write phase.
6767
SkipCommit bool
68+
// BlockingResolverError indicates that diagnosis classified a resolver error as blocking.
69+
BlockingResolverError bool
6870
// ActionRefs are all remote dependency roots attributed to the workflow,
6971
// including refs found inside in-repo `$/…` actions.
7072
ActionRefs []parserlock.ActionRef

‎internal/pipeline/diagnose.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
102102
blockingResolverError = true
103103
}
104104
if blockingResolverError {
105+
wr.BlockingResolverError = true
105106
return wr
106107
}
107108
// Low: we're surfacing the resolver failure itself, not a

0 commit comments

Comments
 (0)