Skip to content

Commit 47ab8dd

Browse files
Copilotpelikhan
andauthored
fix: use artifact prefix in conclusion job and script step downloads for workflow_call context (#21011)
* Initial plan * fix: use artifact prefix in conclusion and script step downloads for workflow_call context In workflow_call-compiled lock files, the conclusion job was downloading the agent artifact with an unprefixed name ('agent') while the upload step used the per-invocation prefix ('${{ needs.activation.outputs.artifact_prefix }}agent'). This mismatch caused the download to miss, leaving GH_AW_AGENT_OUTPUT unset and breaking downstream safe-output processing. Fix: pass artifactPrefixExprForDownstreamJob(data) to buildAgentOutputDownloadSteps in buildConclusionJob, buildGitHubScriptStep, and buildCustomActionStep. Also adds two new tests: - TestConclusionJobWorkflowCallArtifactPrefix: verifies prefixed artifact name in workflow_call context - TestConclusionJobNonWorkflowCallNoArtifactPrefix: verifies no prefix in non-workflow_call context Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: use agent-downstream artifact prefix in buildGitHubScriptStep and buildCustomActionStep buildSafeOutputJob-based jobs (e.g. upload_assets) only depend on the agent job, not the activation job. Using needs.activation.outputs.artifact_prefix in those jobs would break workflow_call workflows since activation is not in their needs. Use artifactPrefixExprForAgentDownstreamJob (needs.agent.outputs.artifact_prefix) in buildGitHubScriptStep and buildCustomActionStep instead. Also adds two tests verifying the correct prefix expression is used per context. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent c87077e commit 47ab8dd

6 files changed

Lines changed: 139 additions & 8 deletions

File tree

.github/workflows/smoke-workflow-call-with-inputs.lock.yml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-workflow-call.lock.yml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/workflow/notify_comment.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa
6060
steps = append(steps, c.buildGitHubAppTokenMintStep(data.SafeOutputs.GitHubApp, permissions, appTokenFallbackRepo)...)
6161
}
6262

63-
// Add artifact download steps once (shared by noop and conclusion steps)
64-
steps = append(steps, buildAgentOutputDownloadSteps("")...)
63+
// Add artifact download steps once (shared by noop and conclusion steps).
64+
// In workflow_call context, use the per-invocation prefix to avoid artifact name clashes.
65+
steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForDownstreamJob(data))...)
6566

6667
// Add noop processing step if noop is configured
6768
if data.SafeOutputs.NoOp != nil {

pkg/workflow/notify_comment_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,3 +914,68 @@ func TestConclusionJobNoPushRepoMemoryResult(t *testing.T) {
914914
t.Error("Expected conclusion job to NOT include GH_AW_PUSH_REPO_MEMORY_RESULT when repo-memory is not configured")
915915
}
916916
}
917+
918+
// TestConclusionJobWorkflowCallArtifactPrefix verifies that the conclusion job uses the
919+
// prefixed artifact name in workflow_call context to match the upload step.
920+
func TestConclusionJobWorkflowCallArtifactPrefix(t *testing.T) {
921+
compiler := NewCompiler()
922+
923+
workflowData := &WorkflowData{
924+
Name: "Test Workflow",
925+
On: "workflow_call",
926+
SafeOutputs: &SafeOutputsConfig{
927+
NoOp: &NoOpConfig{},
928+
},
929+
}
930+
931+
job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{})
932+
if err != nil {
933+
t.Fatalf("Failed to build conclusion job: %v", err)
934+
}
935+
if job == nil {
936+
t.Fatal("Expected conclusion job to be created")
937+
}
938+
939+
allSteps := strings.Join(job.Steps, "\n")
940+
941+
// In workflow_call context, the artifact download must use the prefixed name
942+
// to match the upload step which uses needs.activation.outputs.artifact_prefix.
943+
prefixedArtifactName := "${{ needs.activation.outputs.artifact_prefix }}agent"
944+
if !strings.Contains(allSteps, prefixedArtifactName) {
945+
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)
946+
}
947+
948+
// Ensure the unprefixed artifact name is not used
949+
if strings.Contains(allSteps, "name: agent\n") {
950+
t.Errorf("Expected conclusion job NOT to use unprefixed artifact name 'agent' in workflow_call context.\nGenerated steps:\n%s", allSteps)
951+
}
952+
}
953+
954+
// TestConclusionJobNonWorkflowCallNoArtifactPrefix verifies that the conclusion job uses
955+
// the unprefixed artifact name for non-workflow_call workflows.
956+
func TestConclusionJobNonWorkflowCallNoArtifactPrefix(t *testing.T) {
957+
compiler := NewCompiler()
958+
959+
workflowData := &WorkflowData{
960+
Name: "Test Workflow",
961+
On: "issues",
962+
SafeOutputs: &SafeOutputsConfig{
963+
NoOp: &NoOpConfig{},
964+
},
965+
}
966+
967+
job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{})
968+
if err != nil {
969+
t.Fatalf("Failed to build conclusion job: %v", err)
970+
}
971+
if job == nil {
972+
t.Fatal("Expected conclusion job to be created")
973+
}
974+
975+
allSteps := strings.Join(job.Steps, "\n")
976+
977+
// In non-workflow_call context, the artifact download should use the plain (unprefixed) name.
978+
if strings.Contains(allSteps, "needs.activation.outputs.artifact_prefix") {
979+
t.Errorf("Expected conclusion job NOT to use artifact_prefix in non-workflow_call context.\nGenerated steps:\n%s", allSteps)
980+
}
981+
}

