Add detection job skip condition based on safe outputs and patches#3689
Add detection job skip condition based on safe outputs and patches#3689
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes CI resource usage by adding conditional logic to skip the threat detection job when there's nothing to scan (no safe outputs and no patches). The implementation adds a new has_patch output from the agent job and uses it along with existing output_types to determine whether the detection job should run.
Key changes:
- Added patch detection logic in JavaScript that checks for
/tmp/gh-aw/aw.patchexistence - Exposed
has_patchas a new job output alongsideoutputandoutput_types - Added conditional expression to detection job using OR logic: skip when both no safe outputs AND no patch
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/js/collect_ndjson_output.cjs |
Added patch file existence check and set has_patch output |
pkg/workflow/compiler_jobs.go |
Added has_patch to job outputs map |
pkg/workflow/threat_detection.go |
Implemented conditional logic using BuildDisjunction to skip detection job when not needed |
pkg/workflow/threat_detection_test.go |
Added comprehensive test verifying the new skip condition |
pkg/workflow/agentic_output_test.go |
Updated existing tests to verify has_patch output is declared |
.github/workflows/dev.lock.yml |
Generated workflow reflecting the new output and conditional |
.github/workflows/changeset.lock.yml |
Generated workflow reflecting the new output and conditional |
.github/workflows/technical-doc-writer.lock.yml |
Unrelated change: simplified agent path reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set -o pipefail | ||
| sudo -E awf --env-all --allow-domains '*.githubusercontent.com,api.enterprise.githubcopilot.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com' --log-level info \ | ||
| "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent \"\${GITHUB_WORKSPACE}/.github/agents/technical-doc-writer.md\" --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ | ||
| "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent technical-doc-writer --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ |
There was a problem hiding this comment.
This change appears unrelated to the PR's stated purpose of adding detection job skip conditions. The change from --agent \"\${GITHUB_WORKSPACE}/.github/agents/technical-doc-writer.md\" to --agent technical-doc-writer modifies how the agent path is specified but has no connection to patch detection or safe outputs. Consider moving this to a separate PR focused on agent path handling.
| "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent technical-doc-writer --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ | |
| "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent \"${GITHUB_WORKSPACE}/.github/agents/technical-doc-writer.md\" --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ |
The detection job runs unconditionally after the agent job, wasting CI resources when there's nothing to scan (no safe outputs and no patch).
Changes
JavaScript (
collect_ndjson_output.cjs)/tmp/gh-aw/aw.patchexistence and sethas_patchoutputGo Compiler (
compiler_jobs.go)has_patchoutput from agent job alongside existingoutputandoutput_typesDetection Job (
threat_detection.go)needs.agent.outputs.output_types != '' || needs.agent.outputs.has_patch == 'true'Tests
TestDetectionJobSkipConditionto verify conditional logicExample
Generated workflow now includes:
Detection job now skips when there's nothing to analyze, reducing unnecessary workflow executions.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.