diff --git a/.github/workflows/smoke-workflow-call-with-inputs.lock.yml b/.github/workflows/smoke-workflow-call-with-inputs.lock.yml index e837e53f8a..badb271f9e 100644 --- a/.github/workflows/smoke-workflow-call-with-inputs.lock.yml +++ b/.github/workflows/smoke-workflow-call-with-inputs.lock.yml @@ -1084,7 +1084,7 @@ jobs: continue-on-error: true uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - name: agent + name: ${{ needs.activation.outputs.artifact_prefix }}agent path: /tmp/gh-aw/ - name: Setup agent output environment variable if: steps.download-agent-output.outcome == 'success' diff --git a/.github/workflows/smoke-workflow-call.lock.yml b/.github/workflows/smoke-workflow-call.lock.yml index 7dba9c88cb..b258c31803 100644 --- a/.github/workflows/smoke-workflow-call.lock.yml +++ b/.github/workflows/smoke-workflow-call.lock.yml @@ -1043,7 +1043,7 @@ jobs: continue-on-error: true uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - name: agent + name: ${{ needs.activation.outputs.artifact_prefix }}agent path: /tmp/gh-aw/ - name: Setup agent output environment variable if: steps.download-agent-output.outcome == 'success' diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index e850f56acd..d6cd6e9dcf 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -60,8 +60,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa steps = append(steps, c.buildGitHubAppTokenMintStep(data.SafeOutputs.GitHubApp, permissions, appTokenFallbackRepo)...) } - // Add artifact download steps once (shared by noop and conclusion steps) - steps = append(steps, buildAgentOutputDownloadSteps("")...) + // Add artifact download steps once (shared by noop and conclusion steps). + // In workflow_call context, use the per-invocation prefix to avoid artifact name clashes. + steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForDownstreamJob(data))...) // Add noop processing step if noop is configured if data.SafeOutputs.NoOp != nil { diff --git a/pkg/workflow/notify_comment_test.go b/pkg/workflow/notify_comment_test.go index 4b60d81a9b..4dddeaa6b0 100644 --- a/pkg/workflow/notify_comment_test.go +++ b/pkg/workflow/notify_comment_test.go @@ -914,3 +914,68 @@ func TestConclusionJobNoPushRepoMemoryResult(t *testing.T) { t.Error("Expected conclusion job to NOT include GH_AW_PUSH_REPO_MEMORY_RESULT when repo-memory is not configured") } } + +// TestConclusionJobWorkflowCallArtifactPrefix verifies that the conclusion job uses the +// prefixed artifact name in workflow_call context to match the upload step. +func TestConclusionJobWorkflowCallArtifactPrefix(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test Workflow", + On: "workflow_call", + SafeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + }, + } + + job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{}) + if err != nil { + t.Fatalf("Failed to build conclusion job: %v", err) + } + if job == nil { + t.Fatal("Expected conclusion job to be created") + } + + allSteps := strings.Join(job.Steps, "\n") + + // In workflow_call context, the artifact download must use the prefixed name + // to match the upload step which uses needs.activation.outputs.artifact_prefix. + prefixedArtifactName := "${{ needs.activation.outputs.artifact_prefix }}agent" + if !strings.Contains(allSteps, prefixedArtifactName) { + t.Errorf("Expected conclusion job download step to use prefixed artifact name %q in workflow_call context, but it was not found.\nGenerated steps:\n%s", prefixedArtifactName, allSteps) + } + + // Ensure the unprefixed artifact name is not used + if strings.Contains(allSteps, "name: agent\n") { + t.Errorf("Expected conclusion job NOT to use unprefixed artifact name 'agent' in workflow_call context.\nGenerated steps:\n%s", allSteps) + } +} + +// TestConclusionJobNonWorkflowCallNoArtifactPrefix verifies that the conclusion job uses +// the unprefixed artifact name for non-workflow_call workflows. +func TestConclusionJobNonWorkflowCallNoArtifactPrefix(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test Workflow", + On: "issues", + SafeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + }, + } + + job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{}) + if err != nil { + t.Fatalf("Failed to build conclusion job: %v", err) + } + if job == nil { + t.Fatal("Expected conclusion job to be created") + } + + allSteps := strings.Join(job.Steps, "\n") + + // In non-workflow_call context, the artifact download should use the plain (unprefixed) name. + if strings.Contains(allSteps, "needs.activation.outputs.artifact_prefix") { + t.Errorf("Expected conclusion job NOT to use artifact_prefix in non-workflow_call context.\nGenerated steps:\n%s", allSteps) + } +} diff --git a/pkg/workflow/safe_output_helpers_test.go b/pkg/workflow/safe_output_helpers_test.go index 3d542d5f76..962436f33e 100644 --- a/pkg/workflow/safe_output_helpers_test.go +++ b/pkg/workflow/safe_output_helpers_test.go @@ -682,3 +682,62 @@ func TestBuildGitHubScriptStepNoWorkingDirectory(t *testing.T) { t.Error("Expected step to contain script") } } + +// TestBuildGitHubScriptStepWorkflowCallArtifactPrefix verifies that agent artifact downloads +// in buildGitHubScriptStep use needs.agent.outputs.artifact_prefix in workflow_call context. +// These steps are used in jobs that depend on the agent job (not activation), so the +// agent-downstream prefix expression must be used. +func TestBuildGitHubScriptStepWorkflowCallArtifactPrefix(t *testing.T) { + compiler := &Compiler{} + workflowData := &WorkflowData{ + Name: "Test Workflow", + On: "workflow_call", + } + + config := GitHubScriptStepConfig{ + StepName: "Test Step", + StepID: "test_step", + MainJobName: "agent", + Script: "console.log('test');", + } + + steps := compiler.buildGitHubScriptStep(workflowData, config) + stepsStr := strings.Join(steps, "") + + // In workflow_call context, the download must reference needs.agent (not needs.activation) + // because buildSafeOutputJob-based jobs depend on agent, not activation. + agentPrefix := "${{ needs.agent.outputs.artifact_prefix }}agent" + if !strings.Contains(stepsStr, agentPrefix) { + t.Errorf("Expected buildGitHubScriptStep to use %q in workflow_call context, but it was not found.\nGenerated steps:\n%s", agentPrefix, stepsStr) + } + + // Ensure activation prefix is NOT used + if strings.Contains(stepsStr, "needs.activation.outputs.artifact_prefix") { + t.Errorf("Expected buildGitHubScriptStep NOT to use needs.activation.outputs.artifact_prefix (job does not depend on activation).\nGenerated steps:\n%s", stepsStr) + } +} + +// TestBuildGitHubScriptStepNonWorkflowCallNoArtifactPrefix verifies no prefix is added +// in non-workflow_call context. +func TestBuildGitHubScriptStepNonWorkflowCallNoArtifactPrefix(t *testing.T) { + compiler := &Compiler{} + workflowData := &WorkflowData{ + Name: "Test Workflow", + On: "issues", + } + + config := GitHubScriptStepConfig{ + StepName: "Test Step", + StepID: "test_step", + MainJobName: "agent", + Script: "console.log('test');", + } + + steps := compiler.buildGitHubScriptStep(workflowData, config) + stepsStr := strings.Join(steps, "") + + // In non-workflow_call context, no artifact prefix should be used + if strings.Contains(stepsStr, "artifact_prefix") { + t.Errorf("Expected buildGitHubScriptStep NOT to use artifact_prefix in non-workflow_call context.\nGenerated steps:\n%s", stepsStr) + } +} diff --git a/pkg/workflow/safe_outputs_jobs.go b/pkg/workflow/safe_outputs_jobs.go index 13e36cef30..1d2f9fab4b 100644 --- a/pkg/workflow/safe_outputs_jobs.go +++ b/pkg/workflow/safe_outputs_jobs.go @@ -188,8 +188,11 @@ func (c *Compiler) buildCustomActionStep(data *WorkflowData, config GitHubScript return c.buildGitHubScriptStep(data, config) } - // Add artifact download steps before the custom action step - steps = append(steps, buildAgentOutputDownloadSteps("")...) + // Add artifact download steps before the custom action step. + // In workflow_call context, use the per-invocation prefix to avoid artifact name clashes. + // These steps are used in jobs that depend on the agent job (not activation), so use + // the agent-downstream prefix expression. + steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForAgentDownstreamJob(data))...) // Step name and metadata steps = append(steps, fmt.Sprintf(" - name: %s\n", config.StepName)) @@ -287,8 +290,11 @@ func (c *Compiler) buildGitHubScriptStep(data *WorkflowData, config GitHubScript var steps []string - // Add artifact download steps before the GitHub Script step - steps = append(steps, buildAgentOutputDownloadSteps("")...) + // Add artifact download steps before the GitHub Script step. + // In workflow_call context, use the per-invocation prefix to avoid artifact name clashes. + // These steps are used in jobs that depend on the agent job (not activation), so use + // the agent-downstream prefix expression. + steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForAgentDownstreamJob(data))...) // Step name and metadata steps = append(steps, fmt.Sprintf(" - name: %s\n", config.StepName))