Add on.needs to support pre_activation/activation dependencies for credential-supply jobs#27895
Merged
Add on.needs to support pre_activation/activation dependencies for credential-supply jobs#27895
on.needs to support pre_activation/activation dependencies for credential-supply jobs#27895Conversation
8 tasks
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5ab82b98-edea-42b6-9451-35c2763cc00a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…vation Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5ab82b98-edea-42b6-9451-35c2763cc00a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5ab82b98-edea-42b6-9451-35c2763cc00a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Implement on.needs API for pre_activation and activation dependencies
Add Apr 22, 2026
on.needs to support pre_activation/activation dependencies for credential-supply jobs
Copilot stopped work on behalf of
pelikhan due to an error
April 22, 2026 21:24
Closed
…ependencies 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 (545 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. 📋 Draft ADR SummaryDecision captured: Introduce Alternatives documented:
Key normative requirements:
📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24806829131
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds on.needs to let pre_activation and activation depend on custom upstream jobs (e.g., secret fetchers), enabling safe use of ${{ needs.<job>.outputs.* }} in on.github-app credential fields.
Changes:
- Extend workflow schema + frontmatter/workflow data model to include typed
on.needs. - Wire
on.needsinto compiler dependency generation forpre_activation,activation, and custom job auto-dependency behavior. - Add validation + tests for
on.needstargets andon.github-appneeds-expression usage; update reference docs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/on_steps_test.go | Adds unit test coverage for extractOnNeeds. |
| pkg/workflow/on_needs_validation_test.go | Adds tests for on.needs target validation and on.github-app needs-expression validation. |
| pkg/workflow/on_needs_validation.go | Implements validation for on.needs targets + on.github-app needs-expression constraints. |
| pkg/workflow/on_needs_integration_test.go | Integration test ensuring compiled jobs wire on.needs into pre_activation/activation needs. |
| pkg/workflow/frontmatter_types_test.go | Verifies ParseFrontmatterConfig extracts typed on.needs. |
| pkg/workflow/frontmatter_types.go | Adds OnNeeds typed field to frontmatter config model. |
| pkg/workflow/frontmatter_parsing.go | Extracts typed on.needs during frontmatter parsing. |
| pkg/workflow/compiler_types.go | Adds OnNeeds to WorkflowData for compiler consumption. |
| pkg/workflow/compiler_safe_outputs.go | Ensures on.needs isn’t treated as an event trigger key during on-section parsing. |
| pkg/workflow/compiler_pre_activation_job.go | Extracts on.needs, wires it into pre_activation.needs, and adds parsing helpers. |
| pkg/workflow/compiler_orchestrator_workflow.go | Populates WorkflowData.OnNeeds from frontmatter during orchestration. |
| pkg/workflow/compiler_jobs_test.go | Tests that jobs in on.needs do not receive implicit needs: activation. |
| pkg/workflow/compiler_jobs.go | Updates pre-activation creation conditions and custom job auto-dependency logic to respect on.needs. |
| pkg/workflow/compiler_activation_job_builder.go | Adds on.needs jobs to the activation job dependency set. |
| pkg/workflow/compiler.go | Adds validation call for on.needs + related on.github-app expression checks. |
| pkg/parser/schemas/main_workflow_schema.json | Adds on.needs to schema (array of job IDs). |
| docs/src/content/docs/reference/triggers.md | Documents on.needs semantics and credential-supply example. |
| docs/src/content/docs/reference/frontmatter.md | Adds on.needs to the frontmatter on: field listing. |
| docs/src/content/docs/reference/frontmatter-full.md | Adds on.needs to the full frontmatter reference example. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 20/20 changed files
- Comments generated: 2
| } | ||
|
|
||
| fields := map[string]string{ | ||
| "app-id": data.ActivationGitHubApp.AppID, |
There was a problem hiding this comment.
Error messages and field labels refer to on.github-app.app-id, but the YAML key for this field is client-id (see GitHubAppConfig yaml tag). This will surface confusing diagnostics to users. Use client-id in the field map / error messages so they match the documented/frontmatter key names.
Suggested change
| "app-id": data.ActivationGitHubApp.AppID, | |
| "client-id": data.ActivationGitHubApp.AppID, |
Comment on lines
+59
to
+66
| if jobConfig, ok := data.Jobs[need].(map[string]any); ok { | ||
| if jobDependsOnActivation(jobConfig) || jobDependsOnPreActivation(jobConfig) { | ||
| return fmt.Errorf( | ||
| "on.needs: job %q cannot depend on activation/pre_activation because pre_activation and activation depend on on.needs jobs", | ||
| need, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
validateOnNeedsTargets only rejects on.needs jobs that directly depend on activation/pre_activation. A common configuration can still create a cycle via implicit activation dependencies: if an on.needs job has explicit needs: [B] and job B has no explicit needs and is not in on.needs/prompt-referenced, the compiler will auto-add needs: activation to B, producing activation → A → B → activation. Consider validating the entire dependency chain for each on.needs target (or, minimally, require that any explicit needs of an on.needs job are also listed in on.needs / otherwise guaranteed to run before activation) to prevent these cycle-prone setups with a clearer error.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 80/100✅ Excellent
Test Classification DetailsView all 6 tests
Language SupportTests analyzed:
ObservationsBuild tags: All new and modified test files have the required build tag on line 1. ✅ Assertion messages: All testify calls include descriptive message arguments. ✅ No mock libraries: No Test inflation (informational): Two test files exceed the 2:1 test-to-production line ratio, which triggers the scoring penalty:
Both cases represent tests that need more setup boilerplate to construct the right Minor gap: Neither Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
|
@copilot review all comments |
Contributor
|
✅ Smoke CI completed successfully! |
Contributor
|
🤖 Smoke CI — workflow completed successfully. Run: https://github.com/github/gh-aw/actions/runs/24807361070 |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2d42d763-cfa8-4bcb-9e6d-96a6cae9fe52 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…pendency chains Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2d42d763-cfa8-4bcb-9e6d-96a6cae9fe52 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Reviewed and addressed all actionable review comments in commit ab3ea84: updated diagnostics to use 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:
|
Contributor
|
Great work on this feature addition! 🎉 The It's also great to see Smoke CI passing, which gives confidence that the end-to-end compilation pipeline is healthy. This PR looks ready for maintainer review. 🚀
|
github-actions Bot
added a commit
that referenced
this pull request
Apr 23, 2026
Add glossary entry for 'Pre-Activation Dependencies (on.needs:)' introduced in #27895. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks
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.
This adds activation-side dependency control via
on.needs, enablingpre_activationandactivationto depend on custom upstream jobs (e.g., secret fetchers). It unblocks valid use of${{ needs.<job>.outputs.* }}inon.github-appcredentials.Schema + frontmatter model
on.needsto workflow schema.on.needsinto typed workflow data for compiler use.Compiler dependency wiring
pre_activationnow includeson.needsin itsneeds.activationnow includeson.needsin itsneeds(merged with existing before-activation dependencies).on.needsare treated as pre-activation prerequisites and do not receive implicitneeds: activation.Validation
on.needsvalidation:on.github-appexpression validation forapp-id/private-key:needs.<job>.outputs.*references must resolve to jobs available before activation (includingon.needs).Docs
on.needssemantics and usage for credential-supply patterns.Example:
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:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GOMOD GOMODCACHE x_amd64/vet env -json .go x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE er GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)https://api.github.com/repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b/usr/bin/gh gh api /repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b --jq [.object.sha, .object.type] | @tsv h ../../../.pret.prettierignore GO111MODULE $name) { hasDiscussionsEnabled } } GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)/usr/bin/gh gh api /repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b --jq [.object.sha, .object.type] | @tsv h ../../../.pret.prettierignore GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)If you need me to access, download, or install something from one of these locations, you can either:
🤖 Smoke CI passed — https://github.com/github/gh-aw/actions/runs/24807361070