From 6f0cd86bf6064cfceb6a424a09f171ba4e6cdff0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:04:21 +0000 Subject: [PATCH 1/2] Initial plan From 0e815e6fd4b7c11c05b89843b3e702fb9b5e6d48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:16:33 +0000 Subject: [PATCH 2/2] Preserve resolvable safe-output needs targets Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_engine_env_test.go | 2 ++ pkg/workflow/compiler_main_job.go | 6 ++---- pkg/workflow/compiler_main_job_helpers.go | 1 + pkg/workflow/safe_outputs_config_generation.go | 18 +++++++++++------- .../safe_outputs_config_generation_test.go | 17 +++++++++++++---- pkg/workflow/workflow_data.go | 1 + 6 files changed, 30 insertions(+), 15 deletions(-) diff --git a/pkg/workflow/compiler_jobs_engine_env_test.go b/pkg/workflow/compiler_jobs_engine_env_test.go index 3e16856bf1b..e90b42ed48f 100644 --- a/pkg/workflow/compiler_jobs_engine_env_test.go +++ b/pkg/workflow/compiler_jobs_engine_env_test.go @@ -56,6 +56,8 @@ func TestBuildMainJobEngineEnvNeedsExpression(t *testing.T) { // references its outputs; without this, needs.provide_value_to_agent would be undefined. assert.Contains(t, job.Needs, "provide_value_to_agent", "agent job must directly depend on provide_value_to_agent referenced in engine.env") + assert.Contains(t, workflowData.AgentJobNeeds, "provide_value_to_agent", + "agent dependencies must be available while generating safe-outputs config") assert.Contains(t, job.Needs, string(constants.ActivationJobName), "agent job must also depend on activation") } diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index 3e6e1486ac3..0daf8472675 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -37,8 +37,9 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( compilerMainJobLog.Print("Adding runtime-paths step for safe-outputs") steps = append(steps, c.generateSetRuntimePathsStep()...) } - jobCondition := c.buildMainJobCondition(data, activationJobCreated) + depends, engineEnvContent := c.buildMainJobDependencies(data, activationJobCreated) + c.warnBuiltinJobEnvReferences(depends, engineEnvContent) // Build agent step content (checkout app tokens minted here to avoid masked-value drops). var stepBuilder strings.Builder @@ -49,9 +50,6 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( steps = append(steps, stepsContent) } - depends, engineEnvContent := c.buildMainJobDependencies(data, activationJobCreated) - c.warnBuiltinJobEnvReferences(depends, engineEnvContent) - outputs := c.buildMainJobOutputs(data) env := c.buildMainJobEnv(data) agentConcurrency := GenerateJobConcurrencyConfig(data) diff --git a/pkg/workflow/compiler_main_job_helpers.go b/pkg/workflow/compiler_main_job_helpers.go index de7b257f192..e91d3a99f79 100644 --- a/pkg/workflow/compiler_main_job_helpers.go +++ b/pkg/workflow/compiler_main_job_helpers.go @@ -104,6 +104,7 @@ func (c *Compiler) buildMainJobDependencies(data *WorkflowData, activationJobCre compilerMainJobLog.Printf("Added direct dependency on custom job '%s' because it's referenced in workflow content or engine.env", jobName) } } + data.AgentJobNeeds = depends return depends, engineEnvContent } diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 6208606fb6c..a0a407ab3e6 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "slices" "sort" "strings" @@ -53,13 +54,16 @@ func generateSafeOutputsConfig(data *WorkflowData) (string, error) { if len(safeOutputsConfig) == 0 { return "", nil } - // The agent job never depends on the custom jobs listed in safe-outputs.needs (those - // dependencies are only wired onto the later safe_outputs handler job by - // buildSafeOutputsJobNeeds). Any templated expression referencing needs. for one of - // those jobs would therefore be unresolvable inside the agent job and trip an actionlint - // "undefined property" error. Neutralize such expressions here; the handler job's own copy - // of the config (GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG) is unaffected and keeps the real value. - sanitizeAgentSafeOutputsConfig(safeOutputsConfig, data.SafeOutputs.Needs) + // Custom jobs listed in safe-outputs.needs are wired onto the later safe_outputs handler job, + // not automatically onto the agent job. Neutralize expressions for those jobs unless the + // agent already directly depends on them; the handler job's own config is unaffected. + unresolvableJobs := make([]string, 0, len(data.SafeOutputs.Needs)) + for _, job := range data.SafeOutputs.Needs { + if !slices.Contains(data.AgentJobNeeds, job) { + unresolvableJobs = append(unresolvableJobs, job) + } + } + sanitizeAgentSafeOutputsConfig(safeOutputsConfig, unresolvableJobs) configJSON, err := marshalSafeOutputsConfig(safeOutputsConfig) if err != nil { return "", fmt.Errorf("marshaling safe-outputs config: %w", err) diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index 649dd01db8a..4585c5ee1e2 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -123,17 +123,26 @@ func TestGenerateSafeOutputsConfigNeutralizesAllUnresolvableNeedsForms(t *testin func TestGenerateSafeOutputsConfigPreservesResolvableNeedsExpressions(t *testing.T) { data := &WorkflowData{ + AgentJobNeeds: []string{"setup"}, SafeOutputs: &SafeOutputsConfig{ - Needs: []string{"approval_allowlist"}, - AddComments: &AddCommentsConfig{ - AllowedCommentIDs: []string{"${{ needs.prepare.outputs.comment_ids }}"}, + Needs: []string{"setup", "approval_allowlist"}, + UpdatePullRequests: &UpdatePullRequestsConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + SafeOutputTargetConfig: SafeOutputTargetConfig{ + Target: "${{ needs.setup.outputs.pull_request_number }}", + }, + }, + }, + ApproveWorkflowRun: &ApproveWorkflowRunConfig{ + AllowedPullRequests: []string{"${{ needs.approval_allowlist.outputs.eligible_pull_request_numbers }}"}, }, }, } result, err := generateSafeOutputsConfig(data) require.NoError(t, err) - assert.Contains(t, result, "needs.prepare.outputs.comment_ids") + assert.Contains(t, result, "needs.setup.outputs.pull_request_number") + assert.NotContains(t, result, "needs.approval_allowlist") } func TestSanitizeAgentSafeOutputsConfigNoOp(t *testing.T) { diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index 2332ecaeebe..341f57bd8c7 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -137,6 +137,7 @@ type WorkflowData struct { TopLevelGitHubApp *GitHubAppConfig // top-level github-app fallback for all nested github-app token minting operations LockForAgent bool // whether to lock the issue during agent workflow execution Jobs map[string]any // custom job configurations with dependencies + AgentJobNeeds []string // resolved direct dependencies of the agent job Cache string // cache configuration NeedsTextOutput bool // whether the workflow uses ${{ needs.task.outputs.text }} NetworkPermissions *NetworkPermissions // parsed network permissions