From 8df03e9c5059129147680de2edf8f7dbbf0689fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 11:43:58 +0000 Subject: [PATCH 1/5] Initial plan From c3caa60fdb3976fd7730cd524d188bf0623e5036 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 11:59:10 +0000 Subject: [PATCH 2/5] chore: initial plan for fixing actionlint expression errors Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-gemini.lock.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/smoke-gemini.lock.yml b/.github/workflows/smoke-gemini.lock.yml index d1d6955c678..3e2b7c571ba 100644 --- a/.github/workflows/smoke-gemini.lock.yml +++ b/.github/workflows/smoke-gemini.lock.yml @@ -404,7 +404,7 @@ jobs: version: "", agent_version: "", workflow_name: "Smoke Gemini", - experimental: true, + experimental: false, supports_tools_allowlist: true, run_id: context.runId, run_number: context.runNumber, From 6f1e8d511abc1d54cedbd992f8fe687608484230 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 12:23:59 +0000 Subject: [PATCH 3/5] fix: resolve 22 actionlint expression errors in 4 workflow files - Fix compiler to add custom jobs (whose outputs are referenced in the markdown body) to activation's needs, preventing actionlint from complaining about undefined job outputs in expression contexts - Fix buildCustomJobs to not auto-add activation dependency to jobs whose outputs are referenced in the markdown body (they run before activation now) - Fix buildMainJob to also search frontmatter custom steps when finding referenced custom jobs for agent's needs - Fix generateKnownNeedsExpressions to skip the generic 'output' env var for jobs that declare explicit outputs not including 'output' - Fix release.md: change config and release jobs to depend on pre_activation instead of activation, enabling the correct ordering (pre_activation -> config -> release -> activation -> agent) Fixes expression errors in: - .github/workflows/bot-detection.lock.yml (12 errors) - .github/workflows/hourly-ci-cleaner.lock.yml (6 errors) - .github/workflows/release.lock.yml (3 errors) - .github/workflows/issue-monster.lock.yml (1 error) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/bot-detection.lock.yml | 3 +- .github/workflows/hourly-ci-cleaner.lock.yml | 3 +- .github/workflows/issue-monster.lock.yml | 4 +- .github/workflows/release.lock.yml | 11 +++-- .github/workflows/release.md | 4 +- pkg/workflow/compiler_activation_jobs.go | 22 ++++++++- pkg/workflow/compiler_jobs.go | 50 +++++++++++++++++++- pkg/workflow/known_needs_expressions.go | 20 ++++++-- 8 files changed, 100 insertions(+), 17 deletions(-) diff --git a/.github/workflows/bot-detection.lock.yml b/.github/workflows/bot-detection.lock.yml index ae31b29b37a..c3f0a43b62c 100644 --- a/.github/workflows/bot-detection.lock.yml +++ b/.github/workflows/bot-detection.lock.yml @@ -40,6 +40,8 @@ run-name: "Bot Detection" jobs: activation: + needs: precompute + if: needs.precompute.outputs.action != 'none' runs-on: ubuntu-slim permissions: contents: read @@ -1103,7 +1105,6 @@ jobs: await main(); precompute: - needs: activation runs-on: ubuntu-latest permissions: actions: read diff --git a/.github/workflows/hourly-ci-cleaner.lock.yml b/.github/workflows/hourly-ci-cleaner.lock.yml index e2719295e7a..c403abe0c8d 100644 --- a/.github/workflows/hourly-ci-cleaner.lock.yml +++ b/.github/workflows/hourly-ci-cleaner.lock.yml @@ -44,6 +44,8 @@ run-name: "CI Cleaner" jobs: activation: + needs: check_ci_status + if: needs.check_ci_status.outputs.ci_needs_fix == 'true' runs-on: ubuntu-slim permissions: contents: read @@ -922,7 +924,6 @@ jobs: if-no-files-found: ignore check_ci_status: - needs: activation runs-on: ubuntu-latest permissions: actions: read diff --git a/.github/workflows/issue-monster.lock.yml b/.github/workflows/issue-monster.lock.yml index b5f1f370161..b6917191d23 100644 --- a/.github/workflows/issue-monster.lock.yml +++ b/.github/workflows/issue-monster.lock.yml @@ -232,7 +232,6 @@ jobs: GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_COUNT: ${{ needs.search_issues.outputs.issue_count }} GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_LIST: ${{ needs.search_issues.outputs.issue_list }} GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_NUMBERS: ${{ needs.search_issues.outputs.issue_numbers }} - GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_OUTPUT: ${{ needs.search_issues.outputs.output }} with: script: | const { setupGlobals } = require('/opt/gh-aw/actions/setup_globals.cjs'); @@ -255,8 +254,7 @@ jobs: GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED, GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_COUNT: process.env.GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_COUNT, GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_LIST: process.env.GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_LIST, - GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_NUMBERS: process.env.GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_NUMBERS, - GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_OUTPUT: process.env.GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_OUTPUT + GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_NUMBERS: process.env.GH_AW_NEEDS_SEARCH_ISSUES_OUTPUTS_ISSUE_NUMBERS } }); - name: Validate prompt placeholders diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index 4ffe685b7a0..1f4e3f07195 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -23,7 +23,7 @@ # # Build, test, and release gh-aw extension, then generate and prepend release highlights # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"ec79dab32ab839039ad17c8de0bf76eba00547609fe0fbff497967153dd35069"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"51d800efab24f9ed75b360785ea56475e340f1fce4966f2360cb224dd28f8833"} name: "Release" "on": @@ -50,7 +50,10 @@ run-name: "Release" jobs: activation: - needs: pre_activation + needs: + - config + - pre_activation + - release if: needs.pre_activation.outputs.activated == 'true' runs-on: ubuntu-slim permissions: @@ -973,7 +976,7 @@ jobs: await main(); config: - needs: activation + needs: pre_activation runs-on: ubuntu-latest outputs: release_tag: ${{ steps.compute_config.outputs.release_tag }} @@ -1219,8 +1222,8 @@ jobs: release: needs: - - activation - config + - pre_activation runs-on: ubuntu-latest permissions: attestations: write diff --git a/.github/workflows/release.md b/.github/workflows/release.md index 3d18d92314b..4d6c0f2bf82 100644 --- a/.github/workflows/release.md +++ b/.github/workflows/release.md @@ -38,7 +38,7 @@ safe-outputs: update-release: jobs: config: - needs: ["activation"] + needs: ["pre_activation"] runs-on: ubuntu-latest outputs: release_tag: ${{ steps.compute_config.outputs.release_tag }} @@ -144,7 +144,7 @@ jobs: core.setOutput('release_tag', releaseTag); console.log(`✓ Release tag: ${releaseTag}`); release: - needs: ["activation", "config"] + needs: ["pre_activation", "config"] runs-on: ubuntu-latest permissions: contents: write diff --git a/pkg/workflow/compiler_activation_jobs.go b/pkg/workflow/compiler_activation_jobs.go index ada6a9e3925..4b5d7e9fb8c 100644 --- a/pkg/workflow/compiler_activation_jobs.go +++ b/pkg/workflow/compiler_activation_jobs.go @@ -7,6 +7,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/sliceutil" "github.com/github/gh-aw/pkg/stringutil" ) @@ -617,6 +618,18 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate // Find custom jobs that depend on pre_activation - these run before activation customJobsBeforeActivation := c.getCustomJobsDependingOnPreActivation(data.Jobs) + // Find custom jobs whose outputs are referenced in the markdown body but have no explicit needs. + // These jobs must run before activation so their outputs are available when the activation job + // builds the prompt. Without this, activation would reference their outputs while they haven't + // run yet, causing actionlint errors and incorrect prompt substitutions. + promptReferencedJobs := c.getCustomJobsReferencedInPromptWithNoActivationDep(data) + for _, jobName := range promptReferencedJobs { + if !sliceutil.Contains(customJobsBeforeActivation, jobName) { + customJobsBeforeActivation = append(customJobsBeforeActivation, jobName) + compilerActivationJobsLog.Printf("Added '%s' to activation dependencies: referenced in markdown body and has no explicit needs", jobName) + } + } + if preActivationJobCreated { // Activation job depends on pre-activation job and checks the "activated" output activationNeeds = []string{string(constants.PreActivationJobName)} @@ -807,7 +820,14 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( // through the activation job, if the workflow content directly references their outputs // (e.g., ${{ needs.search_issues.outputs.* }}), we MUST add them as direct dependencies. // This is required for GitHub Actions expression evaluation and actionlint validation. - referencedJobs := c.getReferencedCustomJobs(data.MarkdownContent, data.Jobs) + // Also check custom steps from the frontmatter, which are also added to the agent job. + var contentBuilder strings.Builder + contentBuilder.WriteString(data.MarkdownContent) + if data.CustomSteps != "" { + contentBuilder.WriteByte('\n') + contentBuilder.WriteString(data.CustomSteps) + } + referencedJobs := c.getReferencedCustomJobs(contentBuilder.String(), data.Jobs) for _, jobName := range referencedJobs { // Skip jobs.pre-activation (or pre_activation) as it's handled specially if jobName == string(constants.PreActivationJobName) || jobName == "pre-activation" { diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index a426f55eb29..24009aea29e 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -120,6 +120,38 @@ func (c *Compiler) getReferencedCustomJobs(content string, customJobs map[string return refs } +// getCustomJobsReferencedInPromptWithNoActivationDep returns custom jobs whose outputs are referenced +// in the markdown body content but have no explicit needs (and therefore no activation dependency). +// These jobs need to run before the activation job so their outputs are available when the +// activation job builds the prompt. Without this, activation's prompt-building steps would reference +// those job outputs before the jobs have run, causing actionlint errors and empty substitutions. +// +// Only jobs with NO explicit needs are returned - jobs that explicitly depend on activation/pre_activation/etc. +// are excluded because they either already run before activation or cannot run before it. +func (c *Compiler) getCustomJobsReferencedInPromptWithNoActivationDep(data *WorkflowData) []string { + if data == nil || data.Jobs == nil || data.MarkdownContent == "" { + return nil + } + + referencedJobs := c.getReferencedCustomJobs(data.MarkdownContent, data.Jobs) + var result []string + for _, jobName := range referencedJobs { + jobConfig, ok := data.Jobs[jobName].(map[string]any) + if !ok { + continue + } + // Only include jobs with no explicit needs - those get activation auto-added normally. + // Jobs with explicit needs either already run before activation (pre_activation dependency) + // or explicitly depend on activation/agent and must run after. + if _, hasNeeds := jobConfig["needs"]; hasNeeds { + continue + } + result = append(result, jobName) + compilerJobsLog.Printf("Found custom job '%s' referenced in markdown body with no explicit needs: will run before activation", jobName) + } + return result +} + // buildJobs creates all jobs for the workflow and adds them to the job manager. // This function orchestrates the building of all job types by delegating to focused helper functions. func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error { @@ -360,6 +392,15 @@ func (c *Compiler) extractJobsFromFrontmatter(frontmatter map[string]any) map[st // buildCustomJobs creates custom jobs defined in the frontmatter jobs section func (c *Compiler) buildCustomJobs(data *WorkflowData, activationJobCreated bool) error { compilerJobsLog.Printf("Building %d custom jobs", len(data.Jobs)) + + // Pre-compute jobs referenced in the markdown body with no explicit needs. + // These run before activation (not after), so we must not auto-add activation to them. + promptReferencedJobsSlice := c.getCustomJobsReferencedInPromptWithNoActivationDep(data) + promptReferencedJobs := make(map[string]bool, len(promptReferencedJobsSlice)) + for _, j := range promptReferencedJobsSlice { + promptReferencedJobs[j] = true + } + for jobName, jobConfig := range data.Jobs { // Skip jobs.pre-activation (or pre_activation) as it's handled specially in buildPreActivationJob if jobName == string(constants.PreActivationJobName) || jobName == "pre-activation" { @@ -389,10 +430,15 @@ func (c *Compiler) buildCustomJobs(data *WorkflowData, activationJobCreated bool } // If no explicit needs and activation job exists, automatically add activation as dependency - // This ensures custom jobs wait for workflow validation before executing - if !hasExplicitNeeds && activationJobCreated { + // This ensures custom jobs wait for workflow validation before executing. + // 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 { 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) } // Extract other job properties diff --git a/pkg/workflow/known_needs_expressions.go b/pkg/workflow/known_needs_expressions.go index 63ce709ebd6..253e8d2e749 100644 --- a/pkg/workflow/known_needs_expressions.go +++ b/pkg/workflow/known_needs_expressions.go @@ -71,9 +71,23 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b if data.Jobs != nil { customJobNames := getCustomJobsBeforeActivation(data) for _, jobName := range customJobNames { - // For custom jobs, we can't know all possible outputs ahead of time - // But we can add the most commonly used output name: "output" - // Users can add more specific outputs if needed + // If the job has explicit outputs declared in the frontmatter, skip the generic "output" + // env var unless "output" is explicitly among those declared outputs. + // This prevents actionlint errors when the job declares specific outputs but not "output". + if jobConfig, ok := data.Jobs[jobName].(map[string]any); ok { + if outputsField, hasOutputs := jobConfig["outputs"]; hasOutputs && outputsField != nil { + if outputsMap, ok := outputsField.(map[string]any); ok { + if _, hasOutputKey := outputsMap["output"]; !hasOutputKey { + // Job has explicit outputs but "output" is not among them - skip + knownNeedsLog.Printf("Skipping generic 'output' env var for job '%s': has explicit outputs without 'output'", jobName) + continue + } + } + } + } + + // For custom jobs without explicit outputs (or with "output" declared), + // add the most commonly used output name: "output" commonCustomOutputs := []string{ "output", } From 67ab7dc23054e76af12e274cc1f413af29e4152a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 12:47:48 +0000 Subject: [PATCH 4/5] fix: add activation to config and release needs in release.md; add compiler filter for post-activation job expressions Per reviewer feedback, add `activation` (alongside `pre_activation`) to the `needs:` of both `config` and `release` jobs so they correctly gate on activation completing. This required two compiler improvements to avoid a circular dependency: - `jobDependsOnActivation()` helper function - `getCustomJobsDependingOnPreActivation()` now excludes jobs that also depend on activation, preventing them from being incorrectly added to activation's needs - `generatePrompt()` / `generatePromptInActivationJob()` accept `beforeActivationJobs` and filter expression mappings so activation's prompt steps don't reference outputs from post-activation jobs (which haven't run yet), preventing actionlint errors - Update two test call-sites for the new parameter Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/release.lock.yml | 18 ++++----- .github/workflows/release.md | 4 +- pkg/workflow/compiler_activation_jobs.go | 9 +++-- pkg/workflow/compiler_jobs.go | 25 ++++++++++-- pkg/workflow/compiler_yaml.go | 9 ++++- pkg/workflow/known_needs_expressions.go | 48 ++++++++++++++++++++++++ pkg/workflow/missing_tool_test.go | 4 +- pkg/workflow/xml_comments_test.go | 2 +- 8 files changed, 96 insertions(+), 23 deletions(-) diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index 1f4e3f07195..2f4d772a239 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -23,7 +23,7 @@ # # Build, test, and release gh-aw extension, then generate and prepend release highlights # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"51d800efab24f9ed75b360785ea56475e340f1fce4966f2360cb224dd28f8833"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"7bac240d8646b61e0b920ab0aaf714c94e7290cd6476e767d9d695b9431cf50d"} name: "Release" "on": @@ -50,10 +50,7 @@ run-name: "Release" jobs: activation: - needs: - - config - - pre_activation - - release + needs: pre_activation if: needs.pre_activation.outputs.activated == 'true' runs-on: ubuntu-slim permissions: @@ -110,7 +107,6 @@ jobs: GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} GH_AW_GITHUB_WORKSPACE: ${{ github.workspace }} - GH_AW_NEEDS_RELEASE_OUTPUTS_RELEASE_ID: ${{ needs.release.outputs.release_id }} run: | bash /opt/gh-aw/actions/create_prompt_first.sh cat << 'GH_AW_PROMPT_EOF' > "$GH_AW_PROMPT" @@ -207,7 +203,6 @@ jobs: env: GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} - GH_AW_NEEDS_RELEASE_OUTPUTS_RELEASE_ID: ${{ needs.release.outputs.release_id }} with: script: | const { setupGlobals } = require('/opt/gh-aw/actions/setup_globals.cjs'); @@ -227,7 +222,6 @@ jobs: GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} GH_AW_GITHUB_WORKSPACE: ${{ github.workspace }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: ${{ needs.pre_activation.outputs.activated }} - GH_AW_NEEDS_RELEASE_OUTPUTS_RELEASE_ID: ${{ needs.release.outputs.release_id }} with: script: | const { setupGlobals } = require('/opt/gh-aw/actions/setup_globals.cjs'); @@ -247,8 +241,7 @@ jobs: GH_AW_GITHUB_REPOSITORY: process.env.GH_AW_GITHUB_REPOSITORY, GH_AW_GITHUB_RUN_ID: process.env.GH_AW_GITHUB_RUN_ID, GH_AW_GITHUB_WORKSPACE: process.env.GH_AW_GITHUB_WORKSPACE, - GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED, - GH_AW_NEEDS_RELEASE_OUTPUTS_RELEASE_ID: process.env.GH_AW_NEEDS_RELEASE_OUTPUTS_RELEASE_ID + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED } }); - name: Validate prompt placeholders @@ -976,7 +969,9 @@ jobs: await main(); config: - needs: pre_activation + needs: + - activation + - pre_activation runs-on: ubuntu-latest outputs: release_tag: ${{ steps.compute_config.outputs.release_tag }} @@ -1222,6 +1217,7 @@ jobs: release: needs: + - activation - config - pre_activation runs-on: ubuntu-latest diff --git a/.github/workflows/release.md b/.github/workflows/release.md index 4d6c0f2bf82..9bbbd2f1677 100644 --- a/.github/workflows/release.md +++ b/.github/workflows/release.md @@ -38,7 +38,7 @@ safe-outputs: update-release: jobs: config: - needs: ["pre_activation"] + needs: ["pre_activation", "activation"] runs-on: ubuntu-latest outputs: release_tag: ${{ steps.compute_config.outputs.release_tag }} @@ -144,7 +144,7 @@ jobs: core.setOutput('release_tag', releaseTag); console.log(`✓ Release tag: ${releaseTag}`); release: - needs: ["pre_activation", "config"] + needs: ["pre_activation", "activation", "config"] runs-on: ubuntu-latest permissions: contents: write diff --git a/pkg/workflow/compiler_activation_jobs.go b/pkg/workflow/compiler_activation_jobs.go index 4b5d7e9fb8c..e1958734c32 100644 --- a/pkg/workflow/compiler_activation_jobs.go +++ b/pkg/workflow/compiler_activation_jobs.go @@ -680,7 +680,7 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate // Generate prompt in the activation job (before agent job runs) compilerActivationJobsLog.Print("Generating prompt in activation job") - c.generatePromptInActivationJob(&steps, data, preActivationJobCreated) + c.generatePromptInActivationJob(&steps, data, preActivationJobCreated, customJobsBeforeActivation) // Upload prompt.txt as an artifact for the agent job to download compilerActivationJobsLog.Print("Adding prompt artifact upload step") @@ -976,14 +976,17 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( // generatePromptInActivationJob generates the prompt creation steps and adds them to the activation job // This creates the prompt.txt file that will be uploaded as an artifact and downloaded by the agent job -func (c *Compiler) generatePromptInActivationJob(steps *[]string, data *WorkflowData, preActivationJobCreated bool) { +// beforeActivationJobs is the list of custom job names that run before (i.e., are dependencies of) activation. +// Passing nil or an empty slice means no custom jobs run before activation; expressions referencing any +// custom job will be filtered out of the substitution step to avoid actionlint errors. +func (c *Compiler) generatePromptInActivationJob(steps *[]string, data *WorkflowData, preActivationJobCreated bool, beforeActivationJobs []string) { compilerActivationJobsLog.Print("Generating prompt steps in activation job") // Use a string builder to collect the YAML var yaml strings.Builder // Call the existing generatePrompt method to get all the prompt steps - c.generatePrompt(&yaml, data, preActivationJobCreated) + c.generatePrompt(&yaml, data, preActivationJobCreated, beforeActivationJobs) // Append the generated YAML content as a single string to steps yamlContent := yaml.String() diff --git a/pkg/workflow/compiler_jobs.go b/pkg/workflow/compiler_jobs.go index 24009aea29e..aeb19e3b6b0 100644 --- a/pkg/workflow/compiler_jobs.go +++ b/pkg/workflow/compiler_jobs.go @@ -66,6 +66,23 @@ func jobDependsOnPreActivation(jobConfig map[string]any) bool { return false } +// jobDependsOnActivation checks if a job config has activation as a dependency. +// Jobs that depend on activation run AFTER activation, not before it. +func jobDependsOnActivation(jobConfig map[string]any) bool { + if needs, hasNeeds := jobConfig["needs"]; hasNeeds { + if needsList, ok := needs.([]any); ok { + for _, need := range needsList { + if needStr, ok := need.(string); ok && needStr == string(constants.ActivationJobName) { + return true + } + } + } else if needStr, ok := needs.(string); ok && needStr == string(constants.ActivationJobName) { + return true + } + } + return false +} + // jobDependsOnAgent checks if a job config has agent as a dependency. // Jobs that depend on agent should run AFTER the agent job, not before it. // The jobConfig parameter is expected to be a map representing the job's YAML configuration, @@ -86,13 +103,15 @@ func jobDependsOnAgent(jobConfig map[string]any) bool { return false } -// getCustomJobsDependingOnPreActivation returns custom job names that explicitly depend on pre_activation. -// These jobs run after pre_activation but before activation, and activation should depend on them. +// getCustomJobsDependingOnPreActivation returns custom job names that explicitly depend on pre_activation +// but NOT on activation. These jobs run after pre_activation but before activation, and activation +// should depend on them. Jobs that also depend on activation cannot run before activation. func (c *Compiler) getCustomJobsDependingOnPreActivation(customJobs map[string]any) []string { compilerJobsLog.Printf("Finding custom jobs depending on pre_activation: total_custom_jobs=%d", len(customJobs)) deps := sliceutil.FilterMapKeys(customJobs, func(jobName string, jobConfig any) bool { if configMap, ok := jobConfig.(map[string]any); ok { - return jobDependsOnPreActivation(configMap) + // Must depend on pre_activation AND must NOT depend on activation + return jobDependsOnPreActivation(configMap) && !jobDependsOnActivation(configMap) } return false }) diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index c0dcec63ea8..ddeef4b3c04 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -254,7 +254,7 @@ func splitContentIntoChunks(content string) []string { return chunks } -func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, preActivationJobCreated bool) { +func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, preActivationJobCreated bool, beforeActivationJobs []string) { compilerYamlLog.Printf("Generating prompt for workflow: %s (markdown size: %d bytes)", data.Name, len(data.MarkdownContent)) // Collect built-in prompt sections (these should be prepended to user prompt) @@ -343,6 +343,13 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre } } + // Filter out expression mappings referencing custom jobs that run AFTER activation. + // These jobs (which explicitly depend on activation) cannot have outputs available when + // the activation job builds and substitutes the prompt. Keeping them would cause actionlint + // errors because the jobs are not in activation's needs, yet their outputs would be + // referenced in activation's step env vars. + expressionMappings = filterExpressionsForActivation(expressionMappings, data.Jobs, beforeActivationJobs) + // Step 2: Add main workflow markdown content to the prompt if c.inlinePrompt || data.InlinedImports { // Inline mode (Wasm/browser): embed the markdown content directly in the YAML diff --git a/pkg/workflow/known_needs_expressions.go b/pkg/workflow/known_needs_expressions.go index 253e8d2e749..500cde14897 100644 --- a/pkg/workflow/known_needs_expressions.go +++ b/pkg/workflow/known_needs_expressions.go @@ -3,6 +3,7 @@ package workflow import ( "fmt" "sort" + "strings" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" @@ -109,6 +110,53 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b return mappings } +// filterExpressionsForActivation filters expression mappings to remove any that reference +// custom jobs NOT in beforeActivationJobs. This prevents actionlint errors when a custom job +// explicitly depends on activation (and therefore runs AFTER activation) but the markdown body +// contains expressions like ${{ needs.that_job.outputs.foo }} that would be impossible to +// evaluate at activation time. +// +// If beforeActivationJobs is nil or empty, any expression referencing a custom job (one present +// in customJobs) is dropped because no custom job runs before activation. +// +// Only expressions referencing jobs in customJobs are considered for filtering; standard +// GitHub Actions contexts (github.*, env.*, etc.) and system job outputs (pre_activation) are +// always kept. +func filterExpressionsForActivation(mappings []*ExpressionMapping, customJobs map[string]any, beforeActivationJobs []string) []*ExpressionMapping { + if customJobs == nil || len(mappings) == 0 { + return mappings + } + + beforeActivationSet := make(map[string]bool, len(beforeActivationJobs)) + for _, j := range beforeActivationJobs { + beforeActivationSet[j] = true + } + + filtered := make([]*ExpressionMapping, 0, len(mappings)) + for _, m := range mappings { + // Only examine needs.* expressions + if !strings.HasPrefix(m.Content, "needs.") { + filtered = append(filtered, m) + continue + } + // Extract the job name (needs..*) + rest := m.Content[len("needs."):] + dotIdx := strings.Index(rest, ".") + if dotIdx < 0 { + filtered = append(filtered, m) + continue + } + jobName := rest[:dotIdx] + // If it's a custom job NOT in beforeActivationJobs, drop it + if _, isCustomJob := customJobs[jobName]; isCustomJob && !beforeActivationSet[jobName] { + knownNeedsLog.Printf("Filtered post-activation expression from activation substitution step: %s", m.Content) + continue + } + filtered = append(filtered, m) + } + return filtered +} + // normalizeJobNameForEnvVar converts a job name to a valid environment variable segment // Examples: "activation" -> "ACTIVATION", "pre_activation" -> "PRE_ACTIVATION" func normalizeJobNameForEnvVar(jobName string) string { diff --git a/pkg/workflow/missing_tool_test.go b/pkg/workflow/missing_tool_test.go index 4acff076c31..d37d024192f 100644 --- a/pkg/workflow/missing_tool_test.go +++ b/pkg/workflow/missing_tool_test.go @@ -148,7 +148,7 @@ func TestGeneratePromptIncludesGitHubAWPrompt(t *testing.T) { } var yaml strings.Builder - compiler.generatePrompt(&yaml, data, false) + compiler.generatePrompt(&yaml, data, false, nil) output := yaml.String() @@ -175,7 +175,7 @@ func TestMissingToolPromptGeneration(t *testing.T) { } var yaml strings.Builder - compiler.generatePrompt(&yaml, data, false) + compiler.generatePrompt(&yaml, data, false, nil) output := yaml.String() diff --git a/pkg/workflow/xml_comments_test.go b/pkg/workflow/xml_comments_test.go index 663fb63d054..82278522496 100644 --- a/pkg/workflow/xml_comments_test.go +++ b/pkg/workflow/xml_comments_test.go @@ -239,7 +239,7 @@ Final content.`, } var yaml strings.Builder - compiler.generatePrompt(&yaml, data, false) + compiler.generatePrompt(&yaml, data, false, nil) output := yaml.String() From 32c197707aa9c8e793e8a4ff719b5863f7fc8127 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 13:17:11 +0000 Subject: [PATCH 5/5] test: add tests for new jobDependsOnActivation, filterExpressionsForActivation, and updated getCustomJobsDependingOnPreActivation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 21 new test cases across two test files: compiler_jobs_test.go: - TestJobDependsOnActivation — 8 cases covering string, array and invalid input - TestGetCustomJobsDependingOnPreActivationExcludesActivationDependents — 4 cases verifying jobs that also depend on activation are not returned as pre-activation jobs - TestBuildCustomJobsDoesNotAutoAddActivationToOutputReferencedJobs — verifies the compiler does not auto-add needs:activation to jobs whose outputs appear in the markdown known_needs_expressions_test.go: - TestFilterExpressionsForActivation — 8 unit cases covering nil customJobs, empty mappings, keep/filter by beforeActivationJobs, non-custom-job needs expressions, non-needs expressions, and malformed expressions - TestFilterExpressionsForActivationEndToEnd — integration test compiling a full workflow with precompute (before activation) and config (after activation) jobs, asserting that activation depends on precompute, not config, and that the substitution step only references precompute's outputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_test.go | 202 ++++++++++++++++++- pkg/workflow/known_needs_expressions_test.go | 195 ++++++++++++++++++ 2 files changed, 396 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index 170cf19d0e4..6a156672c3a 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -213,7 +213,207 @@ func TestGetCustomJobsDependingOnPreActivationEdgeCases(t *testing.T) { } } -// TestGetReferencedCustomJobs tests the getReferencedCustomJobs method +// TestJobDependsOnActivation tests the jobDependsOnActivation function +func TestJobDependsOnActivation(t *testing.T) { + tests := []struct { + name string + jobConfig map[string]any + expected bool + }{ + { + name: "no needs field", + jobConfig: map[string]any{"runs-on": "ubuntu-latest"}, + expected: false, + }, + { + name: "needs: activation as string", + jobConfig: map[string]any{"needs": "activation"}, + expected: true, + }, + { + name: "needs: pre_activation only", + jobConfig: map[string]any{"needs": "pre_activation"}, + expected: false, + }, + { + name: "needs: agent only", + jobConfig: map[string]any{"needs": "agent"}, + expected: false, + }, + { + name: "needs: [activation, pre_activation] array", + jobConfig: map[string]any{ + "needs": []any{"pre_activation", "activation"}, + }, + expected: true, + }, + { + name: "needs: array without activation", + jobConfig: map[string]any{ + "needs": []any{"pre_activation", "config"}, + }, + expected: false, + }, + { + name: "needs: array with mixed types including activation", + jobConfig: map[string]any{ + "needs": []any{123, "activation"}, + }, + expected: true, + }, + { + name: "needs: invalid type", + jobConfig: map[string]any{"needs": 123}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := jobDependsOnActivation(tt.jobConfig) + if result != tt.expected { + t.Errorf("jobDependsOnActivation() = %v, want %v", result, tt.expected) + } + }) + } +} + +// TestGetCustomJobsDependingOnPreActivationExcludesActivationDependents tests that +// getCustomJobsDependingOnPreActivation excludes jobs that also depend on activation. +// This prevents the compiler from adding such jobs to activation's needs (which would +// create a circular dependency: activation → job → activation). +func TestGetCustomJobsDependingOnPreActivationExcludesActivationDependents(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + customJobs map[string]any + expectedJobs []string + excludedJobs []string + }{ + { + name: "job with both pre_activation and activation is excluded", + customJobs: map[string]any{ + "config": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": []any{"pre_activation", "activation"}, + }, + }, + expectedJobs: []string{}, + excludedJobs: []string{"config"}, + }, + { + name: "job with only pre_activation is included", + customJobs: map[string]any{ + "precompute": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": []any{"pre_activation"}, + }, + }, + expectedJobs: []string{"precompute"}, + excludedJobs: []string{}, + }, + { + name: "mixed: pre_activation-only included, pre_activation+activation excluded", + customJobs: map[string]any{ + "precompute": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": "pre_activation", + }, + "config": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": []any{"pre_activation", "activation"}, + }, + "release": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": []any{"pre_activation", "activation", "config"}, + }, + }, + expectedJobs: []string{"precompute"}, + excludedJobs: []string{"config", "release"}, + }, + { + name: "job with only activation dependency is excluded", + customJobs: map[string]any{ + "post_job": map[string]any{ + "runs-on": "ubuntu-latest", + "needs": "activation", + }, + }, + expectedJobs: []string{}, + excludedJobs: []string{"post_job"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := compiler.getCustomJobsDependingOnPreActivation(tt.customJobs) + resultSet := make(map[string]bool, len(result)) + for _, j := range result { + resultSet[j] = true + } + for _, expected := range tt.expectedJobs { + if !resultSet[expected] { + t.Errorf("Expected job %q in result, got: %v", expected, result) + } + } + for _, excluded := range tt.excludedJobs { + if resultSet[excluded] { + t.Errorf("Job %q should be excluded from result, got: %v", excluded, result) + } + } + }) + } +} + +// TestBuildCustomJobsDoesNotAutoAddActivationToOutputReferencedJobs tests that +// buildCustomJobs does NOT auto-add needs: activation to custom jobs whose outputs +// are referenced in the markdown body (they must run before activation). +func TestBuildCustomJobsDoesNotAutoAddActivationToOutputReferencedJobs(t *testing.T) { + compiler := NewCompiler() + compiler.jobManager = NewJobManager() + + // Add activation job to manager + activationJob := &Job{Name: string(constants.ActivationJobName)} + if err := compiler.jobManager.AddJob(activationJob); err != nil { + t.Fatal(err) + } + + data := &WorkflowData{ + Name: "Test Workflow", + AI: "copilot", + RunsOn: "runs-on: ubuntu-latest", + // precompute has no explicit needs and its output is referenced in the markdown + MarkdownContent: "Action: ${{ needs.precompute.outputs.action }}", + Jobs: map[string]any{ + "precompute": map[string]any{ + "runs-on": "ubuntu-latest", + "steps": []any{ + map[string]any{"run": "echo 'precompute'"}, + }, + // No explicit needs — normally would auto-get needs: activation + // But since precompute's output is referenced in markdown, it should NOT + }, + }, + } + + err := compiler.buildCustomJobs(data, true) + if err != nil { + t.Fatalf("buildCustomJobs() returned error: %v", err) + } + + job, exists := compiler.jobManager.GetJob("precompute") + if !exists { + t.Fatal("Expected precompute job to be added") + } + + for _, need := range job.Needs { + if need == string(constants.ActivationJobName) { + t.Errorf("precompute job should NOT have needs: activation when its output is referenced in markdown (it must run before activation)") + } + } +} + func TestGetReferencedCustomJobs(t *testing.T) { compiler := NewCompiler() diff --git a/pkg/workflow/known_needs_expressions_test.go b/pkg/workflow/known_needs_expressions_test.go index d2033635177..4e50ff1bd60 100644 --- a/pkg/workflow/known_needs_expressions_test.go +++ b/pkg/workflow/known_needs_expressions_test.go @@ -587,3 +587,198 @@ func TestParseNeedsFieldArrayTypes(t *testing.T) { }) } } + +// ======================================== +// filterExpressionsForActivation Tests +// ======================================== + +func TestFilterExpressionsForActivation(t *testing.T) { + customJobs := map[string]any{ + "precompute": map[string]any{"runs-on": "ubuntu-latest"}, + "config": map[string]any{"runs-on": "ubuntu-latest"}, + } + + tests := []struct { + name string + mappings []*ExpressionMapping + customJobs map[string]any + beforeActivationJobs []string + expectedContents []string + excludedContents []string + }{ + { + name: "nil customJobs returns all mappings unchanged", + customJobs: nil, + mappings: []*ExpressionMapping{ + {Content: "needs.precompute.outputs.action", EnvVar: "GH_AW_NEEDS_PRECOMPUTE_OUTPUTS_ACTION"}, + {Content: "github.event.issue.number", EnvVar: "GH_AW_GITHUB_EVENT_ISSUE_NUMBER"}, + }, + beforeActivationJobs: nil, + expectedContents: []string{"needs.precompute.outputs.action", "github.event.issue.number"}, + }, + { + name: "empty mappings returns empty slice", + customJobs: customJobs, + mappings: []*ExpressionMapping{}, + beforeActivationJobs: []string{"precompute"}, + expectedContents: []string{}, + }, + { + name: "job in beforeActivationJobs is kept", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "needs.precompute.outputs.action", EnvVar: "GH_AW_NEEDS_PRECOMPUTE_OUTPUTS_ACTION"}, + }, + beforeActivationJobs: []string{"precompute"}, + expectedContents: []string{"needs.precompute.outputs.action"}, + }, + { + name: "job NOT in beforeActivationJobs is filtered out", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "needs.config.outputs.release_tag", EnvVar: "GH_AW_NEEDS_CONFIG_OUTPUTS_RELEASE_TAG"}, + }, + beforeActivationJobs: []string{"precompute"}, // config is not before activation + excludedContents: []string{"needs.config.outputs.release_tag"}, + }, + { + name: "nil beforeActivationJobs filters all custom job expressions", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "needs.precompute.outputs.action", EnvVar: "GH_AW_NEEDS_PRECOMPUTE_OUTPUTS_ACTION"}, + {Content: "needs.config.outputs.release_tag", EnvVar: "GH_AW_NEEDS_CONFIG_OUTPUTS_RELEASE_TAG"}, + {Content: "github.event.issue.number", EnvVar: "GH_AW_GITHUB_EVENT_ISSUE_NUMBER"}, + }, + beforeActivationJobs: nil, + // All custom job expressions are dropped; non-needs.* expression is kept + excludedContents: []string{"needs.precompute.outputs.action", "needs.config.outputs.release_tag"}, + expectedContents: []string{"github.event.issue.number"}, + }, + { + name: "non-custom-job needs.* expression is always kept", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "needs.pre_activation.outputs.activated", EnvVar: "GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED"}, + {Content: "needs.precompute.outputs.action", EnvVar: "GH_AW_NEEDS_PRECOMPUTE_OUTPUTS_ACTION"}, + }, + beforeActivationJobs: []string{"precompute"}, + // pre_activation is not in customJobs so it's kept; precompute is in beforeActivationJobs so it's kept + expectedContents: []string{ + "needs.pre_activation.outputs.activated", + "needs.precompute.outputs.action", + }, + }, + { + name: "non-needs expression is always kept regardless of beforeActivationJobs", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "github.event.issue.number", EnvVar: "GH_AW_GITHUB_EVENT_ISSUE_NUMBER"}, + {Content: "steps.sanitized.outputs.text", EnvVar: "GH_AW_STEPS_SANITIZED_OUTPUTS_TEXT"}, + {Content: "needs.config.outputs.release_tag", EnvVar: "GH_AW_NEEDS_CONFIG_OUTPUTS_RELEASE_TAG"}, + }, + beforeActivationJobs: nil, + expectedContents: []string{ + "github.event.issue.number", + "steps.sanitized.outputs.text", + }, + excludedContents: []string{"needs.config.outputs.release_tag"}, + }, + { + name: "malformed needs expression without second dot is kept", + customJobs: customJobs, + mappings: []*ExpressionMapping{ + {Content: "needs.precompute", EnvVar: "GH_AW_NEEDS_PRECOMPUTE"}, + }, + beforeActivationJobs: nil, + // No second dot → parsing fails → kept as-is (conservative) + expectedContents: []string{"needs.precompute"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := filterExpressionsForActivation(tt.mappings, tt.customJobs, tt.beforeActivationJobs) + + resultSet := make(map[string]bool, len(result)) + for _, m := range result { + resultSet[m.Content] = true + } + + for _, expected := range tt.expectedContents { + assert.True(t, resultSet[expected], + "Expected expression %q to be kept in result, got: %v", expected, resultSet) + } + for _, excluded := range tt.excludedContents { + assert.False(t, resultSet[excluded], + "Expected expression %q to be filtered out of result, got: %v", excluded, resultSet) + } + }) + } +} + +// TestFilterExpressionsForActivationEndToEnd verifies that a workflow with a custom job +// that depends on both pre_activation and activation does not produce expression references +// to that job in the activation prompt's substitution step. +func TestFilterExpressionsForActivationEndToEnd(t *testing.T) { + workflowContent := `--- +engine: copilot +on: issues +permissions: + issues: read +jobs: + precompute: + runs-on: ubuntu-latest + needs: pre_activation + outputs: + action: ${{ steps.detect.outputs.action }} + steps: + - id: detect + run: echo "action=bot" >> "$GITHUB_OUTPUT" + config: + runs-on: ubuntu-latest + needs: ["pre_activation", "activation"] + outputs: + release_tag: ${{ steps.tag.outputs.release_tag }} + steps: + - id: tag + run: echo "release_tag=v1.0.0" >> "$GITHUB_OUTPUT" +--- + +# Test Workflow + +Action: ${{ needs.precompute.outputs.action }} +Release tag: ${{ needs.config.outputs.release_tag }} +` + tmpDir := t.TempDir() + workflowPath := tmpDir + "/test-filter-workflow.md" + require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644)) + + compiler := NewCompiler() + compiler.SetQuiet(true) + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + + lockContent, err := os.ReadFile(tmpDir + "/test-filter-workflow.lock.yml") + require.NoError(t, err) + lockStr := string(lockContent) + + // activation must depend on precompute (it runs before activation and its output is referenced) + activationSection := extractJobSection(lockStr, "activation") + require.NotEmpty(t, activationSection, "activation job section should exist") + assert.Contains(t, activationSection, "precompute", "activation should depend on precompute") + + // config should NOT be in activation's needs (it depends on activation itself) + assert.NotContains(t, activationSection[:strings.Index(activationSection, "runs-on")], + "config", "activation should NOT depend on config") + + // The substitution step in the activation job should reference precompute's outputs + substIdx := strings.Index(lockStr, "- name: Substitute placeholders") + require.Positive(t, substIdx) + substSection := lockStr[substIdx:] + if nextStep := strings.Index(substSection[50:], "- name:"); nextStep > 0 { + substSection = substSection[:50+nextStep] + } + assert.Contains(t, substSection, "GH_AW_NEEDS_PRECOMPUTE_OUTPUTS_ACTION", + "Substitution step should reference precompute's output (it runs before activation)") + assert.NotContains(t, substSection, "GH_AW_NEEDS_CONFIG_OUTPUTS_RELEASE_TAG", + "Substitution step should NOT reference config's output (it runs after activation)") +}