Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/smoke-workflow-call-with-inputs.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-workflow-call.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions pkg/workflow/notify_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
65 changes: 65 additions & 0 deletions pkg/workflow/notify_comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
59 changes: 59 additions & 0 deletions pkg/workflow/safe_output_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
14 changes: 10 additions & 4 deletions pkg/workflow/safe_outputs_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
Loading