Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- 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>
needs: declarations causing 22 actionlint expression errors in 4 workflows
.github/workflows/release.md
Outdated
| jobs: | ||
| config: | ||
| needs: ["activation"] | ||
| needs: ["pre_activation"] |
There was a problem hiding this comment.
Done in 67ab7dc. Both config and release now have needs: [pre_activation, activation] and needs: [pre_activation, activation, config] respectively. The compiler was also updated to prevent a circular dependency (getCustomJobsDependingOnPreActivation now skips jobs that also depend on activation) and to filter post-activation job expressions from the activation prompt steps.
.github/workflows/release.md
Outdated
| console.log(`✓ Release tag: ${releaseTag}`); | ||
| release: | ||
| needs: ["activation", "config"] | ||
| needs: ["pre_activation", "config"] |
There was a problem hiding this comment.
Pull request overview
This PR fixes 22 actionlint expression errors across 4 workflow files by correcting job dependency declarations and enhancing the compiler to automatically detect and establish proper job ordering when custom job outputs are referenced in activation prompts.
Changes:
- Enhanced compiler logic to automatically add custom jobs to activation's
needs:when their outputs are referenced in the markdown body, preventing empty substitutions and actionlint errors - Updated compiler to skip auto-adding
needs: activationto jobs whose outputs are used in activation prompts, as these must run before activation - Fixed circular dependency in release.md by changing
configandreleasejobs from depending onactivationtopre_activation
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/known_needs_expressions.go | Added logic to skip generating generic "output" env var for jobs with explicit outputs that don't include "output", preventing actionlint type errors |
| pkg/workflow/compiler_jobs.go | Added getCustomJobsReferencedInPromptWithNoActivationDep() to identify jobs that need to run before activation, and updated buildCustomJobs() to skip auto-adding activation dependency for these jobs |
| pkg/workflow/compiler_activation_jobs.go | Enhanced buildActivationJob() to automatically add prompt-referenced jobs to activation dependencies, and updated buildMainJob() to include CustomSteps when analyzing job references |
| .github/workflows/smoke-gemini.lock.yml | Incidental fix: corrected experimental flag from true to false for Gemini engine (recompilation side effect) |
| .github/workflows/release.md | Fixed circular dependency by changing config and release jobs to depend on pre_activation instead of activation |
| .github/workflows/release.lock.yml | Recompiled with correct job ordering: pre_activation → config → release → activation → agent |
| .github/workflows/issue-monster.lock.yml | Removed generic "output" env var for search_issues job which declares specific outputs without "output" |
| .github/workflows/hourly-ci-cleaner.lock.yml | Fixed activation to depend on check_ci_status (referenced in prompt), removed check_ci_status's incorrect dependency on activation |
| .github/workflows/bot-detection.lock.yml | Fixed activation to depend on precompute (referenced in prompt), removed precompute's incorrect dependency on activation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| agent_version: "", | ||
| workflow_name: "Smoke Gemini", | ||
| experimental: true, | ||
| experimental: false, |
There was a problem hiding this comment.
The change from experimental: true to experimental: false for the Gemini engine appears to be unrelated to the PR's stated objective of fixing missing needs: declarations. While this is technically correct (the Gemini engine is defined as non-experimental in pkg/workflow/gemini_engine.go line 23), it should ideally be mentioned in the PR description or separated into its own commit for clarity.
This change likely occurred because the file was recompiled as part of fixing the other workflows, and the previous lock file had an incorrect value. Consider adding a note about this incidental fix in the PR description.
…mpiler 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>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. 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 add tests |
…ctivation, and updated getCustomJobsDependingOnPreActivation 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>
Added in 32c1977. 21 new test cases across two files:
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:
|
Jobs in 4 workflows referenced
needs.X.outputs.*in activation job prompt steps withoutXinactivation'sneeds:, causing actionlint[expression]errors and incorrect prompt substitutions (empty values at activation time since the referenced jobs hadn't run yet).Compiler fixes
buildActivationJob: When a custom job (no explicitneeds:) has outputs referenced in the markdown body, automatically add it toactivationNeedsso it runs before activationbuildCustomJobs: Skip auto-addingneeds: activationto jobs whose outputs are referenced in the markdown body — they should run independently before activationbuildMainJob: Also searchdata.CustomSteps(frontmattersteps:section) when computing direct dependencies for the agent job, not just the markdown bodygenerateKnownNeedsExpressions: Don't emit the genericoutputenv var for jobs that declare explicit outputs not includingoutput(was causing actionlint to flag undefined properties on known output types)jobDependsOnActivation(): New helper to detect custom jobs that explicitly depend onactivationgetCustomJobsDependingOnPreActivation(): Now excludes jobs that also depend onactivation, preventing them from being incorrectly added to activation'sneeds:(which would create a cycle)generatePrompt()/generatePromptInActivationJob(): Accept abeforeActivationJobs []stringparameter and filter expression mappings so that outputs from post-activation jobs are not embedded in activation's step env vars — preventing actionlint errors when a custom job explicitly gates onactivationcompletingfilterExpressionsForActivation(): New helper that removes expression mappings referencing custom jobs that run after activation from the activation job's prompt generation stepsBefore the fix,
precomputeauto-receivedneeds: activation, so activation's prompt ran beforeprecomputeand substituted empty values. After:Source fix (
release.md)configandreleasejobs now depend on bothpre_activationandactivation, establishing the correct ordering:pre_activation → activation → config → release → agent. This ensures the activation gate (permission check) must pass before these jobs run, while the compiler correctly avoids adding them to activation's ownneeds:(which would create a cycle).Tests
New tests covering all new and updated compiler functions:
TestJobDependsOnActivation— 8 cases covering string, array, and invalidneedsinputs for the new helperTestGetCustomJobsDependingOnPreActivationExcludesActivationDependents— 4 cases verifying jobs with bothpre_activationandactivationin needs are excluded from the pre-activation set (preventing circular dependencies)TestBuildCustomJobsDoesNotAutoAddActivationToOutputReferencedJobs— verifies the compiler skips auto-addingneeds: activationwhen a job's outputs are referenced in the markdown bodyTestFilterExpressionsForActivation— 8 unit cases covering nil customJobs, empty mappings, keep/filter bybeforeActivationJobs, non-custom-jobneeds.*expressions, non-needs.*expressions, and malformed expressionsTestFilterExpressionsForActivationEndToEnd— integration test compiling a full workflow withprecompute(before activation) andconfig(after activation), asserting activation depends onprecomputebut notconfig, and the substitution step only referencesprecompute's outputsOriginal prompt
needs:declarations in 4 workflow files (22 actionlint expression errors) #17669💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.