feat: allow safe-outputs.threat-detection to be controlled by workflow_call inputs

[Copilot]

Reusable workflow_call workflows couldn't parameterize threat detection at runtime — threat-detection only accepted compile-time boolean/object values, forcing separate workflow files for detection-on vs detection-off configurations.

Changes

Schema

• safe-outputs.threat-detection now accepts a GHA expression string (top-level)
• Object-form enabled and continue-on-error now use templatable_boolean (bool literal or expression)

Model & parsing (ThreatDetectionConfig)

• Added EnabledExpr *string and ContinueOnErrorExpr *string for expression forms
• Added IsConditional() / IsConditionalDetection() helpers
• parseThreatDetectionConfig handles expression strings at both the top level and inside the object form; object parsing extracted to parseThreatDetectionObjectConfig

Compilation

• When EnabledExpr != nil, the detection job's if: includes the raw caller expression — GitHub Actions evaluates it at runtime and skips the job when false
• safe_outputs, safe_jobs, and upload_assets jobs switch to always() && buildDetectionPassedCondition() (success || skipped) when detection is conditional, preventing them from being silently skipped when detection is skipped at runtime
• continue-on-error expression is emitted unquoted in both the step field and GH_AW_DETECTION_CONTINUE_ON_ERROR env var
• push_repo_memory and update_cache_memory already used buildDetectionPassedCondition() so they correctly handle conditional detection without code changes

Tests

• Unit tests verify push_repo_memory and update_cache_memory job conditions with conditional detection
• Integration test file threat_detection_job_combinations_integration_test.go covers all job types × detection modes (disabled / boolean-enabled / expression):
  • TestThreatDetectionCombinations — 13 subtests across safe_outputs, detection, push_repo_memory, update_cache_memory, and custom safe-jobs
  • TestWorkflowFilesCompile — compiles all fixture .md files and verifies correct job generation
  • TestRepoMemoryWithThreatDetectionNeedsAndConditions — parametric tests for push_repo_memory across all 3 detection modes
  • TestCacheMemoryWithThreatDetectionNeedsAndConditions — parametric tests for update_cache_memory across all 3 detection modes
• Six workflow fixtures added to pkg/cli/workflows/ documenting every combination

Example

on:
  workflow_call:
    inputs:
      enable-threat-detection:
        type: boolean
        default: true
safe-outputs:
  threat-detection: ${{ inputs.enable-threat-detection }}
  # Object form also works:
  # threat-detection:
  #   enabled: ${{ inputs.enable-threat-detection }}
  #   continue-on-error: ${{ inputs.detection-continue-on-error }}

Literal boolean and object forms remain fully backward compatible.

[!WARNING]

[Copilot]

Pull request overview

Adds runtime (workflow_call input) control over safe-outputs.threat-detection by allowing GitHub Actions expressions in the schema and compiler, and adjusts downstream job conditions to behave correctly when detection is skipped at runtime.

Changes:

• Extend workflow schema to accept expression strings for safe-outputs.threat-detection and templatable booleans for enabled / continue-on-error.
• Update threat detection parsing/model (ThreatDetectionConfig) to capture expression forms and compile them into the detection job’s if: condition.
• Update safe job conditions (and add tests) to handle conditional detection where the detection job can be skipped at runtime.

Show a summary per file

File	Description
pkg/workflow/threat_detection.go	Parses expression-based configs, tracks conditional detection, and injects the caller expression into the detection job if:.
pkg/workflow/safe_jobs_threat_detection_test.go	Adds test coverage for expression parsing and conditional detection job/condition behavior.
pkg/workflow/safe_jobs.go	Wraps safe-jobs if: with always() + detection-passed gating when detection is conditional.
pkg/workflow/publish_assets.go	Updates upload_assets job condition for conditional detection (but currently misses a required needs dependency).
pkg/workflow/compiler_safe_outputs_job.go	Adjusts safe_outputs job condition to run when detection is skipped due to runtime expression.
pkg/parser/schemas/main_workflow_schema.json	Schema updates to allow expression strings/templatable booleans for threat-detection configuration.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

pkg/workflow/publish_assets.go:196

• buildUploadAssetsJob builds jobCondition with buildDetectionPassedCondition() when detection is conditional, but the job's needs does not include the detection job. Since buildDetectionPassedCondition() references needs.detection.result, this will evaluate incorrectly (or error) at runtime. Add constants.DetectionJobName to needs whenever the condition can reference detection (e.g. when IsConditionalDetection(data.SafeOutputs) or more generally when threatDetectionEnabled is true).

	jobCondition := BuildSafeOutputType("upload_asset")
	if IsConditionalDetection(data.SafeOutputs) {
		jobCondition = BuildAnd(
			BuildAnd(BuildFunctionCall("always"), BuildSafeOutputType("upload_asset")),
			buildDetectionPassedCondition(),
		)
	}

	// Build job dependencies — always include activation job for OTLP trace ID correlation
	needs := []string{mainJobName, string(constants.ActivationJobName)}

• Files reviewed: 6/6 changed files
• Comments generated: 0

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	7
✅ Design tests (behavioral contracts)	7 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	7 (100%)
Duplicate test clusters	0
Test inflation detected	No (374 test lines / 202 production lines = 1.85:1)
🚨 Coding-guideline violations	None

