From cace639c8fb190c61fc8d8f6858299beec49c2b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:19:13 +0000 Subject: [PATCH 1/6] Initial plan From db8c7617ab87f45d5bce46d52d062541ff0efda6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:36:53 +0000 Subject: [PATCH 2/6] fix(workflow): skip built-in jobs when building agent dependencies Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cda45cd0-2cd5-4281-8447-72e88f5168af Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_test.go | 100 +++++++++++++++++++++++++++++ pkg/workflow/compiler_main_job.go | 21 ++++-- 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index 18c3a7e2f0f..c9538609455 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -742,6 +742,106 @@ func TestBuildMainJobWithActivation(t *testing.T) { } } +func TestBuildMainJobSkipsBuiltInJobCustomizationsFromNeeds(t *testing.T) { + tests := []struct { + name string + jobName string + forbidden string + verifyCount bool + }{ + { + name: "activation pre-steps does not duplicate activation needs", + jobName: string(constants.ActivationJobName), + forbidden: string(constants.ActivationJobName), + verifyCount: true, + }, + { + name: "agent pre-steps does not create self-cycle", + jobName: string(constants.AgentJobName), + forbidden: string(constants.AgentJobName), + }, + { + name: "safe_outputs pre-steps does not create cycle with agent", + jobName: string(constants.SafeOutputsJobName), + forbidden: string(constants.SafeOutputsJobName), + }, + { + name: "conclusion pre-steps does not create cycle with agent", + jobName: string(constants.ConclusionJobName), + forbidden: string(constants.ConclusionJobName), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + compiler.stepOrderTracker = NewStepOrderTracker() + + workflowData := &WorkflowData{ + Name: "Test Workflow", + AI: "copilot", + RunsOn: "runs-on: ubuntu-latest", + Permissions: "permissions:\n contents: read", + Jobs: map[string]any{ + tt.jobName: map[string]any{ + "pre-steps": []any{ + map[string]any{ + "name": "Pre-step", + "run": "echo test", + }, + }, + }, + }, + } + + job, err := compiler.buildMainJob(workflowData, true) + if err != nil { + t.Fatalf("buildMainJob() returned error: %v", err) + } + + if tt.verifyCount { + count := 0 + for _, need := range job.Needs { + if need == tt.forbidden { + count++ + } + } + if count != 1 { + t.Fatalf("Expected exactly one %q dependency, got %d (needs: %v)", tt.forbidden, count, job.Needs) + } + } else if slices.Contains(job.Needs, tt.forbidden) { + t.Fatalf("Did not expect %q in agent needs, got: %v", tt.forbidden, job.Needs) + } + }) + } +} + +func TestIsBuiltInJobName(t *testing.T) { + tests := []struct { + name string + jobName string + expected bool + }{ + {name: "pre_activation canonical", jobName: string(constants.PreActivationJobName), expected: true}, + {name: "pre-activation alias", jobName: "pre-activation", expected: true}, + {name: "activation", jobName: string(constants.ActivationJobName), expected: true}, + {name: "agent", jobName: string(constants.AgentJobName), expected: true}, + {name: "safe_outputs canonical", jobName: string(constants.SafeOutputsJobName), expected: true}, + {name: "safe-outputs alias", jobName: "safe-outputs", expected: true}, + {name: "conclusion", jobName: string(constants.ConclusionJobName), expected: true}, + {name: "custom job", jobName: "custom_job", expected: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := isBuiltInJobName(tt.jobName) + if actual != tt.expected { + t.Fatalf("isBuiltInJobName(%q) = %t, want %t", tt.jobName, actual, tt.expected) + } + }) + } +} + // TestBuildCustomJobsWithActivation tests building custom jobs with activation dependency func TestBuildCustomJobsWithActivation(t *testing.T) { tmpDir := testutil.TempDir(t, "custom-jobs-test") diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index 99e6b93c14d..abb7c6a3054 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -13,6 +13,19 @@ import ( var compilerMainJobLog = logger.New("workflow:compiler_main_job") +func isBuiltInJobName(jobName string) bool { + switch jobName { + case string(constants.PreActivationJobName), "pre-activation", + string(constants.ActivationJobName), + string(constants.AgentJobName), + string(constants.SafeOutputsJobName), "safe-outputs", + string(constants.ConclusionJobName): + return true + default: + return false + } +} + // buildMainJob creates the main agent job that runs the AI agent with the configured engine and tools. // This job depends on the activation job if it exists, and handles the main workflow logic. func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) (*Job, error) { @@ -92,8 +105,8 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( // Custom jobs that depend on agent should run AFTER the agent job, not before it if data.Jobs != nil { for _, jobName := range slices.Sorted(maps.Keys(data.Jobs)) { - // Skip jobs.pre-activation (or pre_activation) as it's handled specially - if jobName == string(constants.PreActivationJobName) || jobName == "pre-activation" { + // Skip built-in jobs as they are handled separately and should not become custom dependencies. + if isBuiltInJobName(jobName) { continue } @@ -121,8 +134,8 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( } referencedJobs := c.getReferencedCustomJobs(contentBuilder.String(), data.Jobs) for _, jobName := range referencedJobs { - // Skip jobs.pre-activation (or pre_activation) as it's handled specially - if jobName == string(constants.PreActivationJobName) || jobName == "pre-activation" { + // Skip built-in jobs as they are handled separately and should not become custom dependencies. + if isBuiltInJobName(jobName) { continue } From a761e029fa3afebc6e2fe3661b19f7b282236272 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:42:21 +0000 Subject: [PATCH 3/6] test(workflow): cover built-in job name edge cases Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cda45cd0-2cd5-4281-8447-72e88f5168af Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index c9538609455..04a127ecc15 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -829,6 +829,11 @@ func TestIsBuiltInJobName(t *testing.T) { {name: "safe_outputs canonical", jobName: string(constants.SafeOutputsJobName), expected: true}, {name: "safe-outputs alias", jobName: "safe-outputs", expected: true}, {name: "conclusion", jobName: string(constants.ConclusionJobName), expected: true}, + {name: "empty string", jobName: "", expected: false}, + {name: "different casing activation", jobName: "ACTIVATION", expected: false}, + {name: "different casing agent", jobName: "Agent", expected: false}, + {name: "partial pre-activation match", jobName: "pre-activation-custom", expected: false}, + {name: "partial agent match", jobName: "agent-step", expected: false}, {name: "custom job", jobName: "custom_job", expected: false}, } From 619bedc840b468bf71d5ecbf965d38a5ee7620fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:48:28 +0000 Subject: [PATCH 4/6] refactor(workflow): use builtin naming convention for helper Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cda45cd0-2cd5-4281-8447-72e88f5168af Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_test.go | 6 +++--- pkg/workflow/compiler_main_job.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index 04a127ecc15..c53465754a6 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -816,7 +816,7 @@ func TestBuildMainJobSkipsBuiltInJobCustomizationsFromNeeds(t *testing.T) { } } -func TestIsBuiltInJobName(t *testing.T) { +func TestIsBuiltinJobName(t *testing.T) { tests := []struct { name string jobName string @@ -839,9 +839,9 @@ func TestIsBuiltInJobName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual := isBuiltInJobName(tt.jobName) + actual := isBuiltinJobName(tt.jobName) if actual != tt.expected { - t.Fatalf("isBuiltInJobName(%q) = %t, want %t", tt.jobName, actual, tt.expected) + t.Fatalf("isBuiltinJobName(%q) = %t, want %t", tt.jobName, actual, tt.expected) } }) } diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index abb7c6a3054..13c3061ebf5 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -13,7 +13,7 @@ import ( var compilerMainJobLog = logger.New("workflow:compiler_main_job") -func isBuiltInJobName(jobName string) bool { +func isBuiltinJobName(jobName string) bool { switch jobName { case string(constants.PreActivationJobName), "pre-activation", string(constants.ActivationJobName), @@ -106,7 +106,7 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( if data.Jobs != nil { for _, jobName := range slices.Sorted(maps.Keys(data.Jobs)) { // Skip built-in jobs as they are handled separately and should not become custom dependencies. - if isBuiltInJobName(jobName) { + if isBuiltinJobName(jobName) { continue } @@ -135,7 +135,7 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( referencedJobs := c.getReferencedCustomJobs(contentBuilder.String(), data.Jobs) for _, jobName := range referencedJobs { // Skip built-in jobs as they are handled separately and should not become custom dependencies. - if isBuiltInJobName(jobName) { + if isBuiltinJobName(jobName) { continue } From f51e5600e93d05802df772eb0a1ad8e28d646bfb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 04:02:58 +0000 Subject: [PATCH 5/6] refactor(constants): centralize known built-in workflow job names Agent-Logs-Url: https://github.com/github/gh-aw/sessions/297b8ad8-80f4-4721-b825-fda378b948dc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/constants/constants_test.go | 31 ++++++++++++++++++++++++++++++ pkg/constants/job_constants.go | 17 ++++++++++++++++ pkg/workflow/compiler_jobs_test.go | 4 ++++ pkg/workflow/compiler_main_job.go | 12 ++---------- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/pkg/constants/constants_test.go b/pkg/constants/constants_test.go index 824b0f4885f..a6e968dde16 100644 --- a/pkg/constants/constants_test.go +++ b/pkg/constants/constants_test.go @@ -233,7 +233,14 @@ func TestConstantValues(t *testing.T) { {"AgentJobName", string(AgentJobName), "agent"}, {"ActivationJobName", string(ActivationJobName), "activation"}, {"PreActivationJobName", string(PreActivationJobName), "pre_activation"}, + {"PreActivationHyphenJobName", string(PreActivationHyphenJobName), "pre-activation"}, {"DetectionJobName", string(DetectionJobName), "detection"}, + {"SafeOutputsJobName", string(SafeOutputsJobName), "safe_outputs"}, + {"SafeOutputsHyphenJobName", string(SafeOutputsHyphenJobName), "safe-outputs"}, + {"UploadAssetsJobName", string(UploadAssetsJobName), "upload_assets"}, + {"UploadCodeScanningJobName", string(UploadCodeScanningJobName), "upload_code_scanning_sarif"}, + {"ConclusionJobName", string(ConclusionJobName), "conclusion"}, + {"UnlockJobName", string(UnlockJobName), "unlock"}, {"SafeOutputArtifactName", SafeOutputArtifactName, "safe-output"}, {"AgentOutputArtifactName", AgentOutputArtifactName, "agent-output"}, {"SafeOutputItemsArtifactName", SafeOutputItemsArtifactName, "safe-outputs-items"}, @@ -262,6 +269,30 @@ func TestConstantValues(t *testing.T) { } } +func TestKnownBuiltInJobNamesContainsAllKnownJobs(t *testing.T) { + t.Helper() + + knownJobs := []string{ + string(AgentJobName), + string(ActivationJobName), + string(PreActivationJobName), + string(PreActivationHyphenJobName), + string(DetectionJobName), + string(SafeOutputsJobName), + string(SafeOutputsHyphenJobName), + string(UploadAssetsJobName), + string(UploadCodeScanningJobName), + string(ConclusionJobName), + string(UnlockJobName), + } + + for _, jobName := range knownJobs { + if _, ok := KnownBuiltInJobNames[jobName]; !ok { + t.Errorf("KnownBuiltInJobNames missing %q", jobName) + } + } +} + func TestModelNameConstants(t *testing.T) { // Test that ModelName type works correctly tests := []struct { diff --git a/pkg/constants/job_constants.go b/pkg/constants/job_constants.go index d509cff58af..0747f54483c 100644 --- a/pkg/constants/job_constants.go +++ b/pkg/constants/job_constants.go @@ -59,13 +59,30 @@ func (m MCPServerID) String() string { const AgentJobName JobName = "agent" const ActivationJobName JobName = "activation" const PreActivationJobName JobName = "pre_activation" +const PreActivationHyphenJobName JobName = "pre-activation" const DetectionJobName JobName = "detection" const SafeOutputsJobName JobName = "safe_outputs" +const SafeOutputsHyphenJobName JobName = "safe-outputs" const UploadAssetsJobName JobName = "upload_assets" const UploadCodeScanningJobName JobName = "upload_code_scanning_sarif" const ConclusionJobName JobName = "conclusion" const UnlockJobName JobName = "unlock" +// KnownBuiltInJobNames contains all known built-in workflow job names (including aliases). +var KnownBuiltInJobNames = map[string]struct{}{ + string(AgentJobName): {}, + string(ActivationJobName): {}, + string(PreActivationJobName): {}, + string(PreActivationHyphenJobName): {}, + string(DetectionJobName): {}, + string(SafeOutputsJobName): {}, + string(SafeOutputsHyphenJobName): {}, + string(UploadAssetsJobName): {}, + string(UploadCodeScanningJobName): {}, + string(ConclusionJobName): {}, + string(UnlockJobName): {}, +} + // Artifact name constants const SafeOutputArtifactName = "safe-output" const AgentOutputArtifactName = "agent-output" diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index c53465754a6..5d032720ed5 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -829,6 +829,10 @@ func TestIsBuiltinJobName(t *testing.T) { {name: "safe_outputs canonical", jobName: string(constants.SafeOutputsJobName), expected: true}, {name: "safe-outputs alias", jobName: "safe-outputs", expected: true}, {name: "conclusion", jobName: string(constants.ConclusionJobName), expected: true}, + {name: "detection", jobName: string(constants.DetectionJobName), expected: true}, + {name: "upload_assets", jobName: string(constants.UploadAssetsJobName), expected: true}, + {name: "upload_code_scanning_sarif", jobName: string(constants.UploadCodeScanningJobName), expected: true}, + {name: "unlock", jobName: string(constants.UnlockJobName), expected: true}, {name: "empty string", jobName: "", expected: false}, {name: "different casing activation", jobName: "ACTIVATION", expected: false}, {name: "different casing agent", jobName: "Agent", expected: false}, diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index 13c3061ebf5..b48ef701b5c 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -14,16 +14,8 @@ import ( var compilerMainJobLog = logger.New("workflow:compiler_main_job") func isBuiltinJobName(jobName string) bool { - switch jobName { - case string(constants.PreActivationJobName), "pre-activation", - string(constants.ActivationJobName), - string(constants.AgentJobName), - string(constants.SafeOutputsJobName), "safe-outputs", - string(constants.ConclusionJobName): - return true - default: - return false - } + _, isBuiltIn := constants.KnownBuiltInJobNames[jobName] + return isBuiltIn } // buildMainJob creates the main agent job that runs the AI agent with the configured engine and tools. From 1e7e1e0fca780ea6c06632db5d774ec17d464757 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 04:09:01 +0000 Subject: [PATCH 6/6] test(constants): refine built-in job constants coverage comments Agent-Logs-Url: https://github.com/github/gh-aw/sessions/297b8ad8-80f4-4721-b825-fda378b948dc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/constants/constants_test.go | 2 -- pkg/constants/job_constants.go | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/constants/constants_test.go b/pkg/constants/constants_test.go index a6e968dde16..32a30b3a5a2 100644 --- a/pkg/constants/constants_test.go +++ b/pkg/constants/constants_test.go @@ -270,8 +270,6 @@ func TestConstantValues(t *testing.T) { } func TestKnownBuiltInJobNamesContainsAllKnownJobs(t *testing.T) { - t.Helper() - knownJobs := []string{ string(AgentJobName), string(ActivationJobName), diff --git a/pkg/constants/job_constants.go b/pkg/constants/job_constants.go index 0747f54483c..bcf39443322 100644 --- a/pkg/constants/job_constants.go +++ b/pkg/constants/job_constants.go @@ -69,6 +69,9 @@ const ConclusionJobName JobName = "conclusion" const UnlockJobName JobName = "unlock" // KnownBuiltInJobNames contains all known built-in workflow job names (including aliases). +// It is used for O(1) membership checks when validating or filtering user-defined job +// names to avoid collisions with framework-generated jobs. For example, workflow code +// can check this map before treating a frontmatter jobs. entry as a custom job. var KnownBuiltInJobNames = map[string]struct{}{ string(AgentJobName): {}, string(ActivationJobName): {},