From 51683d7d062a12ad42875a67ce8b84cf00fe18c7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:35:42 +0000 Subject: [PATCH 1/2] chore: remove dead functions Remove 5 unreachable functions identified by the deadcode static analyzer, along with their exclusive tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cli/add_command_test.go | 116 ---------------------------- pkg/cli/add_workflow_compilation.go | 24 ------ pkg/cli/compile_external_tools.go | 8 -- pkg/cli/file_tracker_test.go | 100 ------------------------ pkg/cli/update_command_test.go | 47 ----------- 5 files changed, 295 deletions(-) diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 928a9645978..e23106059e7 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -587,122 +587,6 @@ func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *t require.NoError(t, err) } -func TestCompileDispatchWorkflowDependencies_FallsBackToRawFrontmatter(t *testing.T) { - tmpDir := testutil.TempDir(t, "dispatch-workflow-fallback-*") - workflowsDir := setupMinimalGitRepo(t, tmpDir) - - mainPath := filepath.Join(workflowsDir, "dispatcher.md") - workerPath := filepath.Join(workflowsDir, "worker.md") - - require.NoError(t, os.WriteFile(mainPath, []byte(`--- -name: Dispatcher -on: - workflow_dispatch: -safe-outputs: - dispatch-workflow: - workflows: [worker] -imports: - - uses: shared/missing.md ---- - -# Dispatcher -`), 0o644)) - require.NoError(t, os.WriteFile(workerPath, []byte(`--- -name: Worker -on: - workflow_dispatch: ---- - -# Worker -`), 0o644)) - - compileDispatchWorkflowDependencies(context.Background(), mainPath, false, true, "", false, nil) - - lockPath := filepath.Join(workflowsDir, "worker.lock.yml") - _, err := os.Stat(lockPath) - require.NoError(t, err, "dispatch dependency should still be compiled when merged parse fails") - lockContent, err := os.ReadFile(lockPath) - require.NoError(t, err) - assert.Contains(t, string(lockContent), "name: \"Worker\"", "compiled dispatch dependency should preserve its workflow name") -} - -// TestCompileCallWorkflowDependencies_PropagatesError verifies that a worker compilation -// failure causes compileCallWorkflowDependencies to return an error rather than silently -// continuing. A bad worker .md that contains invalid content triggers this path. -func TestCompileCallWorkflowDependencies_PropagatesError(t *testing.T) { - tmpDir := testutil.TempDir(t, "call-workflow-error-*") - workflowsDir := setupMinimalGitRepo(t, tmpDir) - - mainPath := filepath.Join(workflowsDir, "orchestrator.md") - workerPath := filepath.Join(workflowsDir, "worker.md") - - require.NoError(t, os.WriteFile(mainPath, []byte(`--- -name: Orchestrator -on: - workflow_dispatch: -safe-outputs: - call-workflow: - - worker ---- - -# Orchestrator -`), 0o644)) - - // Write an intentionally broken worker file (no frontmatter — compile will fail). - require.NoError(t, os.WriteFile(workerPath, []byte(`not valid workflow content`), 0o644)) - - err := compileCallWorkflowDependencies(context.Background(), mainPath, false, true, "", false, nil) - require.Error(t, err, "worker compilation failure should propagate as an error") - require.ErrorContains(t, err, "worker", "error should mention the worker name") -} - -// TestCompileCallWorkflowDependencies_ForceRecompilesStale verifies that when force=true, -// a worker whose .lock.yml already exists is still recompiled. -func TestCompileCallWorkflowDependencies_ForceRecompilesStale(t *testing.T) { - tmpDir := testutil.TempDir(t, "call-workflow-force-*") - workflowsDir := setupMinimalGitRepo(t, tmpDir) - - mainPath := filepath.Join(workflowsDir, "orchestrator.md") - workerPath := filepath.Join(workflowsDir, "worker.md") - lockPath := filepath.Join(workflowsDir, "worker.lock.yml") - - require.NoError(t, os.WriteFile(mainPath, []byte(`--- -name: Orchestrator -on: - workflow_dispatch: -safe-outputs: - call-workflow: - - worker ---- - -# Orchestrator -`), 0o644)) - require.NoError(t, os.WriteFile(workerPath, []byte(`--- -name: Worker -on: - workflow_dispatch: ---- - -# Worker -`), 0o644)) - - // Write a stale (empty) lock file. - require.NoError(t, os.WriteFile(lockPath, []byte("# stale lock"), 0o644)) - - // Without force: stale lock is preserved. - err := compileCallWorkflowDependencies(context.Background(), mainPath, false, true, "", false, nil) - require.NoError(t, err) - content, _ := os.ReadFile(lockPath) - assert.Equal(t, "# stale lock", string(content), "without force, stale lock should not be recompiled") - - // With force: stale lock gets recompiled. - err = compileCallWorkflowDependencies(context.Background(), mainPath, false, true, "", true, nil) - require.NoError(t, err) - recompiled, _ := os.ReadFile(lockPath) - assert.NotEqual(t, "# stale lock", string(recompiled), "with force, stale lock should be recompiled") - assert.Contains(t, string(recompiled), "name: \"Worker\"", "recompiled lock should contain worker name") -} - func TestValidateWorkflowDestination_SkipsExistingWorkflowFromSameSource(t *testing.T) { t.Parallel() workflowsDir := t.TempDir() diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go index 910db268368..a0e58157b48 100644 --- a/pkg/cli/add_workflow_compilation.go +++ b/pkg/cli/add_workflow_compilation.go @@ -29,12 +29,6 @@ func compileWorkflowWithActionRef(ctx context.Context, filePath string, verbose return compileWorkflowWithRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, actionRef, false, false) } -// compileWorkflowWithRefresh compiles a workflow file with optional stop time refresh. -// This function handles the compilation process and ensures .gitattributes is updated. -func compileWorkflowWithRefresh(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, refreshStopTime bool, approve bool) error { - return compileWorkflowWithRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", refreshStopTime, approve) -} - func compileWorkflowWithRefreshAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, refreshStopTime bool, approve bool) error { addWorkflowCompilationLog.Printf("Compiling workflow: file=%s, refresh_stop_time=%v, engine=%s, approve=%v", filePath, refreshStopTime, engineOverride, approve) @@ -68,12 +62,6 @@ func compileWorkflowWithRefreshAndActionRef(ctx context.Context, filePath string return nil } -// compileWorkflowWithTracking compiles a workflow and tracks generated files. -// This is a convenience wrapper around compileWorkflowWithTrackingAndRefresh. -func compileWorkflowWithTracking(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, tracker *FileTracker) error { - return compileWorkflowWithTrackingAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", tracker) -} - func compileWorkflowWithTrackingAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, tracker *FileTracker) error { return compileWorkflowWithTrackingAndRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, actionRef, tracker, false) } @@ -149,14 +137,6 @@ type compileDepsOptions struct { tracker *FileTracker } -// compileDispatchWorkflowDependencies compiles any dispatch-workflow .md dependencies of -// workflowFile that are present locally but lack a corresponding .lock.yml. This must be -// called before compiling the main workflow, because the dispatch-workflow validator -// requires every referenced .md workflow to have an up-to-date .lock.yml. -func compileDispatchWorkflowDependencies(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride string, force bool, tracker *FileTracker) { - compileDispatchWorkflowDependenciesWithActionRef(ctx, workflowFile, verbose, quiet, engineOverride, "", force, tracker) -} - func compileDispatchWorkflowDependenciesWithActionRef(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride, actionRef string, force bool, tracker *FileTracker) { compileSafeOutputsWorkflowDependencies(ctx, workflowFile, "dispatch-workflow dependency", dispatchWorkflowNamesForCompilation, compileDepsOptions{ verbose: verbose, quiet: quiet, engineOverride: engineOverride, actionRef: actionRef, force: force, propagateErrors: false, tracker: tracker, @@ -172,10 +152,6 @@ func compileDispatchWorkflowDependenciesWithActionRef(ctx context.Context, workf // the dynamic tool-generation path maps every worker .md to a .lock.yml reference, so a // worker whose lock cannot be produced would leave the orchestrator referencing a file that // does not exist. -func compileCallWorkflowDependencies(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride string, force bool, tracker *FileTracker) error { - return compileCallWorkflowDependenciesWithActionRef(ctx, workflowFile, verbose, quiet, engineOverride, "", force, tracker) -} - func compileCallWorkflowDependenciesWithActionRef(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride, actionRef string, force bool, tracker *FileTracker) error { return compileSafeOutputsWorkflowDependencies(ctx, workflowFile, "call-workflow worker", callWorkflowNamesForCompilation, compileDepsOptions{ verbose: verbose, quiet: quiet, engineOverride: engineOverride, actionRef: actionRef, force: force, propagateErrors: true, tracker: tracker, diff --git a/pkg/cli/compile_external_tools.go b/pkg/cli/compile_external_tools.go index 540e5e4572c..96c4b4bd176 100644 --- a/pkg/cli/compile_external_tools.go +++ b/pkg/cli/compile_external_tools.go @@ -83,14 +83,6 @@ func RunYamllintOnFiles(lockFiles []string, verbose bool, strict bool) error { return runBatchLockFileTool("yamllint", lockFiles, verbose, strict, runYamllintOnFiles) } -// RunShellcheckOnLockFiles runs shellcheck on the run: step scripts extracted -// from the provided lock files. Shellcheck must be installed as a system binary; -// unlike other tools it does not use Docker. When shellcheck is not available -// the function returns nil (callers are responsible for warning the user). -func RunShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose bool, strict bool) error { - return RunShellcheckOnLockFilesAndResources(ctx, lockFiles, nil, verbose, strict) -} - // RunShellcheckOnLockFilesAndResources runs shellcheck on run steps extracted // from lock files and shell script resources defined in workflow frontmatter. func RunShellcheckOnLockFilesAndResources(ctx context.Context, lockFiles []string, resources []workflow.ShellScriptResource, verbose bool, strict bool) error { diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index 3494883ed1b..a0fbde5b956 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -3,11 +3,9 @@ package cli import ( - "context" "os" "os/exec" "path/filepath" - "slices" "strings" "testing" ) @@ -250,104 +248,6 @@ func TestFileTracker_RollbackAllFiles(t *testing.T) { } } -func TestCompileWorkflowWithTracking_SharedActions(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "shared-actions-test") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } - defer os.RemoveAll(tempDir) - - // Initialize git repository in temp directory - gitCmd := []string{"git", "init"} - if err := runCommandInDir(gitCmd, tempDir); err != nil { - t.Skipf("Skipping test - git not available or failed to init: %v", err) - } - - // Change to temp directory - oldWd, _ := os.Getwd() - defer func() { - _ = os.Chdir(oldWd) - }() - if err := os.Chdir(tempDir); err != nil { - t.Fatalf("Failed to change to temp directory: %v", err) - } - - // Test 1: Workflow WITH reaction should create shared action - workflowWithReaction := `--- -name: Test Workflow With Reaction -on: - push: - branches: [main] - reaction: heart ---- - -This is a test workflow. - -## Job: test - -This uses reaction. -` - - workflowFileWithReaction := filepath.Join(tempDir, "test-workflow-with-reaction.md") - if err := os.WriteFile(workflowFileWithReaction, []byte(workflowWithReaction), 0644); err != nil { - t.Fatalf("Failed to create workflow file: %v", err) - } - - // Create file tracker - tracker := NewFileTracker() - - // Compile the workflow with tracking - if err := compileWorkflowWithTracking(context.Background(), workflowFileWithReaction, false, false, "", tracker); err != nil { - t.Fatalf("Failed to compile workflow: %v", err) - } - - // Check that shared action files are tracked - allFiles := append(tracker.CreatedFiles, tracker.ModifiedFiles...) - - // Should track the lock file - lockFile := filepath.Join(tempDir, "test-workflow-with-reaction.lock.yml") - found := slices.Contains(allFiles, lockFile) - if !found { - t.Errorf("Lock file %s should be tracked", lockFile) - } - - // Note: The reaction feature now uses inline GitHub Scripts instead of separate action files - // so we don't expect a separate reaction action file to be created - - // Test 2: Workflow WITHOUT ai-reaction should NOT create shared action - workflowWithoutReaction := `--- -name: Test Workflow Without Reaction -on: push ---- - -This is a test workflow. - -## Job: test - -This does NOT use ai-reaction. -` - - workflowFileWithoutReaction := filepath.Join(tempDir, "test-workflow-without-reaction.md") - if err := os.WriteFile(workflowFileWithoutReaction, []byte(workflowWithoutReaction), 0644); err != nil { - t.Fatalf("Failed to create workflow file: %v", err) - } - - // Create new file tracker for second test - tracker2 := NewFileTracker() - - // Remove the existing reaction action to test it's not created again - // (Note: Since reaction is now inline, this removal step is no longer needed) - - // Compile the workflow with tracking - if err := compileWorkflowWithTracking(context.Background(), workflowFileWithoutReaction, false, false, "", tracker2); err != nil { - t.Fatalf("Failed to compile workflow: %v", err) - } - - // Note: Since reaction feature now uses inline GitHub Scripts instead of separate action files, - // we don't expect any reaction action files to be created or tracked -} - func TestFileTracker_StageAllFiles_NonGitRepo(t *testing.T) { tempDir, err := os.MkdirTemp("", "file-tracker-non-git") if err != nil { diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 5d82152839b..73da9d33ffc 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -978,53 +978,6 @@ Test content.`, } } -// TestCompileWorkflowWithRefresh tests that compileWorkflowWithRefresh properly passes refreshStopTime -func TestCompileWorkflowWithRefresh(t *testing.T) { - - // Create a temporary directory for test files - tmpDir := testutil.TempDir(t, "test-*") - - // Create a simple workflow file - workflowFile := filepath.Join(tmpDir, "test-workflow.md") - workflowContent := `--- -on: - workflow_dispatch: - stop-after: "+48h" -permissions: - contents: read -engine: copilot ---- - -# Test Workflow - -This is a test workflow. -` - err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) - if err != nil { - t.Fatalf("Failed to create test workflow file: %v", err) - } - - // Test with refreshStopTime=false (should preserve existing stop time if lock exists) - t.Run("compileWorkflowWithRefresh false", func(t *testing.T) { - err := compileWorkflowWithRefresh(context.Background(), workflowFile, false, false, "", false, false) - if err != nil { - t.Logf("Compilation failed (expected in test environment): %v", err) - // In a test environment without full setup, compilation may fail, - // but we're testing that the function exists and accepts the parameter - } - }) - - // Test with refreshStopTime=true (should regenerate stop time) - t.Run("compileWorkflowWithRefresh true", func(t *testing.T) { - err := compileWorkflowWithRefresh(context.Background(), workflowFile, false, false, "", true, false) - if err != nil { - t.Logf("Compilation failed (expected in test environment): %v", err) - // In a test environment without full setup, compilation may fail, - // but we're testing that the function exists and accepts the parameter - } - }) -} - // TestUpdateWorkflow_OverrideMode tests the default override mode behavior func TestUpdateWorkflow_DefaultMergeMode(t *testing.T) { // By default, merge mode is on. Local changes should be preserved via 3-way merge. From 62e9ba1bcf348cde0b7752b190be812189f7ad54 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 6 Sep 2026 15:40:23 +0000 Subject: [PATCH 2/2] Preserve compatibility and test coverage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_command_test.go | 116 ++++++++++++++++++++++++++++ pkg/cli/add_workflow_compilation.go | 4 +- pkg/cli/compile_external_tools.go | 8 ++ pkg/cli/file_tracker_test.go | 100 ++++++++++++++++++++++++ pkg/cli/update_command_test.go | 47 +++++++++++ 5 files changed, 273 insertions(+), 2 deletions(-) diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index e23106059e7..c0354d05213 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -587,6 +587,122 @@ func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *t require.NoError(t, err) } +func TestCompileDispatchWorkflowDependencies_FallsBackToRawFrontmatter(t *testing.T) { + tmpDir := testutil.TempDir(t, "dispatch-workflow-fallback-*") + workflowsDir := setupMinimalGitRepo(t, tmpDir) + + mainPath := filepath.Join(workflowsDir, "dispatcher.md") + workerPath := filepath.Join(workflowsDir, "worker.md") + + require.NoError(t, os.WriteFile(mainPath, []byte(`--- +name: Dispatcher +on: + workflow_dispatch: +safe-outputs: + dispatch-workflow: + workflows: [worker] +imports: + - uses: shared/missing.md +--- + +# Dispatcher +`), 0o644)) + require.NoError(t, os.WriteFile(workerPath, []byte(`--- +name: Worker +on: + workflow_dispatch: +--- + +# Worker +`), 0o644)) + + compileDispatchWorkflowDependenciesWithActionRef(context.Background(), mainPath, false, true, "", "", false, nil) + + lockPath := filepath.Join(workflowsDir, "worker.lock.yml") + _, err := os.Stat(lockPath) + require.NoError(t, err, "dispatch dependency should still be compiled when merged parse fails") + lockContent, err := os.ReadFile(lockPath) + require.NoError(t, err) + assert.Contains(t, string(lockContent), "name: \"Worker\"", "compiled dispatch dependency should preserve its workflow name") +} + +// TestCompileCallWorkflowDependencies_PropagatesError verifies that a worker compilation +// failure causes compileCallWorkflowDependencies to return an error rather than silently +// continuing. A bad worker .md that contains invalid content triggers this path. +func TestCompileCallWorkflowDependencies_PropagatesError(t *testing.T) { + tmpDir := testutil.TempDir(t, "call-workflow-error-*") + workflowsDir := setupMinimalGitRepo(t, tmpDir) + + mainPath := filepath.Join(workflowsDir, "orchestrator.md") + workerPath := filepath.Join(workflowsDir, "worker.md") + + require.NoError(t, os.WriteFile(mainPath, []byte(`--- +name: Orchestrator +on: + workflow_dispatch: +safe-outputs: + call-workflow: + - worker +--- + +# Orchestrator +`), 0o644)) + + // Write an intentionally broken worker file (no frontmatter — compile will fail). + require.NoError(t, os.WriteFile(workerPath, []byte(`not valid workflow content`), 0o644)) + + err := compileCallWorkflowDependenciesWithActionRef(context.Background(), mainPath, false, true, "", "", false, nil) + require.Error(t, err, "worker compilation failure should propagate as an error") + require.ErrorContains(t, err, "worker", "error should mention the worker name") +} + +// TestCompileCallWorkflowDependencies_ForceRecompilesStale verifies that when force=true, +// a worker whose .lock.yml already exists is still recompiled. +func TestCompileCallWorkflowDependencies_ForceRecompilesStale(t *testing.T) { + tmpDir := testutil.TempDir(t, "call-workflow-force-*") + workflowsDir := setupMinimalGitRepo(t, tmpDir) + + mainPath := filepath.Join(workflowsDir, "orchestrator.md") + workerPath := filepath.Join(workflowsDir, "worker.md") + lockPath := filepath.Join(workflowsDir, "worker.lock.yml") + + require.NoError(t, os.WriteFile(mainPath, []byte(`--- +name: Orchestrator +on: + workflow_dispatch: +safe-outputs: + call-workflow: + - worker +--- + +# Orchestrator +`), 0o644)) + require.NoError(t, os.WriteFile(workerPath, []byte(`--- +name: Worker +on: + workflow_dispatch: +--- + +# Worker +`), 0o644)) + + // Write a stale (empty) lock file. + require.NoError(t, os.WriteFile(lockPath, []byte("# stale lock"), 0o644)) + + // Without force: stale lock is preserved. + err := compileCallWorkflowDependenciesWithActionRef(context.Background(), mainPath, false, true, "", "", false, nil) + require.NoError(t, err) + content, _ := os.ReadFile(lockPath) + assert.Equal(t, "# stale lock", string(content), "without force, stale lock should not be recompiled") + + // With force: stale lock gets recompiled. + err = compileCallWorkflowDependenciesWithActionRef(context.Background(), mainPath, false, true, "", "", true, nil) + require.NoError(t, err) + recompiled, _ := os.ReadFile(lockPath) + assert.NotEqual(t, "# stale lock", string(recompiled), "with force, stale lock should be recompiled") + assert.Contains(t, string(recompiled), "name: \"Worker\"", "recompiled lock should contain worker name") +} + func TestValidateWorkflowDestination_SkipsExistingWorkflowFromSameSource(t *testing.T) { t.Parallel() workflowsDir := t.TempDir() diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go index a0e58157b48..c8cffa418c8 100644 --- a/pkg/cli/add_workflow_compilation.go +++ b/pkg/cli/add_workflow_compilation.go @@ -20,7 +20,7 @@ import ( var addWorkflowCompilationLog = logger.New("cli:add_workflow_compilation") // compileWorkflow compiles a workflow file without refreshing stop time. -// This is a convenience wrapper around compileWorkflowWithRefresh. +// This is a convenience wrapper around compileWorkflowWithActionRef. func compileWorkflow(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string) error { return compileWorkflowWithActionRef(ctx, filePath, verbose, quiet, engineOverride, "") } @@ -143,7 +143,7 @@ func compileDispatchWorkflowDependenciesWithActionRef(ctx context.Context, workf }) } -// compileCallWorkflowDependencies compiles any call-workflow .md worker dependencies of +// compileCallWorkflowDependenciesWithActionRef compiles any call-workflow .md worker dependencies of // workflowFile that are present locally but lack a corresponding .lock.yml. This must be // called before compiling the main workflow, because the call-workflow validator requires // every referenced .md worker to have an up-to-date .lock.yml. diff --git a/pkg/cli/compile_external_tools.go b/pkg/cli/compile_external_tools.go index 96c4b4bd176..540e5e4572c 100644 --- a/pkg/cli/compile_external_tools.go +++ b/pkg/cli/compile_external_tools.go @@ -83,6 +83,14 @@ func RunYamllintOnFiles(lockFiles []string, verbose bool, strict bool) error { return runBatchLockFileTool("yamllint", lockFiles, verbose, strict, runYamllintOnFiles) } +// RunShellcheckOnLockFiles runs shellcheck on the run: step scripts extracted +// from the provided lock files. Shellcheck must be installed as a system binary; +// unlike other tools it does not use Docker. When shellcheck is not available +// the function returns nil (callers are responsible for warning the user). +func RunShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose bool, strict bool) error { + return RunShellcheckOnLockFilesAndResources(ctx, lockFiles, nil, verbose, strict) +} + // RunShellcheckOnLockFilesAndResources runs shellcheck on run steps extracted // from lock files and shell script resources defined in workflow frontmatter. func RunShellcheckOnLockFilesAndResources(ctx context.Context, lockFiles []string, resources []workflow.ShellScriptResource, verbose bool, strict bool) error { diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index a0fbde5b956..a7b0966f62e 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -3,9 +3,11 @@ package cli import ( + "context" "os" "os/exec" "path/filepath" + "slices" "strings" "testing" ) @@ -248,6 +250,104 @@ func TestFileTracker_RollbackAllFiles(t *testing.T) { } } +func TestCompileWorkflowWithTracking_SharedActions(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "shared-actions-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Initialize git repository in temp directory + gitCmd := []string{"git", "init"} + if err := runCommandInDir(gitCmd, tempDir); err != nil { + t.Skipf("Skipping test - git not available or failed to init: %v", err) + } + + // Change to temp directory + oldWd, _ := os.Getwd() + defer func() { + _ = os.Chdir(oldWd) + }() + if err := os.Chdir(tempDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Test 1: Workflow WITH reaction should create shared action + workflowWithReaction := `--- +name: Test Workflow With Reaction +on: + push: + branches: [main] + reaction: heart +--- + +This is a test workflow. + +## Job: test + +This uses reaction. +` + + workflowFileWithReaction := filepath.Join(tempDir, "test-workflow-with-reaction.md") + if err := os.WriteFile(workflowFileWithReaction, []byte(workflowWithReaction), 0644); err != nil { + t.Fatalf("Failed to create workflow file: %v", err) + } + + // Create file tracker + tracker := NewFileTracker() + + // Compile the workflow with tracking + if err := compileWorkflowWithTrackingAndActionRef(context.Background(), workflowFileWithReaction, false, false, "", "", tracker); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Check that shared action files are tracked + allFiles := append(tracker.CreatedFiles, tracker.ModifiedFiles...) + + // Should track the lock file + lockFile := filepath.Join(tempDir, "test-workflow-with-reaction.lock.yml") + found := slices.Contains(allFiles, lockFile) + if !found { + t.Errorf("Lock file %s should be tracked", lockFile) + } + + // Note: The reaction feature now uses inline GitHub Scripts instead of separate action files + // so we don't expect a separate reaction action file to be created + + // Test 2: Workflow WITHOUT ai-reaction should NOT create shared action + workflowWithoutReaction := `--- +name: Test Workflow Without Reaction +on: push +--- + +This is a test workflow. + +## Job: test + +This does NOT use ai-reaction. +` + + workflowFileWithoutReaction := filepath.Join(tempDir, "test-workflow-without-reaction.md") + if err := os.WriteFile(workflowFileWithoutReaction, []byte(workflowWithoutReaction), 0644); err != nil { + t.Fatalf("Failed to create workflow file: %v", err) + } + + // Create new file tracker for second test + tracker2 := NewFileTracker() + + // Remove the existing reaction action to test it's not created again + // (Note: Since reaction is now inline, this removal step is no longer needed) + + // Compile the workflow with tracking + if err := compileWorkflowWithTrackingAndActionRef(context.Background(), workflowFileWithoutReaction, false, false, "", "", tracker2); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Note: Since reaction feature now uses inline GitHub Scripts instead of separate action files, + // we don't expect any reaction action files to be created or tracked +} + func TestFileTracker_StageAllFiles_NonGitRepo(t *testing.T) { tempDir, err := os.MkdirTemp("", "file-tracker-non-git") if err != nil { diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 73da9d33ffc..f1f14bb376a 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -978,6 +978,53 @@ Test content.`, } } +// TestCompileWorkflowWithRefresh tests that compileWorkflowWithRefresh properly passes refreshStopTime +func TestCompileWorkflowWithRefresh(t *testing.T) { + + // Create a temporary directory for test files + tmpDir := testutil.TempDir(t, "test-*") + + // Create a simple workflow file + workflowFile := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- +on: + workflow_dispatch: + stop-after: "+48h" +permissions: + contents: read +engine: copilot +--- + +# Test Workflow + +This is a test workflow. +` + err := os.WriteFile(workflowFile, []byte(workflowContent), 0644) + if err != nil { + t.Fatalf("Failed to create test workflow file: %v", err) + } + + // Test with refreshStopTime=false (should preserve existing stop time if lock exists) + t.Run("compileWorkflowWithRefresh false", func(t *testing.T) { + err := compileWorkflowWithRefreshAndActionRef(context.Background(), workflowFile, false, false, "", "", false, false) + if err != nil { + t.Logf("Compilation failed (expected in test environment): %v", err) + // In a test environment without full setup, compilation may fail, + // but we're testing that the function exists and accepts the parameter + } + }) + + // Test with refreshStopTime=true (should regenerate stop time) + t.Run("compileWorkflowWithRefresh true", func(t *testing.T) { + err := compileWorkflowWithRefreshAndActionRef(context.Background(), workflowFile, false, false, "", "", true, false) + if err != nil { + t.Logf("Compilation failed (expected in test environment): %v", err) + // In a test environment without full setup, compilation may fail, + // but we're testing that the function exists and accepts the parameter + } + }) +} + // TestUpdateWorkflow_OverrideMode tests the default override mode behavior func TestUpdateWorkflow_DefaultMergeMode(t *testing.T) { // By default, merge mode is on. Local changes should be preserved via 3-way merge.