pkg/workflow/safe_output_helpers_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,3 +682,62 @@ func TestBuildGitHubScriptStepNoWorkingDirectory(t *testing.T) {
682682
t.Error("Expected step to contain script")
683683
}
684684
}
685+
686+
// TestBuildGitHubScriptStepWorkflowCallArtifactPrefix verifies that agent artifact downloads
687+
// in buildGitHubScriptStep use needs.agent.outputs.artifact_prefix in workflow_call context.
688+
// These steps are used in jobs that depend on the agent job (not activation), so the
689+
// agent-downstream prefix expression must be used.
690+
func TestBuildGitHubScriptStepWorkflowCallArtifactPrefix(t *testing.T) {
691+
compiler := &Compiler{}
692+
workflowData := &WorkflowData{
693+
Name: "Test Workflow",
694+
On: "workflow_call",
695+
}
696+
697+
config := GitHubScriptStepConfig{
698+
StepName: "Test Step",
699+
StepID: "test_step",
700+
MainJobName: "agent",
701+
Script: "console.log('test');",
702+
}
703+
704+
steps := compiler.buildGitHubScriptStep(workflowData, config)
705+
stepsStr := strings.Join(steps, "")
706+
707+
// In workflow_call context, the download must reference needs.agent (not needs.activation)
708+
// because buildSafeOutputJob-based jobs depend on agent, not activation.
709+
agentPrefix := "${{ needs.agent.outputs.artifact_prefix }}agent"
710+
if !strings.Contains(stepsStr, agentPrefix) {
711+
t.Errorf("Expected buildGitHubScriptStep to use %q in workflow_call context, but it was not found.\nGenerated steps:\n%s", agentPrefix, stepsStr)
712+
}
713+
714+
// Ensure activation prefix is NOT used
715+
if strings.Contains(stepsStr, "needs.activation.outputs.artifact_prefix") {
716+
t.Errorf("Expected buildGitHubScriptStep NOT to use needs.activation.outputs.artifact_prefix (job does not depend on activation).\nGenerated steps:\n%s", stepsStr)
717+
}
718+
}
719+
720+
// TestBuildGitHubScriptStepNonWorkflowCallNoArtifactPrefix verifies no prefix is added
721+
// in non-workflow_call context.
722+
func TestBuildGitHubScriptStepNonWorkflowCallNoArtifactPrefix(t *testing.T) {
723+
compiler := &Compiler{}
724+
workflowData := &WorkflowData{
725+
Name: "Test Workflow",
726+
On: "issues",
727+
}
728+
729+
config := GitHubScriptStepConfig{
730+
StepName: "Test Step",
731+
StepID: "test_step",
732+
MainJobName: "agent",
733+
Script: "console.log('test');",
734+
}
735+
736+
steps := compiler.buildGitHubScriptStep(workflowData, config)
737+
stepsStr := strings.Join(steps, "")
738+
739+
// In non-workflow_call context, no artifact prefix should be used
740+
if strings.Contains(stepsStr, "artifact_prefix") {
741+
t.Errorf("Expected buildGitHubScriptStep NOT to use artifact_prefix in non-workflow_call context.\nGenerated steps:\n%s", stepsStr)
742+
}
743+
}

pkg/workflow/safe_outputs_jobs.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,11 @@ func (c *Compiler) buildCustomActionStep(data *WorkflowData, config GitHubScript
188188
return c.buildGitHubScriptStep(data, config)
189189
}
190190

191-
// Add artifact download steps before the custom action step
192-
steps = append(steps, buildAgentOutputDownloadSteps("")...)
191+
// Add artifact download steps before the custom action step.
192+
// In workflow_call context, use the per-invocation prefix to avoid artifact name clashes.
193+
// These steps are used in jobs that depend on the agent job (not activation), so use
194+
// the agent-downstream prefix expression.
195+
steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForAgentDownstreamJob(data))...)
193196

194197
// Step name and metadata
195198
steps = append(steps, fmt.Sprintf(" - name: %s\n", config.StepName))
@@ -287,8 +290,11 @@ func (c *Compiler) buildGitHubScriptStep(data *WorkflowData, config GitHubScript
287290

288291
var steps []string
289292

290-
// Add artifact download steps before the GitHub Script step
291-
steps = append(steps, buildAgentOutputDownloadSteps("")...)
293+
// Add artifact download steps before the GitHub Script step.
294+
// In workflow_call context, use the per-invocation prefix to avoid artifact name clashes.
295+
// These steps are used in jobs that depend on the agent job (not activation), so use
296+
// the agent-downstream prefix expression.
297+
steps = append(steps, buildAgentOutputDownloadSteps(artifactPrefixExprForAgentDownstreamJob(data))...)
292298

293299
// Step name and metadata
294300
steps = append(steps, fmt.Sprintf(" - name: %s\n", config.StepName))

0 commit comments

Comments
 (0)