Move noop processing step from separate job into conclusion job#4318
Conversation
- Modified notify_comment.go to add noop processing step to conclusion job - Added noop_message output to conclusion job when noop is configured - Removed noop job generation from compiler_jobs.go - Verified all test workflows compile correctly with new structure - All unit tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Created noop_in_conclusion_test.go to validate the new structure - Test verifies no separate noop job exists - Test verifies noop step is in conclusion job - Test verifies conclusion job has noop_message output - Test verifies conclusion job does not depend on noop job - Test passes successfully Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Removed buildCreateOutputNoOpJob function that is no longer used - Removed unused fmt import - Noop job is now generated inside conclusion job instead - All linter checks pass - All unit tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the noop (no-operation) processing by consolidating it into the conclusion job, eliminating a separate noop job. This simplifies the workflow structure while maintaining all functionality.
Key changes:
- Noop step generation moved to
buildConclusionJobinnotify_comment.go - Removed noop job generation code from
compiler_jobs.go - Added comprehensive test coverage in
noop_in_conclusion_test.go
Reviewed Changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pkg/workflow/notify_comment.go |
Adds noop processing step to conclusion job with proper environment variables and outputs |
pkg/workflow/compiler_jobs.go |
Removes dedicated noop job generation code |
pkg/workflow/noop_in_conclusion_test.go |
Adds unit tests verifying noop step placement in conclusion job |
.github/workflows/*.lock.yml |
All compiled workflows updated with noop step now in conclusion job |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| find "/tmp/gh-aw/safeoutputs/" -type f -print | ||
| echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/safeoutputs/agent_output.json" >> "$GITHUB_ENV" | ||
| - name: Process No-Op Messages |
There was a problem hiding this comment.
The noop step is placed after a 'Setup agent output environment variable' step, but then followed by another identical artifact download and setup at lines 4542-4552. This creates duplicate artifact downloads and environment setup. The noop step should be moved after the second artifact download/setup block to avoid running before artifacts are properly downloaded.
| - name: Download agent output artifact | ||
| continue-on-error: true | ||
| uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6 | ||
| with: | ||
| name: agent_output.json | ||
| path: /tmp/gh-aw/safeoutputs/ | ||
| - name: Setup agent output environment variable | ||
| run: | | ||
| mkdir -p /tmp/gh-aw/safeoutputs/ | ||
| find "/tmp/gh-aw/safeoutputs/" -type f -print | ||
| echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/safeoutputs/agent_output.json" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Duplicate artifact download and environment setup steps appear after the noop processing step. This redundancy suggests the noop step was inserted in the wrong location. These duplicate steps at lines 4542-4552 appear to be unnecessary since the same setup already occurred at lines 4448-4456 before the noop step.
|
@copilot avoid generate two artifact download steps for the same resource |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Agentic Changeset Generator completed successfully. |
- Added buildGitHubScriptStepWithoutDownload helper function - Artifact download steps now added once at beginning of conclusion job - Both noop and conclusion steps share the same artifact download - Eliminates duplicate "Download agent output artifact" steps - All workflows compile successfully with no duplicate downloads Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in d42afa8. The artifact download steps are now added once at the beginning of the conclusion job and shared by both the noop and conclusion steps. Created a new 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 the conclusion job has duplicate steps to download and setup artifacts. Remove the later ones |
The duplicate artifact download steps have already been removed in d42afa8. The conclusion job now has only one set of download/setup steps that are shared by both the noop and conclusion script steps. The current state shows:
No further changes needed. |
Move noop processing step from separate job into conclusion job
Consolidates workflow structure by generating the noop JavaScript step inside the conclusion job instead of as a separate job.
Changes
buildConclusionJob(), including environment variables, script invocation, andnoop_messageoutput. Artifact download steps are now added once at the beginning and shared by both noop and conclusion steps.safeOutputJobNamesbuildCreateOutputNoOpJob()functionbuildGitHubScriptStepWithoutDownload()helper function for building script steps without duplicate artifact downloadsStructure Change
Before:
After:
The noop processing logic and JavaScript remain unchanged—only the job structure is simplified and artifact downloads are no longer duplicated.
Fix Details
Fixed duplicate artifact download issue where both the noop step and the conclusion step were independently adding artifact download steps. Now:
buildGitHubScriptStepWithoutDownload()helper for subsequent stepsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.