Keep missing-verdict warnings visible and avoid unresolved detection …

…model aliases Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026
commit 35d1d77511c18a831a154524cc573d7696164198
diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go
@@ -20,7 +20,7 @@ var notifyCommentLog = logger.New("workflow:notify_comment")
 // This job runs when:
 // 1. always() - runs even if agent fails
 // 2. Agent job was not skipped
-// 3. NO add_comment output was produced by the agent (avoids duplicate updates)
+// 3. NO add_comment output was produced, or detection produced no verdict
 // This job depends on all safe output jobs to ensure it runs last
 func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, safeOutputJobNames []string) (*Job, error) {
 	notifyCommentLog.Printf("Building conclusion job: main_job=%s, safe_output_jobs_count=%d", mainJobName, len(safeOutputJobNames))
@@ -84,9 +84,13 @@ func (c *Compiler) buildConclusionJobSteps(data *WorkflowData, mainJobName strin
 	// Only add the conclusion update step if status comments are explicitly enabled
 	if data.StatusComment != nil && *data.StatusComment {
 		var token string
+		var stepCondition string
 		if data.SafeOutputs != nil && data.SafeOutputs.AddComments != nil {
 			token = data.SafeOutputs.AddComments.GitHubToken
 		}
+		if slices.Contains(safeOutputJobNames, "add_comment") {
+			stepCondition = "always() && !(needs.add_comment.outputs.comment_id)"
+		}
 		steps = append(steps, c.buildGitHubScriptStepWithoutDownload(data, GitHubScriptStepConfig{
 			StepName:      "Update reaction comment with completion status",
 			StepID:        "conclusion",
@@ -95,6 +99,7 @@ func (c *Compiler) buildConclusionJobSteps(data *WorkflowData, mainJobName strin
 			Script:        getNotifyCommentErrorScript(),
 			ScriptFile:    "notify_comment_error.cjs",
 			CustomToken:   token,
+			StepCondition: stepCondition,
 		})...)
 	}
 	steps = append(steps, c.buildConclusionSteeringIssueStep(data, mainJobName, steeringToken)...)

diff --git a/pkg/workflow/notify_comment_conclusion_helpers.go b/pkg/workflow/notify_comment_conclusion_helpers.go
@@ -14,6 +14,21 @@ import (
 
 const conclusionWorkQueueActivationArtifactDir = "${{ runner.temp }}/gh-aw-activation"
 
+func buildDetectionNoVerdictCondition() ConditionNode {
+	job := "needs." + string(constants.DetectionJobName)
+	reason := BuildPropertyAccess(job + ".outputs.detection_reason")
+	return BuildAnd(
+		BuildNotEquals(BuildPropertyAccess(job+".result"), BuildStringLiteral("skipped")),
+		BuildOr(
+			BuildEquals(reason, BuildStringLiteral("agent_failure")),
+			BuildOr(
+				BuildEquals(reason, BuildStringLiteral("parse_error")),
+				BuildEquals(BuildPropertyAccess(job+".outputs.detection_conclusion"), BuildStringLiteral("")),
+			),
+		),
+	)
+}
+
 // buildConclusionSetupSteps extracts the common setup, token minting, and artifact steps.
 func (c *Compiler) buildConclusionSetupSteps(data *WorkflowData) []string {
 	var steps []string
@@ -58,7 +73,7 @@ func (c *Compiler) buildConclusionSetupSteps(data *WorkflowData) []string {
 	if IsDetectionJobEnabled(data.SafeOutputs) {
 		steps = append(steps,
 			"      - name: Warn if threat detection produced no verdict\n",
-			"        if: always() && needs.detection.result != 'skipped' && (needs.detection.outputs.detection_reason == 'agent_failure' || needs.detection.outputs.detection_reason == 'parse_error' || needs.detection.outputs.detection_conclusion == '')\n",
+			fmt.Sprintf("        if: %s\n", RenderCondition(BuildAnd(BuildFunctionCall("always"), buildDetectionNoVerdictCondition()))),
 			"        run: echo \"::warning::Threat detection produced no security verdict. Review the detection job logs before trusting the agent outputs.\"\n",
 		)
 		steps = append(steps, buildDetectionArtifactDownloadSteps(artifactPrefixExprForDownstreamJob(data), c.getActionPin)...)
@@ -590,7 +605,11 @@ func (c *Compiler) buildConclusionJobCondition(data *WorkflowData, mainJobName s
 	condition := BuildAnd(alwaysFunc, BuildOr(agentNotSkipped, activationGuardrailsFailed))
 
 	if slices.Contains(safeOutputJobNames, "add_comment") {
-		return BuildAnd(condition, &NotNode{Child: BuildPropertyAccess("needs.add_comment.outputs.comment_id")})
+		noComment := ConditionNode(&NotNode{Child: BuildPropertyAccess("needs.add_comment.outputs.comment_id")})
+		if IsDetectionJobEnabled(data.SafeOutputs) {
+			noComment = BuildOr(noComment, buildDetectionNoVerdictCondition())
+		}
+		return BuildAnd(condition, noComment)
 	}
 	return condition
 }

diff --git a/pkg/workflow/threat_detection_engine_env_test.go b/pkg/workflow/threat_detection_engine_env_test.go
@@ -29,6 +29,19 @@ func TestConclusionWarnsWhenDetectionHasNoVerdict(t *testing.T) {
 		if !strings.Contains(steps, want) {
 			t.Errorf("expected conclusion steps to contain %q", want)
 		}
+		data.SafeOutputs.AddComments = &AddCommentsConfig{}
+		enabled := true
+		data.StatusComment = &enabled
+		job, err := compiler.buildConclusionJob(data, "agent", []string{"add_comment"})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !strings.Contains(job.If, "!(needs.add_comment.outputs.comment_id) || needs.detection.result != 'skipped'") {
+			t.Fatal("missing verdict must allow conclusion even after publishing a comment")
+		}
+		if !strings.Contains(strings.Join(job.Steps, ""), "if: always() && !(needs.add_comment.outputs.comment_id)") {
+			t.Fatal("status-comment update must still suppress duplicate comments")
+		}
 	}
 	data.SafeOutputs.ThreatDetection = nil
 	steps = strings.Join(compiler.buildConclusionSetupSteps(data), "")

diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go
@@ -36,7 +36,7 @@ func (c *Compiler) buildPrepareDetectionEngineConfigForExternalDetectorStep(data
 		codexLogsDir = path.Join(codexHomeDir, "logs")
 	}
 	detectionData := buildExternalDetectorWorkflowData(data, "codex")
-	detectionData.Model = inheritedDetectionModel(data)
+	detectionData.Model = inheritedDetectionModel(data, engineID)
 	if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.Model != "" {
 		detectionData.Model = data.SafeOutputs.ThreatDetection.Model
 	}
@@ -462,7 +462,7 @@ func (c *Compiler) buildExternalDetectorExecutionStep(data *WorkflowData) []stri
 	// ${{ vars.GH_AW_MODEL_DETECTION_COPILOT || ... || 'auto' }}, and when no org var
 	// is set COPILOT_MODEL is 'auto'. The AWF API proxy has no pricing for 'auto' and
 	// returns HTTP 400, causing every inference attempt to fail.
-	resolvedDetectionModel := inheritedDetectionModel(data)
+	resolvedDetectionModel := inheritedDetectionModel(data, engineID)
 	if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.Model != "" {
 		resolvedDetectionModel = data.SafeOutputs.ThreatDetection.Model
 	}

diff --git a/pkg/workflow/threat_detection_external_detector_config_test.go b/pkg/workflow/threat_detection_external_detector_config_test.go
@@ -19,7 +19,8 @@ func TestDetectionProviderSettingsIsolatedAcrossEngines(t *testing.T) {
 			}
 			t.Run(mainEngine+" to "+detectionEngine, func(t *testing.T) {
 				data := &WorkflowData{
-					AI: mainEngine,
+					AI:    mainEngine,
+					Model: "main-alias",
 					EngineConfig: &EngineConfig{
 						ID:        mainEngine,
 						APITarget: "main-provider.example.com",
@@ -32,7 +33,8 @@ func TestDetectionProviderSettingsIsolatedAcrossEngines(t *testing.T) {
 						},
 					},
 					ModelMappings: map[string][]string{
-						"sonnet": {"main-only-model"},
+						"sonnet":     {"main-only-model"},
+						"main-alias": {"main-only-model"},
 					},
 					SafeOutputs: &SafeOutputsConfig{
 						ThreatDetection: &ThreatDetectionConfig{
@@ -71,7 +73,7 @@ func TestDetectionProviderSettingsIsolatedAcrossEngines(t *testing.T) {
 				} {
 					t.Run(name, func(t *testing.T) {
 						steps := strings.Join(build(data), "")
-						for _, forbidden := range []string{"main-provider.example.com", "MAIN_PROVIDER_KEY", "main-only", "main-only-model"} {
+						for _, forbidden := range []string{"main-provider.example.com", "MAIN_PROVIDER_KEY", "main-only", "main-only-model", "main-alias"} {
 							if strings.Contains(steps, forbidden) {
 								t.Errorf("main provider setting %q leaked into detection", forbidden)
 							}

diff --git a/pkg/workflow/threat_detection_helpers.go b/pkg/workflow/threat_detection_helpers.go
@@ -276,11 +276,15 @@ func engineCoreSecretVarNames(engineID string) []string {
 // interpret it. Workflows using a custom engine run detection on a built-in engine that
 // does not understand the custom engine's model IDs, so their model is not inherited and
 // the detection engine's own default model is used instead.
-func inheritedDetectionModel(data *WorkflowData) string {
+func inheritedDetectionModel(data *WorkflowData, detectionEngineID string) string {
 	if data == nil {
 		return ""
 	}
 	engineID := ResolveEngineID(data)
+	// Cross-engine detection uses built-in aliases, not the main engine's alias definitions.
+	if engineID != detectionEngineID && len(data.ModelMappings[data.Model]) > 0 {
+		return ""
+	}
 	if engineID == "" || engineID == "pi" || isThreatDetectionCapableEngineID(engineID) {
 		return data.Model
 	}

diff --git a/pkg/workflow/threat_detection_inline_engine.go b/pkg/workflow/threat_detection_inline_engine.go
@@ -75,7 +75,7 @@ func (c *Compiler) buildDetectionEngineExecutionStep(data *WorkflowData) []strin
 		detectionEngineConfig.HarnessMaxRetries = "0"
 	}
 
-	resolvedDetectionModel := inheritedDetectionModel(data)
+	resolvedDetectionModel := inheritedDetectionModel(data, engineSetting)
 	if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.Model != "" {
 		resolvedDetectionModel = data.SafeOutputs.ThreatDetection.Model
 	}