---

Test Classification Details

View All 7 Tests

Test	File	Classification	Notes
TestParseThreatDetectionConfigExpression	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Table-driven; 5+ rows covering literal true/false, expression string, object forms
TestParseThreatDetectionContinueOnErrorExpression	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Table-driven; tests ContinueOnError and ContinueOnErrorExpr field parsing
TestIsConditionalDetection	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Table-driven; verifies IsConditionalDetection() behavioral contract
TestDetectionJobConditionWithExpression	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Verifies compiled detection job if: condition contains the expression
TestSafeOutputsJobConditionWithConditionalDetection	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Verifies safe_outputs job conditions — always(), skipped/success result checks
TestIsThreatDetectionExplicitlyDisabledExpressionNotDisabled	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	Edge case: expression string must NOT be treated as explicitly disabled
TestDetectionJobWithExpressionCompilation	pkg/workflow/safe_jobs_threat_detection_test.go	✅ Design	End-to-end compilation; verifies full YAML output contains correct conditions

---

Highlights

All 7 new tests are design tests that verify behavioral contracts:

• Table-driven tests (TestParseThreatDetectionConfigExpression, TestParseThreatDetectionContinueOnErrorExpression, TestIsConditionalDetection): Excellent use of the project's preferred table-driven pattern. Each table includes both happy-path rows and error/edge cases (literal false, nil pointers, expression vs. literal distinctions).
• Unit-level job builders (TestDetectionJobConditionWithExpression, TestSafeOutputsJobConditionWithConditionalDetection): Test observable outputs of internal builders — the compiled if: expression strings — not internal call counts.
• End-to-end compilation (TestDetectionJobWithExpressionCompilation): Compiles a real workflow to YAML and checks the output, providing the highest-confidence coverage.
• Critical edge case (TestIsThreatDetectionExplicitlyDisabledExpressionNotDisabled): Explicitly guards the subtle contract that an expression string is not equivalent to false — exactly the kind of regression this feature could accidentally introduce.

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 7 tests — unit (//go:build !integration)
• 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All 7 new tests enforce observable behavioral contracts with strong edge-case coverage.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25170616401

🧪 Test quality analysis by Test Quality Sentinel · ● 850.9K · ◷

[github-actions]

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 7 new tests enforce observable behavioral contracts with strong edge-case and error-path coverage.

[github-actions]

Commit pushed: 7b0a49c

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[pelikhan]

@copilot review all jobs that are gated by detection: push_repo_memory, upload_cache_memory as well

[Copilot]

@copilot review all jobs that are gated by detection: push_repo_memory, upload_cache_memory as well

Both jobs already handle conditional detection correctly without changes needed:

• push_repo_memory: when IsDetectionJobEnabled is true (which includes conditional/expression-based detection since HasRunnableDetection() returns true when EnabledExpr != nil), its condition is already always() && !cancelled() && (detection.result == 'success' || detection.result == 'skipped') && agent.result == 'success' — the skipped check ensures it runs when detection is disabled at runtime.
• update_cache_memory: only created when threatDetectionEnabled = true (same condition); its job condition is hardcoded to always() && (detection.result == 'success' || detection.result == 'skipped') && agent.result == 'success' — already correct.

Both already used buildDetectionPassedCondition() which accepts both success and skipped results, unlike safe_outputs/safe_jobs/upload_assets which previously only checked == 'success'.

Added TestPushRepoMemoryJobConditionalDetection and TestUpdateCacheMemoryJobConditionalDetection in commit 686cd99 to explicitly verify this behaviour.

[pelikhan]

@copilot ensure integration tests workflows in pkg/cli/workflows for every job type and combination of threat detection enablement or not

[Copilot]

@copilot ensure integration tests workflows in pkg/cli/workflows for every job type and combination of threat detection enablement or not

Added in commit ae8a7df:

6 workflow fixtures in pkg/cli/workflows/ covering every job type × detection variant:

• test-copilot-threat-detection-expression.md — object-form expression detection (enabled: + continue-on-error:)
• test-copilot-repo-memory-threat-detection.md — push_repo_memory + boolean detection
• test-copilot-repo-memory-threat-detection-expression.md — push_repo_memory + expression detection
• test-copilot-cache-memory-threat-detection.md — update_cache_memory + boolean detection
• test-copilot-cache-memory-threat-detection-expression.md — update_cache_memory + expression detection
• test-copilot-safe-jobs-threat-detection-expression.md — custom safe-job + expression detection

threat_detection_job_combinations_integration_test.go with:

• TestThreatDetectionCombinations — 13 subtests covering every combination of (safe_outputs, detection, push_repo_memory, update_cache_memory, safe_jobs) × (disabled/enabled/expression)
• TestWorkflowFilesCompile — compiles all 6 fixture .md files and verifies the correct jobs are generated
• TestRepoMemoryWithThreatDetectionNeedsAndConditions — parametric test for push_repo_memory across all 3 detection modes
• TestCacheMemoryWithThreatDetectionNeedsAndConditions — parametric test for update_cache_memory across all 3 detection modes