From d78f9e3b0f9fac4bc20511d2cb33fad2048f513e Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 21 Aug 2025 16:40:51 +0100 Subject: [PATCH 1/2] only emit task if needed --- .github/workflows/issue-triage.lock.yml | 11 ----- .github/workflows/test-claude.lock.yml | 11 ----- .github/workflows/test-codex.lock.yml | 11 ----- .github/workflows/weekly-research.lock.yml | 11 ----- pkg/workflow/compiler.go | 55 +++++++++++++++------- 5 files changed, 37 insertions(+), 62 deletions(-) diff --git a/.github/workflows/issue-triage.lock.yml b/.github/workflows/issue-triage.lock.yml index 55eb9601602..8e26dd61736 100644 --- a/.github/workflows/issue-triage.lock.yml +++ b/.github/workflows/issue-triage.lock.yml @@ -18,18 +18,7 @@ concurrency: run-name: "Agentic Triage" jobs: - task: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: actions/checkout@v4 - with: - sparse-checkout: .github - fetch-depth: 1 - agentic-triage: - needs: task runs-on: ubuntu-latest permissions: actions: read diff --git a/.github/workflows/test-claude.lock.yml b/.github/workflows/test-claude.lock.yml index 9a32a38a343..147fbece113 100644 --- a/.github/workflows/test-claude.lock.yml +++ b/.github/workflows/test-claude.lock.yml @@ -21,18 +21,7 @@ concurrency: run-name: "Test Claude" jobs: - task: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: actions/checkout@v4 - with: - sparse-checkout: .github - fetch-depth: 1 - test-claude: - needs: task runs-on: ubuntu-latest permissions: actions: read diff --git a/.github/workflows/test-codex.lock.yml b/.github/workflows/test-codex.lock.yml index fc28ad87904..858e597ced5 100644 --- a/.github/workflows/test-codex.lock.yml +++ b/.github/workflows/test-codex.lock.yml @@ -17,18 +17,7 @@ concurrency: run-name: "Test Codex" jobs: - task: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: actions/checkout@v4 - with: - sparse-checkout: .github - fetch-depth: 1 - test-codex: - needs: task runs-on: ubuntu-latest permissions: actions: read diff --git a/.github/workflows/weekly-research.lock.yml b/.github/workflows/weekly-research.lock.yml index c50e121c186..28d588307d9 100644 --- a/.github/workflows/weekly-research.lock.yml +++ b/.github/workflows/weekly-research.lock.yml @@ -16,18 +16,7 @@ concurrency: run-name: "Weekly Research" jobs: - task: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: actions/checkout@v4 - with: - sparse-checkout: .github - fetch-depth: 1 - weekly-research: - needs: task runs-on: ubuntu-latest permissions: actions: read diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 135032bdf1d..52a4126d9f9 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1454,23 +1454,35 @@ func (c *Compiler) generateYAML(data *WorkflowData) (string, error) { return yaml.String(), nil } +// isTaskJobNeeded determines if the task job is required +func (c *Compiler) isTaskJobNeeded(data *WorkflowData) bool { + // Task job is needed if: + // 1. Alias is configured (for team member checking) + // 2. Text output is needed (for compute-text action) + return data.Alias != "" || data.NeedsTextOutput +} + // buildJobs creates all jobs for the workflow and adds them to the job manager func (c *Compiler) buildJobs(data *WorkflowData) error { // Generate job name from workflow name jobName := c.generateJobName(data.Name) - // Build task job first (preamble job that handles runtime conditions) - taskJob, err := c.buildTaskJob(data) - if err != nil { - return fmt.Errorf("failed to build task job: %w", err) - } - if err := c.jobManager.AddJob(taskJob); err != nil { - return fmt.Errorf("failed to add task job: %w", err) + // Build task job only if actually needed (preamble job that handles runtime conditions) + var taskJobCreated bool + if c.isTaskJobNeeded(data) { + taskJob, err := c.buildTaskJob(data) + if err != nil { + return fmt.Errorf("failed to build task job: %w", err) + } + if err := c.jobManager.AddJob(taskJob); err != nil { + return fmt.Errorf("failed to add task job: %w", err) + } + taskJobCreated = true } // Build add_reaction job only if ai-reaction is configured if data.AIReaction != "" { - addReactionJob, err := c.buildAddReactionJob(data) + addReactionJob, err := c.buildAddReactionJob(data, taskJobCreated) if err != nil { return fmt.Errorf("failed to build add_reaction job: %w", err) } @@ -1480,7 +1492,7 @@ func (c *Compiler) buildJobs(data *WorkflowData) error { } // Build main workflow job - mainJob, err := c.buildMainJob(data, jobName) + mainJob, err := c.buildMainJob(data, jobName, taskJobCreated) if err != nil { return fmt.Errorf("failed to build main job: %w", err) } @@ -1531,6 +1543,7 @@ func (c *Compiler) buildJobs(data *WorkflowData) error { // buildTaskJob creates the preamble task job that acts as a barrier for runtime conditions func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { + outputs := map[string]string{} var steps []string // Add shallow checkout step to access shared actions @@ -1571,11 +1584,7 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { steps = append(steps, " - name: Compute current body text\n") steps = append(steps, " id: compute-text\n") steps = append(steps, " uses: ./.github/actions/compute-text\n") - } - - // Set up outputs - outputs := map[string]string{} - if data.NeedsTextOutput { + // Set up outputs outputs["text"] = "${{ steps.compute-text.outputs.text }}" } @@ -1592,7 +1601,7 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { } // buildAddReactionJob creates the add_reaction job -func (c *Compiler) buildAddReactionJob(data *WorkflowData) (*Job, error) { +func (c *Compiler) buildAddReactionJob(data *WorkflowData, taskJobCreated bool) (*Job, error) { reactionCondition := buildReactionCondition() var steps []string @@ -1611,6 +1620,11 @@ func (c *Compiler) buildAddReactionJob(data *WorkflowData) (*Job, error) { "reaction_id": "${{ steps.react.outputs.reaction-id }}", } + var depends []string + if taskJobCreated { + depends = []string{"task"} // Depend on the task job only if it exists + } + job := &Job{ Name: "add_reaction", If: fmt.Sprintf("if: %s", reactionCondition.Render()), @@ -1618,7 +1632,7 @@ func (c *Compiler) buildAddReactionJob(data *WorkflowData) (*Job, error) { Permissions: "permissions:\n contents: write # Read .github\n issues: write\n pull-requests: write", Steps: steps, Outputs: outputs, - Depends: []string{"task"}, // Depend on the task job + Depends: depends, } return job, nil @@ -1812,7 +1826,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa } // buildMainJob creates the main workflow job -func (c *Compiler) buildMainJob(data *WorkflowData, jobName string) (*Job, error) { +func (c *Compiler) buildMainJob(data *WorkflowData, jobName string, taskJobCreated bool) (*Job, error) { var steps []string // Build step content using the generateMainJobSteps helper method @@ -1826,13 +1840,18 @@ func (c *Compiler) buildMainJob(data *WorkflowData, jobName string) (*Job, error steps = append(steps, stepsContent) } + var depends []string + if taskJobCreated { + depends = []string{"task"} // Depend on the task job only if it exists + } + job := &Job{ Name: jobName, If: "", // Remove the If condition since task job handles alias checks RunsOn: c.indentYAMLLines(data.RunsOn, " "), Permissions: c.indentYAMLLines(data.Permissions, " "), Steps: steps, - Depends: []string{"task"}, // Depend on the task job + Depends: depends, Outputs: map[string]string{ "output": "${{ steps.collect_output.outputs.output }}", }, From 5d7879d8887c2bbfb043139e507b20981ae23588 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 21 Aug 2025 17:20:37 +0100 Subject: [PATCH 2/2] fix tests --- pkg/workflow/compiler.go | 3 ++- pkg/workflow/compiler_test.go | 8 ++++---- pkg/workflow/is_task_job_needed_test.go | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 pkg/workflow/is_task_job_needed_test.go diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 52a4126d9f9..bc0cddcefa5 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1459,7 +1459,8 @@ func (c *Compiler) isTaskJobNeeded(data *WorkflowData) bool { // Task job is needed if: // 1. Alias is configured (for team member checking) // 2. Text output is needed (for compute-text action) - return data.Alias != "" || data.NeedsTextOutput + // 3. If condition is specified (to handle runtime conditions) + return data.Alias != "" || data.NeedsTextOutput || data.If != "" } // buildJobs creates all jobs for the workflow and adds them to the job manager diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index b1cd6618237..b8e84e7b587 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -3353,8 +3353,8 @@ Test workflow with ai-reaction. // Verify three jobs are created (task, add_reaction, main) jobCount := strings.Count(yamlContent, "runs-on: ubuntu-latest") - if jobCount != 3 { - t.Errorf("Expected 3 jobs (task, add_reaction, main), found %d", jobCount) + if jobCount != 2 { + t.Errorf("Expected 2 jobs (add_reaction, main), found %d", jobCount) } } @@ -3425,8 +3425,8 @@ Test workflow without explicit ai-reaction (should not create reaction action). // Verify only two jobs are created (task and main, no add_reaction) jobCount := strings.Count(yamlContent, "runs-on: ubuntu-latest") - if jobCount != 2 { - t.Errorf("Expected 2 jobs (task, main), found %d", jobCount) + if jobCount != 1 { + t.Errorf("Expected 1 jobs (main), found %d", jobCount) } } diff --git a/pkg/workflow/is_task_job_needed_test.go b/pkg/workflow/is_task_job_needed_test.go new file mode 100644 index 00000000000..ef961883c86 --- /dev/null +++ b/pkg/workflow/is_task_job_needed_test.go @@ -0,0 +1,21 @@ +package workflow + +import "testing" + +func TestIsTaskJobNeeded(t *testing.T) { + compiler := NewCompiler(false, "", "test") + + t.Run("no_conditions", func(t *testing.T) { + data := &WorkflowData{} + if compiler.isTaskJobNeeded(data) { + t.Errorf("Expected isTaskJobNeeded to be false when no alias, no needsTextOutput, and no If condition") + } + }) + + t.Run("if_condition_present", func(t *testing.T) { + data := &WorkflowData{If: "if: github.ref == 'refs/heads/main'"} + if !compiler.isTaskJobNeeded(data) { + t.Errorf("Expected isTaskJobNeeded to be true when If condition is specified") + } + }) +}