Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 120 additions & 54 deletions .github/workflows/daily-token-consumption-report.lock.yml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions .github/workflows/smoke-project.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/test-project-url-default.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# ADR-27895: Introduce `on.needs` for Explicit Pre-Activation Job Dependencies

**Date**: 2026-04-22
**Status**: Draft
**Deciders**: pelikhan

---

## Part 1 — Narrative (Human-Friendly)

### Context

Workflow frontmatter supports `on.github-app` credentials that allow the activation job to mint a short-lived GitHub App token. Some security postures require fetching the App ID and private key from an external secret manager at runtime (e.g., HashiCorp Vault, AWS Secrets Manager) via a dedicated job that runs before activation. Prior to this change there was no way to declare such a dependency explicitly: the `pre_activation` and `activation` jobs always ran as the earliest jobs in the graph, making `${{ needs.<job>.outputs.* }}` expressions in `on.github-app` always resolve to empty values at runtime.

### Decision

We will add an `on.needs` array field to the workflow frontmatter `on:` section. Jobs listed in `on.needs` are wired as explicit dependencies of both `pre_activation` and `activation` so that their outputs are available before credential resolution occurs. Jobs in `on.needs` are excluded from the automatic `needs: activation` guard that would normally force custom jobs to run after activation. Validation ensures that only declared custom jobs (not built-in control jobs) can appear in `on.needs`, and that `on.github-app` expression references resolve exclusively to jobs available before activation.

### Alternatives Considered

#### Alternative 1: Auto-detect credential-supply jobs from `on.github-app` expressions

The compiler could parse `${{ needs.<job>.outputs.* }}` expressions in `on.github-app` fields and automatically promote those jobs to pre-activation dependencies without any user declaration. This approach was not chosen because it would make dependency wiring implicit and hard to reason about: a typo or expression change could silently break the dependency graph in non-obvious ways. Explicit declaration (`on.needs`) keeps the dependency contract visible in the frontmatter.

#### Alternative 2: Require credential-supply logic to be inlined as `on.steps`

The existing `on.steps` mechanism allows injecting arbitrary steps into the pre-activation job. Users could fetch credentials there instead of in a separate job. This was not chosen because it conflates credential supply with pre-activation gate logic, prevents parallel execution of credential-fetch and other pre-activation checks, and does not work for teams that already have standalone credential-supply jobs they want to reuse across multiple workflows.

### Consequences

#### Positive
- `on.github-app` expressions can now reference `needs.<job>.outputs.*` values from jobs that run before activation, enabling dynamic credential supply from external secret managers.
- Validation at compile time rejects invalid `on.needs` entries (built-in job names, cycle-prone jobs, undeclared jobs), turning silent runtime failures into clear compiler errors.

#### Negative
- Jobs listed in `on.needs` are exempt from the automatic `needs: activation` safeguard, meaning they run before the activation gate. This widens the surface of pre-activation execution, which must be considered when auditing workflow security.
- Introduces a new top-level frontmatter concept (`on.needs`) that users must learn; documentation and validation errors must be clear enough to avoid confusion with GitHub Actions' job-level `needs:` field.

#### Neutral
- When `on.needs` is non-empty and no other pre-activation checks exist, `pre_activation` is forced to be created (even if it would otherwise be omitted), so that `on.needs` jobs are properly sequenced before `activation`.
- The `activated` output of `pre_activation` is set unconditionally to `"true"` when the job is created solely due to `on.needs` (no permission checks, stop-time, skip-if, etc.), consistent with existing `on.steps`-only behaviour.

---

## 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).

### Schema and Parsing

1. Implementations **MUST** accept `on.needs` as an optional array of strings in the workflow frontmatter `on:` section.
2. Each entry in `on.needs` **MUST** match the pattern `^[a-zA-Z_][a-zA-Z0-9_-]*$`.
3. `on.needs` **MUST** contain unique entries (no duplicates).
4. If `on.needs` is absent or `null`, implementations **MUST** treat it as an empty list and **MUST NOT** alter the dependency graph.

### Compiler Dependency Wiring

