Skip to content

feat(detector): honor GH_AW_DETECTION_CONTINUE_ON_ERROR for missing artifacts - #751

Merged
davidslater merged 2 commits into
mainfrom
ace/01KZ7EM68TJ69JN4FYVN0R2CRX
Aug 5, 2026
Merged

davidslater merged 2 commits into
mainfrom
ace/01KZ7EM68TJ69JN4FYVN0R2CRX

Conversation

@davidslater

Copy link
Copy Markdown
Collaborator

Created by GitHub Ace · View Session

Closes #720

Parity follow-up for the gh-aw v0.84.2 scan in #720.

Triage of the two flagged changes

  • gh-aw#49454 — model propagation into the external detection path: no change needed here. The fix is entirely in gh-aw's compiler (buildExternalDetectorExecutionStep), which now copies Model/ModelMappings/DefaultAiCreditsPricing into the detection job. The detector already consumes what it produces: --model plus the GH_AW_MODEL_DETECTION_* / COPILOT_MODEL fallbacks in engine.ResolveModel.
  • gh-aw#49415 — GH_AW_DETECTION_CONTINUE_ON_ERROR in detection setup: parity gap, addressed here. That PR taught gh-aw's setup_threat_detection.cjs to warn instead of hard-failing when agent_output.json is missing, or when a patch is expected (HAS_PATCH=true) but absent — and to still fail in strict mode. This repo owns that setup logic in Go going forward, but threat-detect did neither half: it silently proceeded with "No agent output file found" placeholders and ignored both env vars.

Changes

  • pkg/artifacts: Load now records MissingRequired — the expected-but-absent host-staged artifacts (agent_output.json → ERR_SYSTEM, aw-prompts/prompt.txt → ERR_VALIDATION, and a missing aw-*.patch/aw-*.bundle when HAS_PATCH=true → ERR_VALIDATION). Loading never fails on these; the caller decides.
  • cmd/threat-detect: warn mode (default, GH_AW_DETECTION_CONTINUE_ON_ERROR != "false") emits a ::warning:: per entry and continues detection with whatever was staged; strict mode emits ::error:: and returns config_error / exit 2 before the engine is invoked. A new artifacts_missing_required JSONL event records the entries and resolved mode.
  • The GH_AW_DETECTION_CONTINUE_ON_ERROR interpretation is now a single shared helper used by both the detection run and conclude (no behavior change to conclude).
  • Spec: new TD-18c and U-06b; README input section and env table updated.

Verification

make fmt-check lint build test pass. make security-scan and make golint fail identically on main (38 pre-existing gosec findings; golangci-lint binary built with go1.25 vs. the go1.26 target).

…rtifacts

Mirrors gh-aw#49415 in the Go detector so it no longer depends on gh-aw's
JS setup step for required-artifact validation. Load now records the
expected-but-absent artifacts (agent_output.json, aw-prompts/prompt.txt,
and a patch/bundle when HAS_PATCH=true) and the CLI warns and continues in
the default warn mode, or fails with config_error in strict mode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 4, 2026 22:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds host-controlled handling for missing threat-detection artifacts, aligning the Go detector with gh-aw setup behavior.

Changes:

  • Tracks required-but-missing artifacts.
  • Adds warn/strict handling, diagnostics, and JSONL logging.
  • Updates specifications, documentation, and tests.
Show a summary per file
File Description
specs/usage-spec.md Documents host integration behavior.
specs/threat-detection-spec.md Defines missing-artifact requirements.
README.md Documents inputs, variables, and logging.
pkg/artifacts/artifacts.go Detects missing required artifacts.
pkg/artifacts/artifacts_test.go Tests artifact detection.
gosec-report.json Adds a generated scanner report.
cmd/threat-detect/main.go Implements warn and strict modes.
cmd/threat-detect/main_test.go Tests CLI behavior.
cmd/threat-detect/conclude.go Reuses the policy helper.

Review details

Tip

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

  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread cmd/threat-detect/main.go Outdated
// detection run and the conclude subcommand: anything other than "false" is
// warn mode, so an unset variable defaults to warning rather than failing.
func detectionContinueOnError() bool {
return os.Getenv("GH_AW_DETECTION_CONTINUE_ON_ERROR") != "false"
Comment thread gosec-report.json Outdated
@@ -0,0 +1,593 @@
{
Reworks the required-artifact policy on top of #750's degraded-input
warnings instead of a parallel MissingRequired list: ArtifactWarning now
carries RequiredInput, and strict mode
(GH_AW_DETECTION_CONTINUE_ON_ERROR="false") turns those findings into
errors plus a config_error exit while advisory findings stay warnings.

Also addresses review feedback: the continue-on-error comparison is now
case-insensitive (matching gh-aw's setup), and the accidentally committed
gosec-report.json is removed and gitignored along with the other
`make clean` generated reports.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[gh-aw-parity] gh-aw threat-detection changes to review - 2026-08-02

2 participants