From 7a4d4fb2409e11866b57586fff4b7a889c095f78 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:31:50 +0000 Subject: [PATCH 1/8] Initial plan From ad32970c72faf8bf8886bac697214fd6fd4e8c19 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:49:23 +0000 Subject: [PATCH 2/8] feat: support safe-outputs needs extensions for custom jobs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/db7d1958-b3a5-4b6e-8bce-ded0fccd4fd8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../content/docs/reference/safe-outputs.md | 31 ++++++ pkg/parser/schemas/main_workflow_schema.json | 24 ++++ pkg/workflow/compiler.go | 6 + pkg/workflow/compiler_safe_outputs_job.go | 22 ++++ .../compiler_safe_outputs_job_test.go | 105 ++++++++++++++++++ pkg/workflow/compiler_types.go | 2 + pkg/workflow/imports.go | 6 + .../safe_jobs_needs_validation_test.go | 85 ++++++++++++++ pkg/workflow/safe_outputs_config.go | 28 +++++ pkg/workflow/safe_outputs_needs_validation.go | 80 +++++++++++++ 10 files changed, 389 insertions(+) create mode 100644 pkg/workflow/safe_outputs_needs_validation.go diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index cfe428926d0..0d7f7e86b9f 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -1235,6 +1235,37 @@ safe-outputs: Accepts a plain string or an object with `name` and optional `url`, consistent with the top-level `environment:` syntax. +### Safe Outputs Dependencies (`needs:`, `extra-needs:`) + +Extend the consolidated `safe_outputs` job dependencies with custom workflow jobs (for example, credential fetchers). Both fields are merged with built-in dependencies (`agent`, `activation`, optional `detection`, optional `unlock`) and deduplicated. + +```yaml wrap +jobs: + secrets_fetcher: + runs-on: ubuntu-latest + outputs: + app_id: ${{ steps.fetch.outputs.app_id }} + app_private_key: ${{ steps.fetch.outputs.app_private_key }} + steps: + - id: fetch + run: | + echo "app_id=123" >> "$GITHUB_OUTPUT" + echo "app_private_key=***" >> "$GITHUB_OUTPUT" + +safe-outputs: + needs: [secrets_fetcher] + extra-needs: [secrets_fetcher] + github-app: + app-id: ${{ needs.secrets_fetcher.outputs.app_id }} + private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }} +``` + +Validation rules: + +- Values must reference workflow custom jobs from top-level `jobs:` +- Built-in jobs are rejected (`agent`, `activation`, `pre_activation`/`pre-activation`, `conclusion`, `safe_outputs`, `detection`, `unlock`, `push_repo_memory`, `update_cache_memory`) +- Unknown jobs fail compilation with an actionable error + ### Text Sanitization (`allowed-domains:`, `allowed-github-references:`) The text output by AI agents is automatically sanitized to prevent injection of malicious content and ensure safe rendering on GitHub. The auto-sanitization applied is: XML escaped, HTTPS only, domain allowlist (GitHub by default), 0.5MB/65k line limits, control char stripping. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 60717883952..34d08e17492 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -8473,6 +8473,30 @@ "description": "Concurrency group for the safe-outputs job. When set, the safe-outputs job will use this concurrency group with cancel-in-progress: false. Supports GitHub Actions expressions.", "examples": ["my-workflow-safe-outputs", "safe-outputs-${{ github.repository }}"] }, + "needs": { + "type": "array", + "description": "Explicit additional custom workflow jobs that the consolidated safe_outputs job should depend on.", + "items": { + "type": "string", + "pattern": "^[a-zA-Z_][a-zA-Z0-9_-]*$" + }, + "additionalItems": false, + "uniqueItems": true, + "default": [], + "examples": [["secrets_fetcher"]] + }, + "extra-needs": { + "type": "array", + "description": "Additional custom workflow jobs merged into safe_outputs dependencies. Merged with safe-outputs.needs and built-in dependencies.", + "items": { + "type": "string", + "pattern": "^[a-zA-Z_][a-zA-Z0-9_-]*$" + }, + "additionalItems": false, + "uniqueItems": true, + "default": [], + "examples": [["secrets_fetcher"]] + }, "environment": { "description": "Override the GitHub deployment environment for the safe-outputs job. When set, this environment is used instead of the top-level environment: field. When not set, the top-level environment: field is propagated automatically so that environment-scoped secrets are accessible in the safe-outputs job.", "oneOf": [ diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index a092f50542d..4c8f5b4c9d7 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -180,6 +180,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath return formatCompilerError(markdownPath, "error", err.Error(), err) } + // Validate safe-outputs needs declarations + log.Printf("Validating safe-outputs needs declarations") + if err := validateSafeOutputsNeeds(workflowData); err != nil { + return formatCompilerError(markdownPath, "error", err.Error(), err) + } + // Validate safe-job needs: declarations against known generated job IDs log.Printf("Validating safe-job needs declarations") if err := validateSafeJobNeeds(workflowData); err != nil { diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index a35b0b90f1a..fc11b3252b5 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -480,6 +480,28 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa needs = append(needs, "unlock") consolidatedSafeOutputsJobLog.Print("Added unlock job dependency to safe_outputs job") } + seenNeeds := make(map[string]bool, len(needs)) + for _, need := range needs { + seenNeeds[need] = true + } + if data.SafeOutputs != nil { + for _, need := range data.SafeOutputs.Needs { + if seenNeeds[need] { + continue + } + needs = append(needs, need) + seenNeeds[need] = true + consolidatedSafeOutputsJobLog.Printf("Added explicit safe-outputs needs dependency to safe_outputs job: %s", need) + } + for _, need := range data.SafeOutputs.ExtraNeeds { + if seenNeeds[need] { + continue + } + needs = append(needs, need) + seenNeeds[need] = true + consolidatedSafeOutputsJobLog.Printf("Added extra safe-outputs needs dependency to safe_outputs job: %s", need) + } + } // Extract workflow ID from markdown path for GH_AW_WORKFLOW_ID workflowID := GetWorkflowIDFromPath(markdownPath) diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index e1c19b2fbb3..50813972f54 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -3,11 +3,15 @@ package workflow import ( + "os" "path" + "path/filepath" "strings" "testing" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/stringutil" + "github.com/goccy/go-yaml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -215,6 +219,107 @@ func TestBuildConsolidatedSafeOutputsJobConcurrencyGroup(t *testing.T) { } } +func TestBuildConsolidatedSafeOutputsJobNeedsIncludesConfiguredDependencies(t *testing.T) { + compiler := NewCompiler() + compiler.jobManager = NewJobManager() + + workflowData := &WorkflowData{ + Name: "Test Workflow", + LockForAgent: true, + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + TitlePrefix: "[Test] ", + }, + ThreatDetection: &ThreatDetectionConfig{}, + Needs: []string{"secrets_fetcher"}, + ExtraNeeds: []string{"secrets_fetcher", "vault_bootstrap"}, + }, + } + + job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(constants.AgentJobName), "test-workflow.md") + require.NoError(t, err, "Should build job without error") + require.NotNil(t, job, "Job should not be nil") + + assert.Contains(t, job.Needs, string(constants.AgentJobName), "safe_outputs should depend on agent job") + assert.Contains(t, job.Needs, string(constants.ActivationJobName), "safe_outputs should depend on activation job") + assert.Contains(t, job.Needs, string(constants.DetectionJobName), "safe_outputs should depend on detection job when enabled") + assert.Contains(t, job.Needs, "unlock", "safe_outputs should depend on unlock when lock-for-agent is enabled") + assert.Contains(t, job.Needs, "secrets_fetcher", "safe_outputs should include explicit custom dependency") + assert.Contains(t, job.Needs, "vault_bootstrap", "safe_outputs should include extra custom dependency") + + secretsFetcherCount := 0 + for _, need := range job.Needs { + if need == "secrets_fetcher" { + secretsFetcherCount++ + } + } + assert.Equal(t, 1, secretsFetcherCount, "duplicate configured dependencies should be deduplicated") +} + +func TestCompileSafeOutputsNeedsForCustomCredentialJob(t *testing.T) { + tempDir := t.TempDir() + workflowPath := filepath.Join(tempDir, "safe-needs.md") + workflowContent := `--- +on: + workflow_dispatch: {} +permissions: + contents: read +safe-outputs: + needs: [secrets_fetcher] + extra-needs: [secrets_fetcher] + github-app: + app-id: ${{ needs.secrets_fetcher.outputs.app_id }} + private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }} + noop: + report-as-issue: false +jobs: + secrets_fetcher: + runs-on: ubuntu-latest + outputs: + app_id: ${{ steps.fetch.outputs.app_id }} + app_private_key: ${{ steps.fetch.outputs.app_private_key }} + steps: + - id: fetch + run: | + echo "app_id=placeholder" >> "$GITHUB_OUTPUT" + echo "app_private_key=placeholder" >> "$GITHUB_OUTPUT" +--- + +# Safe outputs needs test +` + require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644), "workflow should be written") + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowPath), "workflow should compile") + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + lockContent, err := os.ReadFile(lockPath) + require.NoError(t, err, "compiled lock file should exist") + + var workflow map[string]any + require.NoError(t, yaml.Unmarshal(lockContent, &workflow), "lock YAML should parse") + + jobs, ok := workflow["jobs"].(map[string]any) + require.True(t, ok, "jobs should be present") + safeOutputsJob, ok := jobs["safe_outputs"].(map[string]any) + require.True(t, ok, "safe_outputs job should be present") + + needs, ok := safeOutputsJob["needs"].([]any) + require.True(t, ok, "safe_outputs job should include needs list") + + needsValues := make([]string, 0, len(needs)) + for _, need := range needs { + if needStr, ok := need.(string); ok { + needsValues = append(needsValues, needStr) + } + } + + assert.Contains(t, needsValues, string(constants.AgentJobName), "safe_outputs should keep built-in dependency on agent") + assert.Contains(t, needsValues, string(constants.ActivationJobName), "safe_outputs should keep built-in dependency on activation") + assert.Contains(t, needsValues, "secrets_fetcher", "safe_outputs should include custom credential supplier job") + assert.Contains(t, string(lockContent), "needs.secrets_fetcher.outputs.app_id", "compiled workflow should retain custom needs output references") +} + func TestBuildJobLevelSafeOutputEnvVars(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 9903ce26be0..fcd5698ccf6 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -606,6 +606,8 @@ type SafeOutputsConfig struct { Steps []any `yaml:"steps,omitempty"` // User-provided steps injected after setup/checkout and before safe-output code IDToken *string `yaml:"id-token,omitempty"` // Override id-token permission: "write" to force-add, "none" to disable auto-detection ConcurrencyGroup string `yaml:"concurrency-group,omitempty"` // Concurrency group for the safe-outputs job (cancel-in-progress is always false) + Needs []string `yaml:"needs,omitempty"` // Additional custom workflow jobs that safe_outputs should depend on + ExtraNeeds []string `yaml:"extra-needs,omitempty"` // Additional custom workflow jobs merged into safe_outputs needs (alias for explicit extension) Environment string `yaml:"environment,omitempty"` // Override the GitHub deployment environment for the safe-outputs job (defaults to the top-level environment: field) Actions map[string]*SafeOutputActionConfig `yaml:"actions,omitempty"` // Custom GitHub Actions mounted as safe output tools (resolved at compile time) AutoInjectedCreateIssue bool `yaml:"-"` // Internal: true when create-issues was automatically injected by the compiler (not user-configured) diff --git a/pkg/workflow/imports.go b/pkg/workflow/imports.go index 7ec19cccd47..f2f7274b9e2 100644 --- a/pkg/workflow/imports.go +++ b/pkg/workflow/imports.go @@ -611,6 +611,12 @@ func mergeSafeOutputConfig(result *SafeOutputsConfig, config map[string]any, c * if result.RunsOn == "" && importedConfig.RunsOn != "" { result.RunsOn = importedConfig.RunsOn } + if len(importedConfig.Needs) > 0 { + result.Needs = mergeUnique(result.Needs, importedConfig.Needs...) + } + if len(importedConfig.ExtraNeeds) > 0 { + result.ExtraNeeds = mergeUnique(result.ExtraNeeds, importedConfig.ExtraNeeds...) + } // Merge Messages configuration at field level (main workflow entries override imported entries) if importedConfig.Messages != nil { diff --git a/pkg/workflow/safe_jobs_needs_validation_test.go b/pkg/workflow/safe_jobs_needs_validation_test.go index 7024a595b80..555254762d3 100644 --- a/pkg/workflow/safe_jobs_needs_validation_test.go +++ b/pkg/workflow/safe_jobs_needs_validation_test.go @@ -299,6 +299,91 @@ func TestValidateSafeJobNeeds_NeedsNormalization(t *testing.T) { "needs entries should be normalized to underscore form") } +func TestValidateSafeOutputsNeeds(t *testing.T) { + tests := []struct { + name string + data *WorkflowData + wantErr bool + errContains string + }{ + { + name: "valid custom job targets in needs and extra-needs", + data: &WorkflowData{ + Jobs: map[string]any{ + "secrets_fetcher": map[string]any{ + "runs-on": "ubuntu-latest", + "steps": []any{map[string]any{"run": "echo ok"}}, + }, + }, + SafeOutputs: &SafeOutputsConfig{ + Needs: []string{"secrets_fetcher"}, + ExtraNeeds: []string{"secrets_fetcher"}, + }, + }, + }, + { + name: "unknown job in extra-needs should fail", + data: &WorkflowData{ + Jobs: map[string]any{ + "secrets_fetcher": map[string]any{}, + }, + SafeOutputs: &SafeOutputsConfig{ + ExtraNeeds: []string{"missing_job"}, + }, + }, + wantErr: true, + errContains: `safe-outputs.extra-needs: unknown job "missing_job"`, + }, + { + name: "built-in job in needs should fail", + data: &WorkflowData{ + Jobs: map[string]any{ + "secrets_fetcher": map[string]any{}, + }, + SafeOutputs: &SafeOutputsConfig{ + Needs: []string{"agent"}, + }, + }, + wantErr: true, + errContains: `safe-outputs.needs: built-in job "agent" is not allowed`, + }, + { + name: "self reference safe_outputs should fail", + data: &WorkflowData{ + Jobs: map[string]any{ + "secrets_fetcher": map[string]any{}, + }, + SafeOutputs: &SafeOutputsConfig{ + ExtraNeeds: []string{"safe_outputs"}, + }, + }, + wantErr: true, + errContains: `safe-outputs.extra-needs: built-in job "safe_outputs" is not allowed`, + }, + { + name: "missing fields keeps behavior unchanged", + data: &WorkflowData{ + Jobs: map[string]any{ + "secrets_fetcher": map[string]any{}, + }, + SafeOutputs: &SafeOutputsConfig{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSafeOutputsNeeds(tt.data) + if tt.wantErr { + require.Error(t, err, "expected validation error") + assert.Contains(t, err.Error(), tt.errContains, "error should include expected context") + return + } + require.NoError(t, err, "expected validation to pass") + }) + } +} + // TestDetectSafeJobCycles tests cycle detection between custom safe-jobs. func TestDetectSafeJobCycles(t *testing.T) { t.Run("no cycles – linear chain", func(t *testing.T) { diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index 1ac733fb14f..c42c806251c 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -550,6 +550,34 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } + // Handle needs configuration + if needsValue, exists := outputMap["needs"]; exists { + if needsArray, ok := needsValue.([]any); ok { + for _, need := range needsArray { + if needStr, ok := need.(string); ok && needStr != "" { + config.Needs = append(config.Needs, needStr) + } + } + if len(config.Needs) > 0 { + safeOutputsConfigLog.Printf("Configured %d explicit safe-outputs needs dependency(ies)", len(config.Needs)) + } + } + } + + // Handle extra-needs configuration + if extraNeedsValue, exists := outputMap["extra-needs"]; exists { + if extraNeedsArray, ok := extraNeedsValue.([]any); ok { + for _, need := range extraNeedsArray { + if needStr, ok := need.(string); ok && needStr != "" { + config.ExtraNeeds = append(config.ExtraNeeds, needStr) + } + } + if len(config.ExtraNeeds) > 0 { + safeOutputsConfigLog.Printf("Configured %d extra safe-outputs needs dependency(ies)", len(config.ExtraNeeds)) + } + } + } + // Handle environment configuration (override for safe-outputs job; falls back to top-level environment) config.Environment = c.extractTopLevelYAMLSection(outputMap, "environment") if config.Environment != "" { diff --git a/pkg/workflow/safe_outputs_needs_validation.go b/pkg/workflow/safe_outputs_needs_validation.go new file mode 100644 index 00000000000..1e7985ef8af --- /dev/null +++ b/pkg/workflow/safe_outputs_needs_validation.go @@ -0,0 +1,80 @@ +package workflow + +import ( + "fmt" + + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var safeOutputsNeedsValidationLog = logger.New("workflow:safe_outputs_needs_validation") + +func validateSafeOutputsNeeds(data *WorkflowData) error { + if data == nil || data.SafeOutputs == nil { + return nil + } + + if err := validateSafeOutputsNeedsField(data, "needs", data.SafeOutputs.Needs); err != nil { + return err + } + if err := validateSafeOutputsNeedsField(data, "extra-needs", data.SafeOutputs.ExtraNeeds); err != nil { + return err + } + + return nil +} + +func validateSafeOutputsNeedsField(data *WorkflowData, fieldName string, needs []string) error { + if len(needs) == 0 { + return nil + } + + customJobs := make(map[string]bool, len(data.Jobs)) + for jobName := range data.Jobs { + if isReservedSafeOutputsNeedsTarget(jobName) { + continue + } + customJobs[jobName] = true + } + + for _, need := range needs { + if isReservedSafeOutputsNeedsTarget(need) { + return fmt.Errorf( + "safe-outputs.%s: built-in job %q is not allowed. Expected one of the workflow's custom jobs. Example: safe-outputs.%s: [secrets_fetcher]", + fieldName, + need, + fieldName, + ) + } + if !customJobs[need] { + return fmt.Errorf( + "safe-outputs.%s: unknown job %q. Expected one of the workflow's custom jobs. Example: safe-outputs.%s: [secrets_fetcher]", + fieldName, + need, + fieldName, + ) + } + } + + safeOutputsNeedsValidationLog.Printf("Validated %d safe-outputs.%s dependency target(s)", len(needs), fieldName) + return nil +} + +func isReservedSafeOutputsNeedsTarget(jobName string) bool { + switch jobName { + case string(constants.AgentJobName), + string(constants.ActivationJobName), + string(constants.PreActivationJobName), + "pre-activation", + string(constants.ConclusionJobName), + string(constants.SafeOutputsJobName), + "safe-outputs", + string(constants.DetectionJobName), + string(constants.UnlockJobName), + "push_repo_memory", + "update_cache_memory": + return true + default: + return false + } +} From fecb5efafa8e65b6381f76a71b7a292a3404e646 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:03:03 +0000 Subject: [PATCH 3/8] refactor: remove safe-outputs extra-needs and keep needs only Agent-Logs-Url: https://github.com/github/gh-aw/sessions/105c3953-c3ab-4fd0-b368-3082997396fb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/reference/safe-outputs.md | 5 ++--- pkg/parser/schemas/main_workflow_schema.json | 12 ------------ pkg/workflow/compiler_safe_outputs_job.go | 8 -------- pkg/workflow/compiler_safe_outputs_job_test.go | 4 +--- pkg/workflow/compiler_types.go | 1 - pkg/workflow/imports.go | 3 --- pkg/workflow/safe_jobs_needs_validation_test.go | 15 +++++++-------- pkg/workflow/safe_outputs_config.go | 14 -------------- pkg/workflow/safe_outputs_needs_validation.go | 3 --- 9 files changed, 10 insertions(+), 55 deletions(-) diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index 0d7f7e86b9f..6fe0b926fc8 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -1235,9 +1235,9 @@ safe-outputs: Accepts a plain string or an object with `name` and optional `url`, consistent with the top-level `environment:` syntax. -### Safe Outputs Dependencies (`needs:`, `extra-needs:`) +### Safe Outputs Dependencies (`needs:`) -Extend the consolidated `safe_outputs` job dependencies with custom workflow jobs (for example, credential fetchers). Both fields are merged with built-in dependencies (`agent`, `activation`, optional `detection`, optional `unlock`) and deduplicated. +Extend the consolidated `safe_outputs` job dependencies with custom workflow jobs (for example, credential fetchers). `safe-outputs.needs` is merged with built-in dependencies (`agent`, `activation`, optional `detection`, optional `unlock`) and deduplicated. ```yaml wrap jobs: @@ -1254,7 +1254,6 @@ jobs: safe-outputs: needs: [secrets_fetcher] - extra-needs: [secrets_fetcher] github-app: app-id: ${{ needs.secrets_fetcher.outputs.app_id }} private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }} diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 34d08e17492..0006fa521ab 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -8485,18 +8485,6 @@ "default": [], "examples": [["secrets_fetcher"]] }, - "extra-needs": { - "type": "array", - "description": "Additional custom workflow jobs merged into safe_outputs dependencies. Merged with safe-outputs.needs and built-in dependencies.", - "items": { - "type": "string", - "pattern": "^[a-zA-Z_][a-zA-Z0-9_-]*$" - }, - "additionalItems": false, - "uniqueItems": true, - "default": [], - "examples": [["secrets_fetcher"]] - }, "environment": { "description": "Override the GitHub deployment environment for the safe-outputs job. When set, this environment is used instead of the top-level environment: field. When not set, the top-level environment: field is propagated automatically so that environment-scoped secrets are accessible in the safe-outputs job.", "oneOf": [ diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index fc11b3252b5..38a70a1332f 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -493,14 +493,6 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa seenNeeds[need] = true consolidatedSafeOutputsJobLog.Printf("Added explicit safe-outputs needs dependency to safe_outputs job: %s", need) } - for _, need := range data.SafeOutputs.ExtraNeeds { - if seenNeeds[need] { - continue - } - needs = append(needs, need) - seenNeeds[need] = true - consolidatedSafeOutputsJobLog.Printf("Added extra safe-outputs needs dependency to safe_outputs job: %s", need) - } } // Extract workflow ID from markdown path for GH_AW_WORKFLOW_ID diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index 50813972f54..be6d95ca571 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -231,8 +231,7 @@ func TestBuildConsolidatedSafeOutputsJobNeedsIncludesConfiguredDependencies(t *t TitlePrefix: "[Test] ", }, ThreatDetection: &ThreatDetectionConfig{}, - Needs: []string{"secrets_fetcher"}, - ExtraNeeds: []string{"secrets_fetcher", "vault_bootstrap"}, + Needs: []string{"secrets_fetcher", "vault_bootstrap"}, }, } @@ -266,7 +265,6 @@ permissions: contents: read safe-outputs: needs: [secrets_fetcher] - extra-needs: [secrets_fetcher] github-app: app-id: ${{ needs.secrets_fetcher.outputs.app_id }} private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }} diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index fcd5698ccf6..4167b4bf284 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -607,7 +607,6 @@ type SafeOutputsConfig struct { IDToken *string `yaml:"id-token,omitempty"` // Override id-token permission: "write" to force-add, "none" to disable auto-detection ConcurrencyGroup string `yaml:"concurrency-group,omitempty"` // Concurrency group for the safe-outputs job (cancel-in-progress is always false) Needs []string `yaml:"needs,omitempty"` // Additional custom workflow jobs that safe_outputs should depend on - ExtraNeeds []string `yaml:"extra-needs,omitempty"` // Additional custom workflow jobs merged into safe_outputs needs (alias for explicit extension) Environment string `yaml:"environment,omitempty"` // Override the GitHub deployment environment for the safe-outputs job (defaults to the top-level environment: field) Actions map[string]*SafeOutputActionConfig `yaml:"actions,omitempty"` // Custom GitHub Actions mounted as safe output tools (resolved at compile time) AutoInjectedCreateIssue bool `yaml:"-"` // Internal: true when create-issues was automatically injected by the compiler (not user-configured) diff --git a/pkg/workflow/imports.go b/pkg/workflow/imports.go index f2f7274b9e2..5220177f92c 100644 --- a/pkg/workflow/imports.go +++ b/pkg/workflow/imports.go @@ -614,9 +614,6 @@ func mergeSafeOutputConfig(result *SafeOutputsConfig, config map[string]any, c * if len(importedConfig.Needs) > 0 { result.Needs = mergeUnique(result.Needs, importedConfig.Needs...) } - if len(importedConfig.ExtraNeeds) > 0 { - result.ExtraNeeds = mergeUnique(result.ExtraNeeds, importedConfig.ExtraNeeds...) - } // Merge Messages configuration at field level (main workflow entries override imported entries) if importedConfig.Messages != nil { diff --git a/pkg/workflow/safe_jobs_needs_validation_test.go b/pkg/workflow/safe_jobs_needs_validation_test.go index 555254762d3..7a96bb4b06d 100644 --- a/pkg/workflow/safe_jobs_needs_validation_test.go +++ b/pkg/workflow/safe_jobs_needs_validation_test.go @@ -307,7 +307,7 @@ func TestValidateSafeOutputsNeeds(t *testing.T) { errContains string }{ { - name: "valid custom job targets in needs and extra-needs", + name: "valid custom job targets in needs", data: &WorkflowData{ Jobs: map[string]any{ "secrets_fetcher": map[string]any{ @@ -316,23 +316,22 @@ func TestValidateSafeOutputsNeeds(t *testing.T) { }, }, SafeOutputs: &SafeOutputsConfig{ - Needs: []string{"secrets_fetcher"}, - ExtraNeeds: []string{"secrets_fetcher"}, + Needs: []string{"secrets_fetcher"}, }, }, }, { - name: "unknown job in extra-needs should fail", + name: "unknown job in needs should fail", data: &WorkflowData{ Jobs: map[string]any{ "secrets_fetcher": map[string]any{}, }, SafeOutputs: &SafeOutputsConfig{ - ExtraNeeds: []string{"missing_job"}, + Needs: []string{"missing_job"}, }, }, wantErr: true, - errContains: `safe-outputs.extra-needs: unknown job "missing_job"`, + errContains: `safe-outputs.needs: unknown job "missing_job"`, }, { name: "built-in job in needs should fail", @@ -354,11 +353,11 @@ func TestValidateSafeOutputsNeeds(t *testing.T) { "secrets_fetcher": map[string]any{}, }, SafeOutputs: &SafeOutputsConfig{ - ExtraNeeds: []string{"safe_outputs"}, + Needs: []string{"safe_outputs"}, }, }, wantErr: true, - errContains: `safe-outputs.extra-needs: built-in job "safe_outputs" is not allowed`, + errContains: `safe-outputs.needs: built-in job "safe_outputs" is not allowed`, }, { name: "missing fields keeps behavior unchanged", diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index c42c806251c..ff8688ed98b 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -564,20 +564,6 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } - // Handle extra-needs configuration - if extraNeedsValue, exists := outputMap["extra-needs"]; exists { - if extraNeedsArray, ok := extraNeedsValue.([]any); ok { - for _, need := range extraNeedsArray { - if needStr, ok := need.(string); ok && needStr != "" { - config.ExtraNeeds = append(config.ExtraNeeds, needStr) - } - } - if len(config.ExtraNeeds) > 0 { - safeOutputsConfigLog.Printf("Configured %d extra safe-outputs needs dependency(ies)", len(config.ExtraNeeds)) - } - } - } - // Handle environment configuration (override for safe-outputs job; falls back to top-level environment) config.Environment = c.extractTopLevelYAMLSection(outputMap, "environment") if config.Environment != "" { diff --git a/pkg/workflow/safe_outputs_needs_validation.go b/pkg/workflow/safe_outputs_needs_validation.go index 1e7985ef8af..5280779c41b 100644 --- a/pkg/workflow/safe_outputs_needs_validation.go +++ b/pkg/workflow/safe_outputs_needs_validation.go @@ -17,9 +17,6 @@ func validateSafeOutputsNeeds(data *WorkflowData) error { if err := validateSafeOutputsNeedsField(data, "needs", data.SafeOutputs.Needs); err != nil { return err } - if err := validateSafeOutputsNeedsField(data, "extra-needs", data.SafeOutputs.ExtraNeeds); err != nil { - return err - } return nil } From 554bebe3da19e26a913cdf459f99c56a0d64c388 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:03:41 +0000 Subject: [PATCH 4/8] docs: clarify using single safe-outputs needs field Agent-Logs-Url: https://github.com/github/gh-aw/sessions/105c3953-c3ab-4fd0-b368-3082997396fb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/reference/safe-outputs.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index 6fe0b926fc8..71d14b20326 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -1259,6 +1259,8 @@ safe-outputs: private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }} ``` +Use the single `safe-outputs.needs` field for all explicit custom dependencies. + Validation rules: - Values must reference workflow custom jobs from top-level `jobs:` From d08d28202914ac82c07e0d746a9111f2d5ee7f62 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:13:07 +0000 Subject: [PATCH 5/8] docs(adr): add draft ADR-27476 for safe-outputs needs extension Co-Authored-By: Claude Sonnet 4.6 --- ...fe-outputs-dependencies-via-needs-field.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md diff --git a/docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md b/docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md new file mode 100644 index 00000000000..1b994d6f538 --- /dev/null +++ b/docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md @@ -0,0 +1,79 @@ +# ADR-27476: Extend `safe_outputs` Job Dependencies via `safe-outputs.needs` Field + +**Date**: 2026-04-21 +**Status**: Draft +**Deciders**: pelikhan + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The consolidated `safe_outputs` job in gh-aw workflows had a hardcoded dependency set (`agent`, `activation`, optional `detection`, optional `unlock`). Some workflows need credentials supplied by a user-defined job (e.g., a `secrets_fetcher` that retrieves app IDs and private keys from a vault) before the `safe_outputs` job can run. Because the `safe_outputs` dependency list was not extensible, `needs..outputs.*` expressions inside `safe-outputs` handler configs (notably `github-app`) referenced jobs that were not declared as upstream dependencies, causing `actionlint` lint failures and runtime breakage. + +### Decision + +We will add a `safe-outputs.needs` array field to the frontmatter schema that lets workflow authors declare explicit custom upstream jobs for the consolidated `safe_outputs` job. At compile time, the compiler merges these user-declared dependencies with the built-in set and deduplicates them. A validation pass rejects references to unknown jobs and built-in/control job names, providing actionable error messages. + +### Alternatives Considered + +#### Alternative 1: Rely Solely on Environment-Scoped Secrets + +Workflows could store credentials in GitHub environment secrets, accessible to the `safe_outputs` job without any custom upstream job. This avoids the dependency wiring problem entirely. It was not chosen because it requires credentials to be pre-registered in the GitHub environment UI and cannot dynamically fetch or rotate secrets at workflow runtime — a real constraint for vault-backed or short-lived credentials. + +#### Alternative 2: Auto-Detect Dependencies from Expression References + +The compiler could parse expression strings inside `safe-outputs` config values (e.g., `${{ needs.secrets_fetcher.outputs.app_id }}`) and automatically infer the required `needs` entries. This approach was not chosen because parsing arbitrary GitHub Actions expression syntax inside YAML values is fragile, error-prone, and creates a tight coupling between the expression evaluator and the compiler. An explicit declaration (`safe-outputs.needs`) is simpler to validate, easier to understand, and immune to expression parsing edge cases. + +### Consequences + +#### Positive +- Unlocks the credential-fetcher pattern: workflows can now supply dynamic, vault-retrieved credentials to `safe_outputs` handlers such as `github-app`. +- Eliminates `actionlint` failures caused by undeclared `needs.*` references in the compiled GitHub Actions workflow. +- The deduplication logic prevents duplicate dependency entries regardless of how `needs` is populated (built-in vs. user-declared vs. imported). + +#### Negative +- Authors must know which job names are reserved (built-in control jobs) and cannot accidentally use them as custom dependencies; this adds cognitive overhead and requires consulting documentation or error messages. +- Import merge behavior for `safe-outputs.needs` must be maintained in sync with other merge fields as the import system evolves. + +#### Neutral +- A new validation function (`validateSafeOutputsNeeds`) is added to the compiler pipeline, following the same structure as the existing `validateSafeJobNeeds` validator. +- The schema change (`main_workflow_schema.json`) affects all schema-validation tooling and IDE integrations that consume the schema. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Dependency Declaration + +1. Workflow authors **MAY** declare additional upstream job dependencies for the consolidated `safe_outputs` job using the `safe-outputs.needs` array field in workflow frontmatter. +2. Each entry in `safe-outputs.needs` **MUST** be a string matching the pattern `^[a-zA-Z_][a-zA-Z0-9_-]*$`. +3. The `safe-outputs.needs` array **MUST** have `uniqueItems: true` enforced at the schema level; duplicate entries **SHOULD** be deduplicated silently at compile time. + +### Compiler Behavior + +1. The compiler **MUST** merge `safe-outputs.needs` entries with the built-in dependency set (`agent`, `activation`, and optionally `detection` and `unlock`) before writing the compiled `safe_outputs` job's `needs` list. +2. The compiler **MUST** deduplicate the final `needs` list so that no job name appears more than once. +3. The compiler **MUST** validate `safe-outputs.needs` entries before generating the compiled workflow and **MUST** reject invalid entries with an actionable error message. + +### Validation Rules + +1. A `safe-outputs.needs` entry **MUST NOT** reference a built-in or control job name (`agent`, `activation`, `pre_activation`, `pre-activation`, `conclusion`, `safe_outputs`, `safe-outputs`, `detection`, `unlock`, `push_repo_memory`, `update_cache_memory`). +2. A `safe-outputs.needs` entry **MUST NOT** reference a job name that is not declared in the workflow's top-level `jobs:` map. +3. When a violation of rules 1 or 2 is detected, the compiler **MUST** emit a compile-time error and **MUST NOT** produce a compiled workflow artifact. + +### Import Merge Behavior + +1. When a workflow imports another workflow, the importer's `safe-outputs.needs` **MUST** be merged with the imported workflow's `safe-outputs.needs` as a deduplicated union. +2. Import merge **MUST NOT** introduce duplicate entries into the merged `needs` list. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24701927396) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From c51d8accf82a390c6b3d64f69dc0c8d52ba68ba1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:18:41 +0000 Subject: [PATCH 6/8] test: verify safe-outputs needs merge from imports Agent-Logs-Url: https://github.com/github/gh-aw/sessions/116ec68e-66da-403e-b58a-f875af93b4c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schema_compiler.go | 1 + pkg/parser/schema_test.go | 1 + pkg/workflow/safe_outputs_import_test.go | 87 ++++++++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/pkg/parser/schema_compiler.go b/pkg/parser/schema_compiler.go index 0e95577f96d..b273b1934c3 100644 --- a/pkg/parser/schema_compiler.go +++ b/pkg/parser/schema_compiler.go @@ -142,6 +142,7 @@ var safeOutputMetaFields = map[string]bool{ "jobs": true, "runs-on": true, "messages": true, + "needs": true, } // GetSafeOutputTypeKeys returns the list of safe output type keys from the embedded main workflow schema. diff --git a/pkg/parser/schema_test.go b/pkg/parser/schema_test.go index 36d6000d785..965b5817302 100644 --- a/pkg/parser/schema_test.go +++ b/pkg/parser/schema_test.go @@ -436,6 +436,7 @@ func TestGetSafeOutputTypeKeys(t *testing.T) { "jobs", "runs-on", "messages", + "needs", } for _, meta := range metaFields { diff --git a/pkg/workflow/safe_outputs_import_test.go b/pkg/workflow/safe_outputs_import_test.go index 1253b2ad149..bbabccb9330 100644 --- a/pkg/workflow/safe_outputs_import_test.go +++ b/pkg/workflow/safe_outputs_import_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.yaml.in/yaml/v3" ) // TestSafeOutputsImport tests that safe-output types can be imported from shared workflows @@ -2285,3 +2286,89 @@ Run a task. require.NotNil(t, workflowData.SafeOutputs.ReportIncomplete, "ReportIncomplete should be imported from report-incomplete.md") assert.Equal(t, "[report-incomplete import] ", workflowData.SafeOutputs.ReportIncomplete.TitlePrefix) } + +func TestSafeOutputsNeedsMergedFromImports(t *testing.T) { + compiler := NewCompiler(WithVersion("1.0.0")) + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + importedWorkflow := `--- +safe-outputs: + needs: [imported_job, shared_job] +--- +` + importedFile := filepath.Join(workflowsDir, "shared-needs.md") + err = os.WriteFile(importedFile, []byte(importedWorkflow), 0644) + require.NoError(t, err, "Failed to write imported file") + + mainWorkflow := `--- +on: + workflow_dispatch: +imports: + - ./shared-needs.md +safe-outputs: + needs: [main_job, imported_job] +jobs: + main_job: + runs-on: ubuntu-latest + steps: + - run: echo "main" + imported_job: + runs-on: ubuntu-latest + steps: + - run: echo "imported" + shared_job: + runs-on: ubuntu-latest + steps: + - run: echo "shared" +--- +Run a task. +` + mainFile := filepath.Join(workflowsDir, "main.md") + err = os.WriteFile(mainFile, []byte(mainWorkflow), 0644) + require.NoError(t, err, "Failed to write main file") + + oldDir, err := os.Getwd() + require.NoError(t, err, "Failed to get current directory") + err = os.Chdir(workflowsDir) + require.NoError(t, err, "Failed to change directory") + defer func() { _ = os.Chdir(oldDir) }() + + workflowData, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err, "ParseWorkflowFile should not error") + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + assert.ElementsMatch(t, []string{"main_job", "imported_job", "shared_job"}, workflowData.SafeOutputs.Needs, + "safe-outputs.needs should merge main and imported dependencies with deduplication") + + err = compiler.CompileWorkflow("main.md") + require.NoError(t, err, "CompileWorkflow should not error") + + lockPath := filepath.Join(workflowsDir, "main.lock.yml") + lockContent, err := os.ReadFile(lockPath) + require.NoError(t, err, "compiled lock file should exist") + + var compiled map[string]any + err = yaml.Unmarshal(lockContent, &compiled) + require.NoError(t, err, "compiled lock file should be valid YAML") + + jobs, ok := compiled["jobs"].(map[string]any) + require.True(t, ok, "compiled workflow should include jobs") + safeOutputsJob, ok := jobs["safe_outputs"].(map[string]any) + require.True(t, ok, "compiled workflow should include safe_outputs job") + needsRaw, ok := safeOutputsJob["needs"].([]any) + require.True(t, ok, "safe_outputs job should include needs array") + + needs := make([]string, 0, len(needsRaw)) + for _, need := range needsRaw { + needStr, typeOK := need.(string) + require.True(t, typeOK, "safe_outputs needs entries should be strings") + needs = append(needs, needStr) + } + + assert.Contains(t, needs, "main_job", "safe_outputs needs should include dependency from main workflow") + assert.Contains(t, needs, "imported_job", "safe_outputs needs should include deduped dependency from import") + assert.Contains(t, needs, "shared_job", "safe_outputs needs should include dependency from import") +} From ccf465846b97040af15d83cd5d4ae73c0525b975 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:21:41 +0000 Subject: [PATCH 7/8] test: tighten type assertion for merged safe-outputs needs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/116ec68e-66da-403e-b58a-f875af93b4c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/safe_outputs_import_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/safe_outputs_import_test.go b/pkg/workflow/safe_outputs_import_test.go index bbabccb9330..fbe25369f9b 100644 --- a/pkg/workflow/safe_outputs_import_test.go +++ b/pkg/workflow/safe_outputs_import_test.go @@ -2363,8 +2363,8 @@ Run a task. needs := make([]string, 0, len(needsRaw)) for _, need := range needsRaw { - needStr, typeOK := need.(string) - require.True(t, typeOK, "safe_outputs needs entries should be strings") + require.IsType(t, "", need, "safe_outputs needs entries should be strings") + needStr := need.(string) needs = append(needs, needStr) } From 2883a46ce4f6da93ceaf1cc09cc14e3049d43e77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 03:31:39 +0000 Subject: [PATCH 8/8] test: add cli integration fixture for safe-outputs needs imports Agent-Logs-Url: https://github.com/github/gh-aw/sessions/299d3235-457b-42df-afad-89398c4648c9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ..._outputs_needs_imports_integration_test.go | 72 +++++++++++++++++++ .../shared/safe-outputs-needs-import.md | 6 ++ .../test-safe-outputs-needs-imports.md | 27 +++++++ 3 files changed, 105 insertions(+) create mode 100644 pkg/cli/compile_safe_outputs_needs_imports_integration_test.go create mode 100644 pkg/cli/workflows/shared/safe-outputs-needs-import.md create mode 100644 pkg/cli/workflows/test-safe-outputs-needs-imports.md diff --git a/pkg/cli/compile_safe_outputs_needs_imports_integration_test.go b/pkg/cli/compile_safe_outputs_needs_imports_integration_test.go new file mode 100644 index 00000000000..1388902a480 --- /dev/null +++ b/pkg/cli/compile_safe_outputs_needs_imports_integration_test.go @@ -0,0 +1,72 @@ +//go:build integration + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + goyaml "github.com/goccy/go-yaml" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCompileSafeOutputsNeedsMergedWithImports(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + srcMainPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-safe-outputs-needs-imports.md") + srcImportPath := filepath.Join(projectRoot, "pkg/cli/workflows/shared/safe-outputs-needs-import.md") + + dstMainPath := filepath.Join(setup.workflowsDir, "test-safe-outputs-needs-imports.md") + dstImportDir := filepath.Join(setup.workflowsDir, "shared") + dstImportPath := filepath.Join(dstImportDir, "safe-outputs-needs-import.md") + + require.NoError(t, os.MkdirAll(dstImportDir, 0755), "Failed to create shared import directory") + + srcMainContent, err := os.ReadFile(srcMainPath) + require.NoError(t, err, "Failed to read source workflow fixture") + require.NoError(t, os.WriteFile(dstMainPath, srcMainContent, 0644), "Failed to write main workflow fixture") + + srcImportContent, err := os.ReadFile(srcImportPath) + require.NoError(t, err, "Failed to read source imported workflow fixture") + require.NoError(t, os.WriteFile(dstImportPath, srcImportContent, 0644), "Failed to write imported workflow fixture") + + cmd := exec.Command(setup.binaryPath, "compile", dstMainPath) + output, err := cmd.CombinedOutput() + require.NoError(t, err, "Compile failed:\n%s", string(output)) + + lockFilePath := filepath.Join(setup.workflowsDir, "test-safe-outputs-needs-imports.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + require.NoError(t, err, "Failed to read lock file") + + var workflow map[string]any + require.NoError(t, goyaml.Unmarshal(lockContent, &workflow), "Lock file should be valid YAML") + + jobs, ok := workflow["jobs"].(map[string]any) + require.True(t, ok, "Compiled workflow should include jobs map") + safeOutputsJob, ok := jobs["safe_outputs"].(map[string]any) + require.True(t, ok, "Compiled workflow should include safe_outputs job") + needsRaw, ok := safeOutputsJob["needs"].([]any) + require.True(t, ok, "safe_outputs job should include needs array") + + needs := make([]string, 0, len(needsRaw)) + for _, need := range needsRaw { + require.IsType(t, "", need, "safe_outputs needs entries should be strings") + needs = append(needs, need.(string)) + } + + assert.Contains(t, needs, "main_job", "safe_outputs.needs should include top-level custom dependency") + assert.Contains(t, needs, "imported_job", "safe_outputs.needs should include imported custom dependency") + assert.Contains(t, needs, "shared_job", "safe_outputs.needs should include imported custom dependency") + + importedJobCount := 0 + for _, need := range needs { + if need == "imported_job" { + importedJobCount++ + } + } + assert.Equal(t, 1, importedJobCount, "safe_outputs.needs should dedupe duplicate custom dependencies") +} diff --git a/pkg/cli/workflows/shared/safe-outputs-needs-import.md b/pkg/cli/workflows/shared/safe-outputs-needs-import.md new file mode 100644 index 00000000000..585fee14844 --- /dev/null +++ b/pkg/cli/workflows/shared/safe-outputs-needs-import.md @@ -0,0 +1,6 @@ +--- +safe-outputs: + needs: + - shared_job + - imported_job +--- diff --git a/pkg/cli/workflows/test-safe-outputs-needs-imports.md b/pkg/cli/workflows/test-safe-outputs-needs-imports.md new file mode 100644 index 00000000000..6e859a34da5 --- /dev/null +++ b/pkg/cli/workflows/test-safe-outputs-needs-imports.md @@ -0,0 +1,27 @@ +--- +on: + workflow_dispatch: +imports: + - ./shared/safe-outputs-needs-import.md +safe-outputs: + needs: + - main_job + - imported_job +jobs: + main_job: + runs-on: ubuntu-latest + steps: + - run: echo "main" + imported_job: + runs-on: ubuntu-latest + steps: + - run: echo "imported" + shared_job: + runs-on: ubuntu-latest + steps: + - run: echo "shared" +--- + +# Safe outputs needs imports fixture + +Verify that safe-outputs.needs from imports is merged with top-level needs.