Skip to content

Detection job should be skipped at job-level when there is nothing to detect against #23183

@davidslater

Description

@davidslater

Problem

When threat detection is enabled and the agent job produces no outputs and no patch, the detection job still runs — it just burns a job runner to spin up, execute a guard step, skip all substantive steps, and conclude skipped. More critically, safe_outputs still runs even though the agent produced nothing useful.

Current behavior

The detection job has this job-level condition (generated in pkg/workflow/threat_detection.go line 613):

if: always() && needs.agent.result != 'skipped'

Inside the job, a guard step (detection_guard) checks whether there is anything to detect:

if [[ -n "$OUTPUT_TYPES" || "$HAS_PATCH" == "true" ]]; then
  echo "run_detection=true" >> "$GITHUB_OUTPUT"
else
  echo "run_detection=false" >> "$GITHUB_OUTPUT"
fi

When run_detection=false, all substantive steps are individually skipped via if: steps.detection_guard.outputs.run_detection == 'true'. The parse/conclusion step (parse_threat_detection_results.cjs) then sets:

conclusion = "skipped"
success    = "true"

The detection job exits 0, so its result = success.

Why this is a problem

The safe_outputs job condition is:

if: (!cancelled()) && needs.agent.result != 'skipped' && needs.detection.result == 'success'

Because the detection job always exits with success (even when it has nothing to detect), safe_outputs always runs when the agent ran — even when the agent produced zero outputs and there is nothing to publish.


Root cause

Three levels of "skip" exist for detection today:

Level Mechanism Trigger
1 Job not created at compile time threat-detection: false in frontmatter
2 Job-level if: is false Agent job was skipped
3 Individual steps inside the job are skipped run_detection=false from the guard

Level 3 does step-level skipping only — the job still runs and exits successfully, letting safe_outputs proceed when it shouldn't.


Proposed solution

1. Update the detection job-level condition

In pkg/workflow/threat_detection.go, update buildDetectionJob() (line ~613):

Current:

jobCondition := fmt.Sprintf("always() && needs.%s.result != 'skipped'", constants.AgentJobName)

Proposed:

jobCondition := fmt.Sprintf(
    "always() && needs.%s.result != 'skipped' && (needs.%s.outputs.output_types != '' || needs.%s.outputs.has_patch == 'true')",
    constants.AgentJobName, constants.AgentJobName, constants.AgentJobName,
)

This causes the detection job to be skipped (result = skipped, not success) when the agent produced nothing. The agent job already exposes these outputs for safe-outputs workflows (pkg/workflow/compiler_main_job.go lines 167–169).

2. Keep the safe_outputs condition unchanged

The buildDetectionSuccessCondition() function (compiler_safe_outputs_job.go line 544) currently returns:

needs.detection.result == 'success'

Do not change this. With detection now truly skipped (result = skipped), safe_outputs will correctly be skipped too — it has nothing to publish anyway.

3. Update other downstream job conditions to accept skipped

Jobs that need to run regardless of whether detection skipped (unlock, conclusion, cache-memory) currently check needs.detection.result == 'success'. Update them to also accept skipped:

needs.detection.result == 'success' || needs.detection.result == 'skipped'

Files to review: pkg/workflow/compiler_unlock_job.go, pkg/workflow/cache.go, pkg/workflow/safe_jobs.go.

Files to change

File Change
pkg/workflow/threat_detection.go Add output_types != '' || has_patch == 'true' to job-level condition
pkg/workflow/compiler_safe_outputs_job.go No change — needs.detection.result == 'success' is correct
pkg/workflow/compiler_unlock_job.go Accept skipped detection result
pkg/workflow/cache.go Accept skipped detection result
pkg/workflow/safe_jobs.go Accept skipped detection result for custom safe jobs that must run
Lock files Run make recompile after code changes

How to test

Unit tests

  1. pkg/workflow/threat_detection_test.go — add a test asserting the generated detection job if: condition contains:

    needs.agent.outputs.output_types != '' || needs.agent.outputs.has_patch == 'true'
    
  2. pkg/workflow/detection_success_test.go — add a test verifying that when needs.detection.result == 'skipped', the safe_outputs condition evaluates to false (job does not run).

  3. pkg/workflow/compiler_safe_outputs_job_test.go — verify buildDetectionSuccessCondition() still returns exactly needs.detection.result == 'success' (no change, just regression check).

Compile and inspect generated YAML

./gh-aw compile my-workflow.md
grep -A5 "detection:" .github/workflows/my-workflow.lock.yml

Verify the detection job if: block contains the output-type check.

Runtime test (manual)

Trigger a workflow run where the agent produces no outputs:

  1. Detection job → result = skipped in Actions UI
  2. safe_outputs job → result = skipped in Actions UI (blocked by needs.detection.result == 'success')

Acceptance criteria

  • Detection job is skipped (result = skipped) when agent produces no outputs and no patch
  • safe_outputs job is skipped when detection job is skipped
  • Detection job still runs normally when agent produces outputs or a patch
  • safe_outputs still runs when detection job succeeds (clean outputs)
  • safe_outputs is still skipped when threats are detected (detection job fails)
  • Unlock, conclusion, and cache-memory jobs still run correctly (their conditions accept skipped detection result)
  • All existing unit tests pass (make test-unit)
  • Lock files are recompiled (make recompile)

Metadata

Metadata

Labels

security:medium-severityA security issue that should be addressed within one week.

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions