diff --git a/pkg/workflow/safe_outputs_integration_test.go b/pkg/workflow/safe_outputs_integration_test.go index 53ee5d33d3..82d8dc7f7c 100644 --- a/pkg/workflow/safe_outputs_integration_test.go +++ b/pkg/workflow/safe_outputs_integration_test.go @@ -184,23 +184,6 @@ func TestSafeOutputJobsIntegration(t *testing.T) { return c.buildUploadAssetsJob(data, mainJobName, false) }, }, - { - name: "update_project", - safeOutputType: "update-project", - configBuilder: func() *SafeOutputsConfig { - return &SafeOutputsConfig{ - UpdateProjects: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 5, - }, - }, - } - }, - requiredEnvVar: "GH_AW_WORKFLOW_ID", - jobBuilder: func(c *Compiler, data *WorkflowData, mainJobName string) (*Job, error) { - return c.buildUpdateProjectJob(data, mainJobName) - }, - }, } // Known issue: Individual job builders are missing GH_AW_WORKFLOW_ID @@ -215,7 +198,6 @@ func TestSafeOutputJobsIntegration(t *testing.T) { "create_code_scanning_alert": true, "create_agent_session": true, "upload_assets": true, - "update_project": true, } for _, tt := range tests { diff --git a/pkg/workflow/update_project_job.go b/pkg/workflow/update_project_job.go deleted file mode 100644 index 284dcb72f9..0000000000 --- a/pkg/workflow/update_project_job.go +++ /dev/null @@ -1,91 +0,0 @@ -package workflow - -import ( - "encoding/json" - "fmt" - - "github.com/githubnext/gh-aw/pkg/logger" -) - -var updateProjectJobLog = logger.New("workflow:update_project_job") - -// buildUpdateProjectJob creates the update_project job -func (c *Compiler) buildUpdateProjectJob(data *WorkflowData, mainJobName string) (*Job, error) { - updateProjectJobLog.Printf("Building update_project job: mainJobName=%s", mainJobName) - - if data.SafeOutputs == nil || data.SafeOutputs.UpdateProjects == nil { - updateProjectJobLog.Print("Missing safe-outputs.update-project configuration") - return nil, fmt.Errorf("safe-outputs.update-project configuration is required") - } - - // Build custom environment variables specific to update-project - var customEnvVars []string - - // Add common safe output job environment variables (staged/target repo) - // Note: Project operations always work on the current repo, so targetRepoSlug is "" - customEnvVars = append(customEnvVars, buildSafeOutputJobEnvVars( - c.trialMode, - c.trialLogicalRepoSlug, - data.SafeOutputs.Staged, - "", // targetRepoSlug - projects always work on current repo - )...) - - // Get token from config - var token string - if data.SafeOutputs.UpdateProjects != nil { - token = data.SafeOutputs.UpdateProjects.GitHubToken - } - - // Get the effective token using the Projects-specific precedence chain - // This includes fallback to GH_AW_PROJECT_GITHUB_TOKEN if no custom token is configured - // Note: Projects v2 requires a PAT or GitHub App - the default GITHUB_TOKEN cannot work - effectiveToken := getEffectiveProjectGitHubToken(token, data.GitHubToken) - - // Always expose the effective token as GH_AW_PROJECT_GITHUB_TOKEN environment variable - // The JavaScript code checks process.env.GH_AW_PROJECT_GITHUB_TOKEN to provide helpful error messages - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_GITHUB_TOKEN: %s\n", effectiveToken)) - - // If views are configured in frontmatter, pass them to the JavaScript via environment variable - if data.SafeOutputs.UpdateProjects != nil && len(data.SafeOutputs.UpdateProjects.Views) > 0 { - updateProjectJobLog.Printf("Marshaling %d project views to environment variable", len(data.SafeOutputs.UpdateProjects.Views)) - viewsJSON, err := json.Marshal(data.SafeOutputs.UpdateProjects.Views) - if err != nil { - updateProjectJobLog.Printf("Failed to marshal views configuration: %v", err) - return nil, fmt.Errorf("failed to marshal views configuration: %w", err) - } - // lgtm[go/unsafe-quoting] - This generates YAML environment variable declarations, not shell commands. - // The %q format specifier properly escapes the JSON string for YAML syntax. There is no shell injection - // risk because this value is set as an environment variable in the GitHub Actions YAML configuration, - // not executed as shell code. - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON))) - } - - jobCondition := BuildSafeOutputType("update_project") - permissions := NewPermissionsContentsReadProjectsWrite() - - updateProjectJobLog.Print("Building safe output job with update_project configuration") - - // Use buildSafeOutputJob helper to get common scaffolding including app token minting - job, err := c.buildSafeOutputJob(data, SafeOutputJobConfig{ - JobName: "update_project", - StepName: "Update Project", - StepID: "update_project", - MainJobName: mainJobName, - CustomEnvVars: customEnvVars, - Script: "", // Script is now handled by project handler manager - ScriptName: "update_project", - Permissions: permissions, - Outputs: nil, - Condition: jobCondition, - Needs: []string{mainJobName}, - Token: effectiveToken, - }) - - if err != nil { - updateProjectJobLog.Printf("Failed to build safe output job: %v", err) - } else { - updateProjectJobLog.Print("Successfully built update_project job") - } - - return job, err -} diff --git a/pkg/workflow/update_project_test.go b/pkg/workflow/update_project_test.go deleted file mode 100644 index d42571f0e1..0000000000 --- a/pkg/workflow/update_project_test.go +++ /dev/null @@ -1,328 +0,0 @@ -//go:build !integration - -package workflow - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestParseUpdateProjectConfig(t *testing.T) { - tests := []struct { - name string - outputMap map[string]any - expectedConfig *UpdateProjectConfig - expectedNil bool - }{ - { - name: "basic config with max", - outputMap: map[string]any{ - "update-project": map[string]any{ - "max": 5, - }, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 5, - }, - GitHubToken: "", - }, - }, - { - name: "config with custom github-token", - outputMap: map[string]any{ - "update-project": map[string]any{ - "max": 3, - "github-token": "${{ secrets.PROJECTS_PAT }}", - }, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 3, - }, - GitHubToken: "${{ secrets.PROJECTS_PAT }}", - }, - }, - { - name: "config with default max when not specified", - outputMap: map[string]any{ - "update-project": map[string]any{}, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 10, - }, - GitHubToken: "", - }, - }, - { - name: "config with only github-token", - outputMap: map[string]any{ - "update-project": map[string]any{ - "github-token": "${{ secrets.MY_TOKEN }}", - }, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 10, - }, - GitHubToken: "${{ secrets.MY_TOKEN }}", - }, - }, - { - name: "config with field-definitions", - outputMap: map[string]any{ - "update-project": map[string]any{ - "field-definitions": []any{ - map[string]any{ - "name": "status", - "data-type": "SINGLE_SELECT", - "options": []any{"Todo", "Done"}, - }, - map[string]any{ - "name": "campaign_id", - "data-type": "TEXT", - }, - }, - }, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 10}, - FieldDefinitions: []ProjectFieldDefinition{ - {Name: "status", DataType: "SINGLE_SELECT", Options: []string{"Todo", "Done"}}, - {Name: "campaign_id", DataType: "TEXT"}, - }, - }, - }, - { - name: "no update-project config", - outputMap: map[string]any{ - "create-issue": map[string]any{}, - }, - expectedNil: true, - }, - { - name: "empty outputMap", - outputMap: map[string]any{}, - expectedNil: true, - }, - { - name: "update-project is nil", - outputMap: map[string]any{ - "update-project": nil, - }, - expectedConfig: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 10, - }, - GitHubToken: "", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - compiler := NewCompiler() - config := compiler.parseUpdateProjectConfig(tt.outputMap) - - if tt.expectedNil { - assert.Nil(t, config, "Expected nil config") - } else { - require.NotNil(t, config, "Expected non-nil config") - assert.Equal(t, tt.expectedConfig.Max, config.Max, "Max should match") - assert.Equal(t, tt.expectedConfig.GitHubToken, config.GitHubToken, "GitHubToken should match") - assert.Equal(t, tt.expectedConfig.FieldDefinitions, config.FieldDefinitions, "FieldDefinitions should match") - } - }) - } -} - -func TestUpdateProjectConfig_DefaultMax(t *testing.T) { - compiler := NewCompiler() - - outputMap := map[string]any{ - "update-project": map[string]any{ - "github-token": "${{ secrets.TOKEN }}", - }, - } - - config := compiler.parseUpdateProjectConfig(outputMap) - require.NotNil(t, config) - - // Default max should be 10 when not specified - assert.Equal(t, 10, config.Max, "Default max should be 10") -} - -func TestUpdateProjectConfig_TokenPrecedence(t *testing.T) { - tests := []struct { - name string - configToken string - expectedToken string - }{ - { - name: "custom token specified", - configToken: "${{ secrets.CUSTOM_PAT }}", - expectedToken: "${{ secrets.CUSTOM_PAT }}", - }, - { - name: "empty token", - configToken: "", - expectedToken: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - compiler := NewCompiler() - - outputMap := map[string]any{ - "update-project": map[string]any{ - "github-token": tt.configToken, - }, - } - - config := compiler.parseUpdateProjectConfig(outputMap) - require.NotNil(t, config) - assert.Equal(t, tt.expectedToken, config.GitHubToken) - }) - } -} - -func TestBuildUpdateProjectJob(t *testing.T) { - tests := []struct { - name string - workflowData *WorkflowData - expectError bool - errorMsg string - }{ - { - name: "valid config", - workflowData: &WorkflowData{ - Name: "test-workflow", - SafeOutputs: &SafeOutputsConfig{ - UpdateProjects: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 5, - }, - GitHubToken: "${{ secrets.PROJECTS_PAT }}", - }, - }, - }, - expectError: false, - }, - { - name: "missing safe outputs config", - workflowData: &WorkflowData{ - Name: "test-workflow", - SafeOutputs: nil, - }, - expectError: true, - errorMsg: "safe-outputs.update-project configuration is required", - }, - { - name: "missing update project config", - workflowData: &WorkflowData{ - Name: "test-workflow", - SafeOutputs: &SafeOutputsConfig{ - UpdateProjects: nil, - }, - }, - expectError: true, - errorMsg: "safe-outputs.update-project configuration is required", - }, - { - name: "with default max", - workflowData: &WorkflowData{ - Name: "test-workflow", - SafeOutputs: &SafeOutputsConfig{ - UpdateProjects: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 10, - }, - }, - }, - }, - expectError: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - compiler := NewCompiler() - - job, err := compiler.buildUpdateProjectJob(tt.workflowData, "main") - - if tt.expectError { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.errorMsg) - assert.Nil(t, job) - } else { - require.NoError(t, err) - require.NotNil(t, job) - - // Verify job has basic structure - assert.NotEmpty(t, job.Steps, "Job should have steps") - } - }) - } -} - -func TestUpdateProjectJob_EnvironmentVariables(t *testing.T) { - compiler := NewCompiler() - - workflowData := &WorkflowData{ - Name: "test-workflow", - SafeOutputs: &SafeOutputsConfig{ - UpdateProjects: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 5, - }, - GitHubToken: "${{ secrets.PROJECTS_PAT }}", - }, - }, - } - - job, err := compiler.buildUpdateProjectJob(workflowData, "main") - require.NoError(t, err) - require.NotNil(t, job) - - // Job should contain steps - assert.NotEmpty(t, job.Steps, "Job should have steps") - - // Check that GH_AW_PROJECT_GITHUB_TOKEN is set in the environment - hasProjectToken := false - for _, step := range job.Steps { - if strings.Contains(step, "GH_AW_PROJECT_GITHUB_TOKEN") { - hasProjectToken = true - break - } - } - assert.True(t, hasProjectToken, "Job should set GH_AW_PROJECT_GITHUB_TOKEN environment variable") -} - -func TestUpdateProjectJob_Permissions(t *testing.T) { - compiler := NewCompiler() - - workflowData := &WorkflowData{ - Name: "test-workflow", - SafeOutputs: &SafeOutputsConfig{ - UpdateProjects: &UpdateProjectConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{ - Max: 10, - }, - }, - }, - } - - job, err := compiler.buildUpdateProjectJob(workflowData, "main") - require.NoError(t, err) - require.NotNil(t, job) - - // Verify permissions are set correctly - // update_project requires contents: read permission - require.NotEmpty(t, job.Permissions, "Job should have permissions") - assert.Contains(t, job.Permissions, "contents: read", "Should have contents: read permission") -} diff --git a/pkg/workflow/update_project_token_test.go b/pkg/workflow/update_project_token_test.go deleted file mode 100644 index b84b5777b4..0000000000 --- a/pkg/workflow/update_project_token_test.go +++ /dev/null @@ -1,104 +0,0 @@ -//go:build !integration - -package workflow - -import ( - "strings" - "testing" -) - -// TestUpdateProjectGitHubTokenEnvVar verifies that GH_AW_PROJECT_GITHUB_TOKEN -// is exposed as an environment variable for all update_project jobs -func TestUpdateProjectGitHubTokenEnvVar(t *testing.T) { - tests := []struct { - name string - frontmatter map[string]any - expectedEnvVarValue string - }{ - { - name: "update-project with custom github-token", - frontmatter: map[string]any{ - "name": "Test Workflow", - "safe-outputs": map[string]any{ - "update-project": map[string]any{ - "github-token": "${{ secrets.PROJECTS_PAT }}", - }, - }, - }, - expectedEnvVarValue: "GH_AW_PROJECT_GITHUB_TOKEN: ${{ secrets.PROJECTS_PAT }}", - }, - { - name: "update-project without custom github-token (uses GH_AW_PROJECT_GITHUB_TOKEN)", - frontmatter: map[string]any{ - "name": "Test Workflow", - "safe-outputs": map[string]any{ - "update-project": nil, - }, - }, - expectedEnvVarValue: "GH_AW_PROJECT_GITHUB_TOKEN: ${{ secrets.GH_AW_PROJECT_GITHUB_TOKEN }}", - }, - { - name: "update-project with top-level github-token", - frontmatter: map[string]any{ - "name": "Test Workflow", - "github-token": "${{ secrets.CUSTOM_TOKEN }}", - "safe-outputs": map[string]any{ - "update-project": nil, - }, - }, - expectedEnvVarValue: "GH_AW_PROJECT_GITHUB_TOKEN: ${{ secrets.CUSTOM_TOKEN }}", - }, - { - name: "update-project with per-config token overrides top-level", - frontmatter: map[string]any{ - "name": "Test Workflow", - "github-token": "${{ secrets.GLOBAL_TOKEN }}", - "safe-outputs": map[string]any{ - "update-project": map[string]any{ - "github-token": "${{ secrets.PROJECT_SPECIFIC_TOKEN }}", - }, - }, - }, - expectedEnvVarValue: "GH_AW_PROJECT_GITHUB_TOKEN: ${{ secrets.PROJECT_SPECIFIC_TOKEN }}", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - compiler := NewCompiler() - - // Parse frontmatter - workflowData := &WorkflowData{ - Name: "test-workflow", - SafeOutputs: compiler.extractSafeOutputsConfig(tt.frontmatter), - } - - // Set top-level github-token if present in frontmatter - if githubToken, ok := tt.frontmatter["github-token"].(string); ok { - workflowData.GitHubToken = githubToken - } - - // Build the update_project job - job, err := compiler.buildUpdateProjectJob(workflowData, "main") - if err != nil { - t.Fatalf("Failed to build update_project job: %v", err) - } - - // Convert job to YAML to check for environment variables - yamlStr := strings.Join(job.Steps, "") - - // Check that the environment variable is present with the expected value - if !strings.Contains(yamlStr, tt.expectedEnvVarValue) { - t.Errorf("Expected environment variable %q to be set in update_project job, but it was not found.\nGenerated YAML:\n%s", - tt.expectedEnvVarValue, yamlStr) - } - - // Also verify the token is passed to github-token parameter - expectedWith := "github-token: " + strings.TrimPrefix(tt.expectedEnvVarValue, "GH_AW_PROJECT_GITHUB_TOKEN: ") - if !strings.Contains(yamlStr, expectedWith) { - t.Errorf("Expected github-token parameter to be set to %q, but it was not found.\nGenerated YAML:\n%s", - expectedWith, yamlStr) - } - }) - } -}