1. Implementations **MUST** add every job named in `on.needs` to the `needs` list of the `pre_activation` job.
2. Implementations **MUST** add every job named in `on.needs` to the `needs` list of the `activation` job (merged with any existing before-activation dependencies).
3. Implementations **MUST NOT** add an implicit `needs: activation` dependency to any job that is listed in `on.needs`.
4. If `on.needs` is non-empty, implementations **MUST** create a `pre_activation` job even if no other pre-activation checks (permission, stop-time, skip-if, on.steps) are present.
5. When `pre_activation` is created solely because `on.needs` is non-empty (no other checks), the `activated` output **MUST** be set unconditionally to `"true"`.

### Validation

1. Implementations **MUST** reject any `on.needs` entry that references a built-in or compiler-generated job ID (e.g., `pre_activation`, `activation`).
2. Implementations **MUST** reject any `on.needs` entry that references a job that already depends on `pre_activation` or `activation`, to prevent dependency cycles.
3. Implementations **MUST** reject any `on.needs` entry that does not correspond to a job declared in the top-level `jobs:` section.
4. When `on.github-app` fields contain `${{ needs.<job>.outputs.* }}` expressions, implementations **MUST** verify that the referenced job is available before activation (i.e., listed in `on.needs` or otherwise before-activation). Implementations **MUST** emit a compiler error if the referenced job would run after activation.
5. Implementations **SHOULD** emit descriptive error messages that distinguish `on.needs` validation failures from job-level `needs:` validation failures.

### 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/24806829131) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
5 changes: 5 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,11 @@ on:
# (optional)
statuses: "read"

# Explicit additional custom workflow jobs that pre_activation and activation
# should depend on.
# (optional)
needs: ["secrets_fetcher"]

# When set to false, disables the frontmatter hash check step in the activation
# job. Default is true (check is enabled). Useful when the workflow source files
# are managed outside the default GitHub repo context (e.g. cross-repo org
Expand Down
1 change: 1 addition & 0 deletions docs/src/content/docs/reference/frontmatter.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ The `on:` section uses standard GitHub Actions syntax to define workflow trigger
- `skip-if-no-match:` - Skip execution when a search query has no matches (supports `scope: none`; use top-level `on.github-token` / `on.github-app` for custom auth)
- `steps:` - Inject custom deterministic steps into the pre-activation job (saves one workflow job vs. multi-job pattern)
- `permissions:` - Grant additional GitHub token scopes to the pre-activation job (for use with `on.steps:` API calls)
- `needs:` - Add custom job dependencies that both `pre_activation` and `activation` must wait for
- `github-token:` - Custom token for activation job reactions, status comments, and skip-if search queries
- `github-app:` - GitHub App for minting a short-lived token used by the activation job and all skip-if search steps

Expand Down
27 changes: 27 additions & 0 deletions docs/src/content/docs/reference/triggers.md
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,33 @@ if: needs.pre_activation.outputs.has_bug_label == 'true'

Explicit outputs defined in `jobs.pre-activation.outputs` take precedence over auto-wired `<id>_result` outputs on key collision.

### Pre-Activation and Activation Dependencies (`on.needs:`)

Add custom jobs that both `pre_activation` and `activation` should depend on. Use this when `on.github-app` credentials come from a job output (for example, a secret-manager fetch job).

```yaml wrap
on:
workflow_dispatch:
needs: [secrets_fetcher]
github-app:
client-id: ${{ needs.secrets_fetcher.outputs.app_id }}
private-key: ${{ needs.secrets_fetcher.outputs.private_key }}

jobs:
secrets_fetcher:
runs-on: ubuntu-latest
outputs:
app_id: ${{ steps.fetch.outputs.app_id }}
private_key: ${{ steps.fetch.outputs.private_key }}
steps:
- id: fetch
run: |
echo "app_id=123" >> "$GITHUB_OUTPUT"
echo "private_key=***" >> "$GITHUB_OUTPUT"
```

`on.needs` values must reference custom jobs from top-level `jobs:`. Built-in jobs are rejected.

### Pre-Activation Permissions (`on.permissions:`)

