Repository navigation
feat(conclude): distinguish threat detection engine failures from real findings - #752
Merged
Merged
Conversation
Mirrors gh-aw #49527 and #49497: tooling failures (agent_failure, parse_error) now carry the <!-- gh-aw-threat-engine-error --> marker and a "Threat Detection Engine Failure" title in the verdict step summary and job log, while real verdicts keep <!-- gh-aw-threat-detected -->. Adds shared reason constants/helpers in pkg/detector, spec rule TD-20i, and documents that --model may be a gh-aw alias (gh-aw #49586). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns standalone conclusion reporting with gh-aw by distinguishing engine failures from genuine security findings.
Changes:
- Adds shared reason, marker, and headline helpers.
- Updates summaries and logs with failure-specific messaging.
- Documents and tests the revised contract.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents markers and model aliases. |
specs/threat-detection-spec.md |
Defines normative reason-to-marker mapping. |
pkg/detector/reason.go |
Adds reason classification and rendering helpers. |
pkg/detector/reason_test.go |
Tests reason helpers. |
pkg/detector/summary.go |
Renders differentiated verdict summaries. |
pkg/detector/summary_test.go |
Tests markers, titles, and headlines. |
cmd/threat-detect/conclude.go |
Applies shared reasons and log headlines. |
cmd/threat-detect/conclude_test.go |
Tests tooling-failure and threat paths. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Balanced
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #732
Parity follow-up for #732 (gh-aw v0.84.3).
What gh-aw changed
agent_failure,parse_error) get a distinct<!-- gh-aw-threat-engine-error -->marker instead of<!-- gh-aw-threat-detected -->detectionaliasthreat-detectthroughCOPILOT_MODEL/GH_AW_MODEL_DETECTION_*and is forwarded verbatim; aliases are resolved by the AWF API proxy. Documented only.Changes
pkg/detector/reason.go(new) — shared host-side reason constants (ReasonThreatDetected,ReasonAgentFailure,ReasonParseError), theThreatDetectedMarker/ThreatEngineErrorMarkermarkers mirrored from gh-aw, plusIsToolingFailureReason,ThreatMarker, andThreatHeadline.pkg/detector/summary.go—FormatVerdictSummarynow emits the marker matching the reason, titles the blockThreat Detection Engine Failurefor tooling failures (Threat Detection Verdictotherwise), and adds the one-line headline. Clean outcomes (success,skipped) carry no marker.cmd/threat-detect/conclude.go— echoes the same headline into the job log, and reusesdetector.IsToolingFailureReasonplus the reason constants instead of duplicating string literals.specs/threat-detection-spec.md— new normative rule TD-20i defining the reason → marker/headline mapping.README.md— documents the marker/title table and that--modelmay be a gh-aw alias.concludepaths (tooling failure vs real verdict).Rendered output for a missing verdict:
Validation
make fmt lint build test— all green; manually smoke-tested bothconcludepaths.