From 32271f0be0616057efa4d52fe70645c6a5fa33dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 Aug 2025 04:44:11 +0000 Subject: [PATCH 1/2] Initial plan From 5d2dc7f1d47d709f2f71408eb4070ff30e39f6a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 Aug 2025 05:09:58 +0000 Subject: [PATCH 2/2] Implement lazy compute-text step insertion Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/actions/compute-text/action.yml | 67 -------- .github/workflows/test-codex.lock.yml | 5 - .github/workflows/weekly-research.lock.yml | 5 - pkg/workflow/compiler.go | 60 ++++--- pkg/workflow/compute_text_lazy_test.go | 177 +++++++++++++++++++++ 5 files changed, 219 insertions(+), 95 deletions(-) delete mode 100644 .github/actions/compute-text/action.yml create mode 100644 pkg/workflow/compute_text_lazy_test.go diff --git a/.github/actions/compute-text/action.yml b/.github/actions/compute-text/action.yml deleted file mode 100644 index 7c32af2634..0000000000 --- a/.github/actions/compute-text/action.yml +++ /dev/null @@ -1,67 +0,0 @@ -name: "Compute current body text" -description: "Computes the current body text based on the GitHub event context" -outputs: - text: - description: "The computed current body text based on event type" -runs: - using: "composite" - steps: - - name: Compute current body text - id: compute-text - uses: actions/github-script@v7 - with: - script: | - let text = ''; - - // Determine current body text based on event context - switch (context.eventName) { - case 'issues': - // For issues: title + body - if (context.payload.issue) { - const title = context.payload.issue.title || ''; - const body = context.payload.issue.body || ''; - text = `${title}\n\n${body}`; - } - break; - - case 'pull_request': - // For pull requests: title + body - if (context.payload.pull_request) { - const title = context.payload.pull_request.title || ''; - const body = context.payload.pull_request.body || ''; - text = `${title}\n\n${body}`; - } - break; - - case 'issue_comment': - // For issue comments: comment body - if (context.payload.comment) { - text = context.payload.comment.body || ''; - } - break; - - case 'pull_request_review_comment': - // For PR review comments: comment body - if (context.payload.comment) { - text = context.payload.comment.body || ''; - } - break; - - case 'pull_request_review': - // For PR reviews: review body - if (context.payload.review) { - text = context.payload.review.body || ''; - } - break; - - default: - // Default: empty text - text = ''; - break; - } - - // display in logs - console.log(`text: ${text}`); - - // Set the text as output - core.setOutput('text', text); \ No newline at end of file diff --git a/.github/workflows/test-codex.lock.yml b/.github/workflows/test-codex.lock.yml index 494d003c11..3f414f8c65 100644 --- a/.github/workflows/test-codex.lock.yml +++ b/.github/workflows/test-codex.lock.yml @@ -21,16 +21,11 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - outputs: - text: ${{ steps.compute-text.outputs.text }} steps: - uses: actions/checkout@v4 with: sparse-checkout: .github fetch-depth: 1 - - name: Compute current body text - id: compute-text - uses: ./.github/actions/compute-text add-reaction: needs: task diff --git a/.github/workflows/weekly-research.lock.yml b/.github/workflows/weekly-research.lock.yml index 9ef1c892ae..39a1a3f554 100644 --- a/.github/workflows/weekly-research.lock.yml +++ b/.github/workflows/weekly-research.lock.yml @@ -20,16 +20,11 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - outputs: - text: ${{ steps.compute-text.outputs.text }} steps: - uses: actions/checkout@v4 with: sparse-checkout: .github fetch-depth: 1 - - name: Compute current body text - id: compute-text - uses: ./.github/actions/compute-text add-reaction: needs: task diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 6b853967ce..3e848f6287 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -130,6 +130,7 @@ type WorkflowData struct { AIReaction string // AI reaction type like "eyes", "heart", etc. Jobs map[string]any // custom job configurations with dependencies Cache string // cache configuration + NeedsTextOutput bool // whether the workflow uses ${{ needs.task.outputs.text }} } // CompileWorkflow converts a markdown workflow to GitHub Actions YAML @@ -192,18 +193,20 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { return errors.New(formattedErr) } - // Write shared compute-text action (always generated for task job) - if err := c.writeComputeTextAction(markdownPath); err != nil { - formattedErr := console.FormatError(console.CompilerError{ - Position: console.ErrorPosition{ - File: markdownPath, - Line: 1, - Column: 1, - }, - Type: "error", - Message: fmt.Sprintf("failed to write compute-text action: %v", err), - }) - return errors.New(formattedErr) + // Write shared compute-text action (only if needed for task job) + if workflowData.NeedsTextOutput { + if err := c.writeComputeTextAction(markdownPath); err != nil { + formattedErr := console.FormatError(console.CompilerError{ + Position: console.ErrorPosition{ + File: markdownPath, + Line: 1, + Column: 1, + }, + Type: "error", + Message: fmt.Sprintf("failed to write compute-text action: %v", err), + }) + return errors.New(formattedErr) + } } // Write shared check-team-member action (only for alias workflows) @@ -516,6 +519,9 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Extracted workflow name: '%s'", workflowName))) } + // Check if the markdown content uses the text output + needsTextOutput := c.detectTextOutputUsage(markdownContent) + // Build workflow data workflowData := &WorkflowData{ Name: workflowName, @@ -523,6 +529,7 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) MarkdownContent: markdownContent, AI: engineSetting, EngineConfig: engineConfig, + NeedsTextOutput: needsTextOutput, } // Extract YAML sections from frontmatter - use direct frontmatter map extraction @@ -1140,6 +1147,20 @@ func (c *Compiler) applyDefaultGitHubMCPTools(tools map[string]any) map[string]a return tools } +// detectTextOutputUsage checks if the markdown content uses ${{ needs.task.outputs.text }} +func (c *Compiler) detectTextOutputUsage(markdownContent string) bool { + // Check for the specific GitHub Actions expression + hasUsage := strings.Contains(markdownContent, "${{ needs.task.outputs.text }}") + if c.verbose { + if hasUsage { + fmt.Println(console.FormatInfoMessage("Detected usage of task.outputs.text - compute-text step will be included")) + } else { + fmt.Println(console.FormatInfoMessage("No usage of task.outputs.text found - compute-text step will be skipped")) + } + } + return hasUsage +} + // computeAllowedTools computes the comma-separated list of allowed tools for Claude func (c *Compiler) computeAllowedTools(tools map[string]any) string { var allowedTools []string @@ -1413,14 +1434,17 @@ func (c *Compiler) buildTaskJob(data *WorkflowData) (*Job, error) { steps = append(steps, " exit 1\n") } - // Use shared compute-text action - 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") + // Use shared compute-text action only if needed + if data.NeedsTextOutput { + 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{ - "text": "${{ steps.compute-text.outputs.text }}", + outputs := map[string]string{} + if data.NeedsTextOutput { + outputs["text"] = "${{ steps.compute-text.outputs.text }}" } job := &Job{ diff --git a/pkg/workflow/compute_text_lazy_test.go b/pkg/workflow/compute_text_lazy_test.go new file mode 100644 index 0000000000..2fdb9f5f06 --- /dev/null +++ b/pkg/workflow/compute_text_lazy_test.go @@ -0,0 +1,177 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestComputeTextLazyInsertion(t *testing.T) { + // Create a temporary directory for the test + tempDir, err := os.MkdirTemp("", "compute-text-lazy-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create a .git directory to simulate a git repository + gitDir := filepath.Join(tempDir, ".git") + if err := os.MkdirAll(gitDir, 0755); err != nil { + t.Fatalf("Failed to create .git dir: %v", err) + } + + // Test case 1: Workflow that uses task.outputs.text + workflowWithText := `--- +on: + issues: + types: [opened] +permissions: + issues: write +tools: + github: + allowed: [add_issue_comment] +--- + +# Test Workflow With Text Output + +This workflow uses the text output: "${{ needs.task.outputs.text }}" + +Please analyze this issue and provide a helpful response.` + + workflowWithTextPath := filepath.Join(tempDir, "with-text.md") + if err := os.WriteFile(workflowWithTextPath, []byte(workflowWithText), 0644); err != nil { + t.Fatalf("Failed to write workflow with text: %v", err) + } + + // Test case 2: Workflow that does NOT use task.outputs.text + workflowWithoutText := `--- +on: + schedule: + - cron: "0 9 * * 1" +permissions: + issues: write +tools: + github: + allowed: [create_issue] +--- + +# Test Workflow Without Text Output + +This workflow does NOT use the text output. + +Create a report based on repository analysis.` + + workflowWithoutTextPath := filepath.Join(tempDir, "without-text.md") + if err := os.WriteFile(workflowWithoutTextPath, []byte(workflowWithoutText), 0644); err != nil { + t.Fatalf("Failed to write workflow without text: %v", err) + } + + compiler := NewCompiler(false, "", "test-version") + + // Test workflow WITH text usage + t.Run("workflow_with_text_usage", func(t *testing.T) { + err := compiler.CompileWorkflow(workflowWithTextPath) + if err != nil { + t.Fatalf("Failed to compile workflow with text: %v", err) + } + + // Check that compute-text action was created + actionPath := filepath.Join(tempDir, ".github", "actions", "compute-text", "action.yml") + if _, err := os.Stat(actionPath); os.IsNotExist(err) { + t.Error("Expected compute-text action to be created for workflow that uses text output") + } + + // Check that the compiled YAML contains compute-text step + lockPath := strings.TrimSuffix(workflowWithTextPath, ".md") + ".lock.yml" + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read compiled workflow: %v", err) + } + + lockStr := string(lockContent) + if !strings.Contains(lockStr, "compute-text") { + t.Error("Expected compiled workflow to contain compute-text step") + } + if !strings.Contains(lockStr, "text: ${{ steps.compute-text.outputs.text }}") { + t.Error("Expected compiled workflow to contain text output") + } + }) + + // Remove compute-text action for next test + os.RemoveAll(filepath.Join(tempDir, ".github")) + + // Test workflow WITHOUT text usage + t.Run("workflow_without_text_usage", func(t *testing.T) { + err := compiler.CompileWorkflow(workflowWithoutTextPath) + if err != nil { + t.Fatalf("Failed to compile workflow without text: %v", err) + } + + // Check that compute-text action was NOT created + actionPath := filepath.Join(tempDir, ".github", "actions", "compute-text", "action.yml") + if _, err := os.Stat(actionPath); !os.IsNotExist(err) { + t.Error("Expected compute-text action NOT to be created for workflow that doesn't use text output") + } + + // Check that the compiled YAML does NOT contain compute-text step + lockPath := strings.TrimSuffix(workflowWithoutTextPath, ".md") + ".lock.yml" + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read compiled workflow: %v", err) + } + + lockStr := string(lockContent) + if strings.Contains(lockStr, "compute-text") { + t.Error("Expected compiled workflow NOT to contain compute-text step") + } + if strings.Contains(lockStr, "text: ${{ steps.compute-text.outputs.text }}") { + t.Error("Expected compiled workflow NOT to contain text output") + } + }) +} + +func TestDetectTextOutputUsage(t *testing.T) { + compiler := NewCompiler(false, "", "test-version") + + tests := []struct { + name string + content string + expectedUsage bool + }{ + { + name: "with_text_usage", + content: "Analyze this: \"${{ needs.task.outputs.text }}\"", + expectedUsage: true, + }, + { + name: "without_text_usage", + content: "Create a report based on repository analysis.", + expectedUsage: false, + }, + { + name: "with_other_github_expressions", + content: "Repository: ${{ github.repository }} but no text output", + expectedUsage: false, + }, + { + name: "with_partial_match", + content: "Something about task.outputs but not the full expression", + expectedUsage: false, + }, + { + name: "with_multiple_usages", + content: "First: \"${{ needs.task.outputs.text }}\" and second: \"${{ needs.task.outputs.text }}\"", + expectedUsage: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := compiler.detectTextOutputUsage(tt.content) + if result != tt.expectedUsage { + t.Errorf("detectTextOutputUsage() = %v, expected %v", result, tt.expectedUsage) + } + }) + } +}