Support extending safe_outputs dependencies via safe-outputs.needs#27476
Merged
Support extending safe_outputs dependencies via safe-outputs.needs#27476
safe_outputs dependencies via safe-outputs.needs#27476Conversation
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>
Copilot
AI
changed the title
[WIP] Allow extending safe_outputs.needs from frontmatter for custom jobs
Support extending Apr 21, 2026
safe_outputs dependencies via safe-outputs.needs and safe-outputs.extra-needs
Collaborator
|
@copilot remove extra-needs |
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>
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>
Contributor
Author
Done — Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Support extending
Support extending Apr 21, 2026
safe_outputs dependencies via safe-outputs.needs and safe-outputs.extra-needssafe_outputs dependencies via safe-outputs.needs
Collaborator
|
@copilot make sure it works with imports |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (314 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new safe-outputs.needs configuration surface so the consolidated safe_outputs job can explicitly depend on custom credential-supplying jobs, enabling valid needs.<job>.outputs.* references in safe-output handlers and avoiding actionlint errors.
Changes:
- Introduces
safe-outputs.needsin the schema andSafeOutputsConfig, plus frontmatter extraction and import-merge behavior. - Extends consolidated
safe_outputsjob dependency construction to include (deduped) user-declared dependencies. - Adds validation (with tests) to ensure
safe-outputs.needstargets are valid custom workflow jobs and not reserved/internal jobs; updates docs with examples.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_needs_validation.go | New validator for safe-outputs.needs targets (custom jobs only, reserved targets rejected). |
| pkg/workflow/safe_outputs_config.go | Extracts safe-outputs.needs from frontmatter into SafeOutputsConfig. |
| pkg/workflow/safe_jobs_needs_validation_test.go | Adds unit tests covering safe-outputs.needs validation behavior. |
| pkg/workflow/imports.go | Merges imported safe-outputs.needs into the main config via deduped union. |
| pkg/workflow/compiler_types.go | Adds Needs []string to SafeOutputsConfig. |
| pkg/workflow/compiler_safe_outputs_job.go | Appends configured safe-outputs.needs into consolidated job dependencies with dedupe + logging. |
| pkg/workflow/compiler_safe_outputs_job_test.go | Adds tests ensuring compiled YAML preserves custom needs.*.outputs.* references and dependency wiring. |
| pkg/workflow/compiler.go | Hooks the new safe-outputs.needs validation into the compiler validation pipeline. |
| pkg/parser/schemas/main_workflow_schema.json | Adds schema support for safe-outputs.needs (array of job IDs, unique, default []). |
| docs/src/content/docs/reference/safe-outputs.md | Documents safe-outputs.needs, constraints, and a credential-fetcher example. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 2
| string(constants.SafeOutputsJobName), | ||
| "safe-outputs", | ||
| string(constants.DetectionJobName), | ||
| string(constants.UnlockJobName), |
There was a problem hiding this comment.
The reserved-target list is missing some internal/generated job IDs (e.g., upload_assets / constants.UploadAssetsJobName and upload_code_scanning_sarif / constants.UploadCodeScanningJobName). Without reserving them here, a workflow can declare safe-outputs.needs for a user job named upload_assets, which can later collide with the compiler-generated upload_assets job when upload-assets is configured, producing a confusing duplicate-job failure. Consider adding the missing internal job IDs (preferably via constants) to isReservedSafeOutputsNeedsTarget.
Suggested change
| string(constants.UnlockJobName), | |
| string(constants.UnlockJobName), | |
| string(constants.UploadAssetsJobName), | |
| string(constants.UploadCodeScanningJobName), |
Comment on lines
+39
to
+44
| 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, | ||
| ) |
There was a problem hiding this comment.
The error example string Example: safe-outputs.needs: [secrets_fetcher] isn’t valid YAML (it reads like a dotted path rather than a config snippet). To make the error more actionable, consider showing a minimal valid YAML example (e.g., safe-outputs:\n needs: [secrets_fetcher]) or otherwise labeling it as a config path rather than an example snippet.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification DetailsView all test classifications (3 tests)
Flagged Items
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
|
@copilot add integration workflow tests in pkg/cli/workfkows |
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>
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>
Contributor
Author
Handled in c51d8ac (plus assertion polish in ccf4658): imports now correctly merge Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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>
Contributor
Author
Added in 2883a46: new integration workflow fixtures under Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
github-actions Bot
added a commit
that referenced
this pull request
Apr 21, 2026
- Update Engine entry to include OpenCode as a 6th experimental engine (PR #27466) - Add Merge Pull Request (merge-pull-request:) safe output entry (PR #27193) - Add Safe Outputs Dependencies (safe-outputs.needs:) entry (PR #27476) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
safe_outputshad a hardcoded dependency set, so workflows could not declare custom credential-supply jobs as upstream dependencies. This blocked validneeds.<custom-job>.outputs.*references insafe-outputshandlers (notablygithub-app) and causedactionlintfailures.Frontmatter schema + config surface
safe-outputs.needsto schema (array<string>, job-id pattern,uniqueItems, default[]).SafeOutputsConfigand extraction logic from frontmatter.safe_outputsjob dependency wiringsafe_outputsjob construction to merge:agent, optionaldetection,activation, optionalunlock)safe-outputs.needsValidation for dependency targets
safe-outputs.needs:agent,activation,pre_activation/pre-activation,conclusion,safe_outputs,detection,unlock,push_repo_memory,update_cache_memory)Import merge behavior
needsin safe-outputs import merge logic (deduped union).needsis treated as safe-outputs meta-configuration (not a handler type), ensuring additive merge behavior with imports.Docs + examples
needsin safe-outputs reference with the credential-fetcher pattern and constraints.safe-outputs.needsis the single explicit dependency field.Regression coverage
needsremains excluded from safe-output type keys used for import conflict detection.safe-outputs.needsare merged/deduplicated and propagated into compiledsafe_outputsjob dependencies.pkg/cli/workflowsplus an integration compile test to verify import-basedsafe-outputs.needsmerging and deduplication in generated lock files.> [!WARNING]
>