From 882cfda28af7d0c479e49f1ff65bb26cae470577 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 11:29:28 +0000 Subject: [PATCH 1/2] Initial plan From f759e087fc7d705650a211673c7a8c6d82614612 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 11:39:53 +0000 Subject: [PATCH 2/2] Remove legacy update_project job builder and tests The update-project and create-project-status-update safe outputs are now handled by the unified project handler manager (buildProjectHandlerManagerStep). The individual job builders (buildUpdateProjectJob) were legacy code that was never called in production, only in tests. Changes: - Removed pkg/workflow/update_project_job.go (legacy job builder) - Removed pkg/workflow/update_project_test.go (tests for removed builder) - Removed pkg/workflow/update_project_token_test.go (tests for removed builder) - Updated pkg/workflow/safe_outputs_integration_test.go to remove update_project test case The configuration parsing functions (parseUpdateProjectConfig, parseCreateProjectStatusUpdateConfig) remain as they are used by the unified handler manager. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/safe_outputs_integration_test.go | 18 - pkg/workflow/update_project_job.go | 91 ----- pkg/workflow/update_project_test.go | 328 ------------------ pkg/workflow/update_project_token_test.go | 104 ------ 4 files changed, 541 deletions(-) delete mode 100644 pkg/workflow/update_project_job.go delete mode 100644 pkg/workflow/update_project_test.go delete mode 100644 pkg/workflow/update_project_token_test.go 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) - } - }) - } -}