Grant additional GitHub token permission scopes to the pre-activation job. Use when `on.steps:` make GitHub API calls that require permissions beyond the defaults.
Expand Down
12 changes: 12 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,18 @@
}
]
},
"needs": {
"type": "array",
"description": "Explicit additional custom workflow jobs that pre_activation and activation should depend on.",
"items": {
"type": "string",
"pattern": "^[a-zA-Z_][a-zA-Z0-9_-]*$"
},
"additionalItems": false,
"uniqueItems": true,
"default": [],
"examples": [["secrets_fetcher"]]
},
"steps": {
"type": "array",
"description": "Steps to inject into the pre-activation job. These steps run after all built-in checks (membership, stop-time, skip-if, etc.) and their results are exposed as pre-activation outputs. Use 'id' on steps to reference their results via needs.pre_activation.outputs.<id>_result.",
Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
return formatCompilerError(markdownPath, "error", err.Error(), err)
}

// Validate on.needs declarations and on.github-app needs expressions
log.Printf("Validating on.needs declarations")
if err := c.validateOnNeeds(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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/workflow/compiler_activation_job_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ func (c *Compiler) addActivationCommandAndLabelOutputs(ctx *activationJobBuildCo
func (c *Compiler) configureActivationNeedsAndCondition(ctx *activationJobBuildContext) {
data := ctx.data
customJobsBeforeActivation := c.getCustomJobsDependingOnPreActivation(data.Jobs)
for _, jobName := range data.OnNeeds {
if !slices.Contains(customJobsBeforeActivation, jobName) {
customJobsBeforeActivation = append(customJobsBeforeActivation, jobName)
}
}
promptReferencedJobs := c.getCustomJobsReferencedInPromptWithNoActivationDep(data)
for _, jobName := range promptReferencedJobs {
if !slices.Contains(customJobsBeforeActivation, jobName) {
Expand Down
14 changes: 11 additions & 3 deletions pkg/workflow/compiler_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,11 @@ func (c *Compiler) buildPreActivationAndActivationJobs(data *WorkflowData, front
hasCommandTrigger := len(data.Command) > 0
hasRateLimit := data.RateLimit != nil
hasOnSteps := len(data.OnSteps) > 0
compilerJobsLog.Printf("Job configuration: needsPermissionCheck=%v, hasStopTime=%v, hasSkipIfMatch=%v, hasSkipIfNoMatch=%v, hasSkipRoles=%v, hasSkipBots=%v, hasCommand=%v, hasRateLimit=%v, hasOnSteps=%v", needsPermissionCheck, hasStopTime, hasSkipIfMatch, hasSkipIfNoMatch, hasSkipRoles, hasSkipBots, hasCommandTrigger, hasRateLimit, hasOnSteps)
hasOnNeeds := len(data.OnNeeds) > 0
compilerJobsLog.Printf("Job configuration: needsPermissionCheck=%v, hasStopTime=%v, hasSkipIfMatch=%v, hasSkipIfNoMatch=%v, hasSkipRoles=%v, hasSkipBots=%v, hasCommand=%v, hasRateLimit=%v, hasOnSteps=%v, hasOnNeeds=%v", needsPermissionCheck, hasStopTime, hasSkipIfMatch, hasSkipIfNoMatch, hasSkipRoles, hasSkipBots, hasCommandTrigger, hasRateLimit, hasOnSteps, hasOnNeeds)

// Build pre-activation job if needed (combines membership checks, stop-time validation, skip-if-match check, skip-if-no-match check, skip-roles check, skip-bots check, rate limit check, command position check, and on.steps injection)
if needsPermissionCheck || hasStopTime || hasSkipIfMatch || hasSkipIfNoMatch || hasSkipRoles || hasSkipBots || hasCommandTrigger || hasRateLimit || hasOnSteps {
if needsPermissionCheck || hasStopTime || hasSkipIfMatch || hasSkipIfNoMatch || hasSkipRoles || hasSkipBots || hasCommandTrigger || hasRateLimit || hasOnSteps || hasOnNeeds {
compilerJobsLog.Print("Building pre-activation job")
preActivationJob, err := c.buildPreActivationJob(data, needsPermissionCheck)
if err != nil {
Expand Down Expand Up @@ -490,6 +491,10 @@ func (c *Compiler) buildCustomJobs(data *WorkflowData, activationJobCreated bool
for _, j := range promptReferencedJobsSlice {
promptReferencedJobs[j] = true
}
onNeedsJobs := make(map[string]bool, len(data.OnNeeds))
for _, j := range data.OnNeeds {
onNeedsJobs[j] = true
}

for jobName, jobConfig := range data.Jobs {
// Skip jobs.pre-activation (or pre_activation) as it's handled specially in buildPreActivationJob
Expand Down Expand Up @@ -531,11 +536,14 @@ func (c *Compiler) buildCustomJobs(data *WorkflowData, activationJobCreated bool
// Exception: jobs whose outputs are referenced in the markdown body run before activation
// (so the activation job can include their outputs in the prompt).
isReferencedInMarkdown := promptReferencedJobs[jobName]
if !hasExplicitNeeds && activationJobCreated && !isReferencedInMarkdown {
isOnNeedsDependency := onNeedsJobs[jobName]
if !hasExplicitNeeds && activationJobCreated && !isReferencedInMarkdown && !isOnNeedsDependency {
job.Needs = append(job.Needs, string(constants.ActivationJobName))
compilerJobsLog.Printf("Added automatic dependency: custom job '%s' now depends on '%s'", jobName, string(constants.ActivationJobName))
} else if !hasExplicitNeeds && isReferencedInMarkdown {
compilerJobsLog.Printf("Custom job '%s' referenced in markdown body runs before activation (no auto-added dependency)", jobName)
} else if !hasExplicitNeeds && isOnNeedsDependency {
compilerJobsLog.Printf("Custom job '%s' listed in on.needs runs before activation (no auto-added dependency)", jobName)
}

// Extract other job properties
Expand Down
33 changes: 33 additions & 0 deletions pkg/workflow/compiler_jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/testutil"
"github.com/goccy/go-yaml"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// ========================================
Expand Down Expand Up @@ -2753,6 +2755,37 @@ func TestBuildCustomJobsSkipsPreActivationJob(t *testing.T) {
}
}

// TestBuildCustomJobsDoesNotAutoAddActivationWhenListedInOnNeeds verifies that
// custom jobs listed in on.needs run before activation and therefore do not get
// an implicit needs: activation dependency.
func TestBuildCustomJobsDoesNotAutoAddActivationWhenListedInOnNeeds(t *testing.T) {
compiler := NewCompiler()
compiler.jobManager = NewJobManager()

activationJob := &Job{Name: string(constants.ActivationJobName)}
require.NoError(t, compiler.jobManager.AddJob(activationJob), "activation job should be added")

data := &WorkflowData{
Name: "Test Workflow",
AI: "copilot",
OnNeeds: []string{"secrets_fetcher"},
Jobs: map[string]any{
"secrets_fetcher": map[string]any{
"runs-on": "ubuntu-latest",
"steps": []any{
map[string]any{"run": "echo 'fetch'"},
},
},
},
}

require.NoError(t, compiler.buildCustomJobs(data, true), "custom jobs should build")

job, exists := compiler.jobManager.GetJob("secrets_fetcher")
require.True(t, exists, "secrets_fetcher should be added")
assert.NotContains(t, job.Needs, string(constants.ActivationJobName), "on.needs job should not auto-depend on activation")
}

// TestBuildCustomJobsWithStrategy tests custom jobs with matrix strategy configuration
func TestBuildCustomJobsWithStrategy(t *testing.T) {
tmpDir := testutil.TempDir(t, "strategy-test")
Expand Down
7 changes: 7 additions & 0 deletions pkg/workflow/compiler_orchestrator_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,5 +454,12 @@ func (c *Compiler) processOnSectionAndFilters(
// Extract on.permissions for pre-activation job permissions
workflowData.OnPermissions = extractOnPermissions(frontmatter)

// Extract on.needs for pre-activation/activation job dependencies
onNeeds, err := extractOnNeeds(frontmatter)
if err != nil {
return err
}
workflowData.OnNeeds = onNeeds

return nil
}
Loading
Loading