Skip to content

Commit ff2eccd

Browse files
authored
[WIP] Fix failing GitHub Actions job update (#61938)
1 parent 0045747 commit ff2eccd

2 files changed

Lines changed: 163 additions & 0 deletions

File tree

‎pkg/cli/update_command_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"strings"
1010
"testing"
11+
"time"
1112

1213
"github.com/github/gh-aw/pkg/testutil"
1314
"github.com/github/gh-aw/pkg/workflow"
@@ -1056,6 +1057,153 @@ func TestUpdateWorkflow_NoMergeMode(t *testing.T) {
10561057
})
10571058
}
10581059

1060+
// TestUpdateWorkflow_RestoresContentOnCompileFailure verifies that when an
1061+
// upstream update produces content that fails to compile (for example, a new
1062+
// dispatch-workflow target that doesn't exist locally), the local workflow
1063+
// file is restored to its previous, working content instead of being left in
1064+
// a broken state on disk.
1065+
func TestUpdateWorkflow_RestoresContentOnCompileFailure(t *testing.T) {
1066+
originalResolveLatestRef := resolveLatestRefFn
1067+
originalDownloadWorkflow := downloadWorkflowContentFn
1068+
t.Cleanup(func() {
1069+
resolveLatestRefFn = originalResolveLatestRef
1070+
downloadWorkflowContentFn = originalDownloadWorkflow
1071+
})
1072+
1073+
tmpDir := testutil.TempDir(t, "test-*")
1074+
require.NoError(t, initTestGitRepo(tmpDir), "failed to initialize git repo")
1075+
1076+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
1077+
require.NoError(t, os.MkdirAll(workflowsDir, 0755), "failed to create workflows dir")
1078+
1079+
oldDir, err := os.Getwd()
1080+
require.NoError(t, err, "failed to get current directory")
1081+
t.Cleanup(func() {
1082+
require.NoError(t, os.Chdir(oldDir), "failed to restore directory")
1083+
})
1084+
require.NoError(t, os.Chdir(tmpDir), "failed to change to temp directory")
1085+
1086+
const currentRef = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
1087+
const latestRef = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
1088+
1089+
originalContent := `---
1090+
on: issues
1091+
engine: copilot
1092+
permissions:
1093+
contents: read
1094+
source: owner/repo/workflows/test-workflow.md@` + currentRef + `
1095+
---
1096+
1097+
# Test Workflow
1098+
1099+
Original content.
1100+
`
1101+
1102+
// Simulate an upstream update that introduces a dispatch-workflow target
1103+
// that doesn't exist locally — this must fail to compile.
1104+
brokenNewContent := `---
1105+
on: issues
1106+
engine: copilot
1107+
permissions:
1108+
contents: read
1109+
safe-outputs:
1110+
dispatch-workflow:
1111+
workflows:
1112+
- missing-workflow
1113+
max: 1
1114+
source: owner/repo/workflows/test-workflow.md@` + latestRef + `
1115+
---
1116+
1117+
# Test Workflow
1118+
1119+
Updated content referencing a workflow that doesn't exist locally.
1120+
`
1121+
1122+
workflowFile := filepath.Join(workflowsDir, "test-workflow.md")
1123+
require.NoError(t, os.WriteFile(workflowFile, []byte(originalContent), 0644), "failed to write initial workflow")
1124+
1125+
resolveLatestRefFn = func(_ context.Context, _ string, ref string, _ bool, _ bool, _ time.Duration) (latestRefResolution, error) {
1126+
if ref == currentRef {
1127+
return latestRefResolution{Ref: latestRef}, nil
1128+
}
1129+
return latestRefResolution{Ref: ref}, nil
1130+
}
1131+
downloadWorkflowContentFn = func(_ context.Context, _, _, ref string, _ bool) ([]byte, error) {
1132+
if ref == latestRef {
1133+
return []byte(brokenNewContent), nil
1134+
}
1135+
return []byte(originalContent), nil
1136+
}
1137+
1138+
wf := &workflowWithSource{
1139+
Name: "test-workflow",
1140+
Path: workflowFile,
1141+
SourceSpec: "owner/repo/workflows/test-workflow.md@" + currentRef,
1142+
}
1143+
1144+
opts := UpdateWorkflowsOptions{
1145+
WorkflowsDir: workflowsDir,
1146+
}
1147+
1148+
err = updateWorkflow(context.Background(), wf, opts)
1149+
require.Error(t, err, "update should fail because the new content doesn't compile")
1150+
assert.Contains(t, err.Error(), "failed to compile updated workflow")
1151+
1152+
restoredContent, readErr := os.ReadFile(workflowFile)
1153+
require.NoError(t, readErr, "failed to read workflow file after failed update")
1154+
assert.Equal(t, originalContent, string(restoredContent), "workflow file should be restored to its original content after a compile failure")
1155+
}
1156+
1157+
func TestUpdateWorkflow_BackupReadFailureStopsUpdate(t *testing.T) {
1158+
originalResolveLatestRef := resolveLatestRefFn
1159+
originalDownloadWorkflow := downloadWorkflowContentFn
1160+
t.Cleanup(func() {
1161+
resolveLatestRefFn = originalResolveLatestRef
1162+
downloadWorkflowContentFn = originalDownloadWorkflow
1163+
})
1164+
1165+
tmpDir := testutil.TempDir(t, "test-*")
1166+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
1167+
require.NoError(t, os.MkdirAll(workflowsDir, 0755), "failed to create workflows dir")
1168+
1169+
const currentRef = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
1170+
const latestRef = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
1171+
updatedContent := []byte(`---
1172+
on: issues
1173+
engine: copilot
1174+
permissions:
1175+
contents: read
1176+
source: owner/repo/workflows/test-workflow.md@` + latestRef + `
1177+
---
1178+
1179+
# Updated Workflow
1180+
`)
1181+
1182+
resolveLatestRefFn = func(_ context.Context, _ string, ref string, _ bool, _ bool, _ time.Duration) (latestRefResolution, error) {
1183+
if ref == currentRef {
1184+
return latestRefResolution{Ref: latestRef}, nil
1185+
}
1186+
return latestRefResolution{Ref: ref}, nil
1187+
}
1188+
downloadWorkflowContentFn = func(_ context.Context, _, _, _ string, _ bool) ([]byte, error) {
1189+
return updatedContent, nil
1190+
}
1191+
1192+
workflowFile := filepath.Join(workflowsDir, "missing-workflow.md")
1193+
wf := &workflowWithSource{
1194+
Name: "missing-workflow",
1195+
Path: workflowFile,
1196+
SourceSpec: "owner/repo/workflows/test-workflow.md@" + currentRef,
1197+
}
1198+
1199+
err := updateWorkflow(context.Background(), wf, UpdateWorkflowsOptions{
1200+
WorkflowsDir: workflowsDir,
1201+
NoMerge: true,
1202+
})
1203+
require.ErrorContains(t, err, "failed to read current workflow before update")
1204+
assert.NoFileExists(t, workflowFile, "workflow must not be written without a recoverable backup")
1205+
}
1206+
10591207
// TestMarshalActionsLockSorted tests that the actions lock marshaling produces sorted output
10601208
// using the ActionCache.Save helper.
10611209
func TestMarshalActionsLockSorted(t *testing.T) {

‎pkg/cli/update_workflows.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,15 @@ func updateWorkflow(ctx context.Context, wf *workflowWithSource, opts UpdateWork
935935
fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Security scanning disabled"))
936936
}
937937

938+
// Preserve the pre-update content so it can be restored if the newly
939+
// fetched content fails to compile. Without this, a broken upstream
940+
// change (e.g. a dispatch-workflow target that doesn't exist locally)
941+
// would be left on disk and could break a later, unrelated recompile.
942+
originalContent, err := os.ReadFile(wf.Path)
943+
if err != nil {
944+
return fmt.Errorf("failed to read current workflow before update: %w", err)
945+
}
946+
938947
// Write updated content
939948
if err := os.WriteFile(wf.Path, []byte(finalContent), constants.FilePermPublic); err != nil {
940949
return fmt.Errorf("failed to write updated workflow: %w", err)
@@ -953,6 +962,12 @@ func updateWorkflow(ctx context.Context, wf *workflowWithSource, opts UpdateWork
953962
updateLog.Printf("Compiling updated workflow: %s", wf.Name)
954963
if err := compileWorkflowsForUpdate(ctx, []string{wf.Path}, opts.WorkflowsDir, opts.EngineOverride, opts.Verbose, opts.Approve); err != nil {
955964
updateLog.Printf("Compilation failed for workflow %s: %v", wf.Name, err)
965+
if restoreErr := os.WriteFile(wf.Path, originalContent, constants.FilePermPublic); restoreErr != nil {
966+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to restore original content for %s after compile failure: %v", wf.Name, restoreErr)))
967+
} else {
968+
updateLog.Printf("Restored original content for %s after compile failure", wf.Name)
969+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Reverted %s to its previous content because the updated version failed to compile", wf.Name)))
970+
}
956971
return fmt.Errorf("failed to compile updated workflow: %w", err)
957972
}
958973
} else {

0 commit comments

Comments
 (0)