Skip to content

Commit a93e36e

Browse files
CopilotpelikhanCodex
authored
Fix: engine.agent propagates to threat detection job causing "No such agent" failure (#17949)
* Initial plan * initial plan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix: strip engine.agent from threat detection engine config When engine.agent is set along with an explicit model, the modelExplicitlyConfigured branch was assigning detectionEngineConfig = engineConfig (a pointer copy), propagating the Agent field to the threat detection job. The detection job has no repo checkout, so the agent file can't be found, causing "No such agent" failures. Fix: create a struct copy that excludes Agent in the modelExplicitlyConfigured branch, matching the existing pattern in the else-branch. Add TestBuildEngineStepsStripsAgentField to verify --agent never appears in detection steps for both code paths. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Simplify detection engine config to only inherit ID and Model Per review feedback, the detection engine config should only carry ID and Model — all other fields (Version, MaxTurns, Concurrency, UserAgent, Env, Config, Args, Firewall, Agent) are intentionally stripped. This collapses the two-branch logic into a single minimal-copy pattern. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Also copy Version, Env, Config, Args into detection engine config Per review feedback, the detection engine config now inherits ID, Model, Version, Env, Config, and Args. MaxTurns, Concurrency, UserAgent, Firewall, and Agent remain intentionally omitted. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add changeset [skip-ci] --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Codex <codex@localhost>
1 parent b4d5106 commit a93e36e

8 files changed

Lines changed: 85 additions & 37 deletions

‎.changeset/patch-strip-detection-agent.md‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/cloclo.lock.yml‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/commit-changes-analyzer.lock.yml‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/daily-multi-device-docs-tester.lock.yml‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/smoke-claude.lock.yml‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/unbloat-docs.lock.yml‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/workflow/threat_detection.go‎

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -330,34 +330,20 @@ func (c *Compiler) buildEngineSteps(data *WorkflowData) []string {
330330
// 3. Default detection model from the engine (as environment variable fallback)
331331
detectionEngineConfig := engineConfig
332332

333-
// Only copy the engine config if a model is explicitly configured
334-
// If no model is configured, we'll let the environment variable mechanism handle it
335-
modelExplicitlyConfigured := engineConfig != nil && engineConfig.Model != ""
336-
337-
if modelExplicitlyConfigured {
338-
// Model is explicitly configured, use it as-is
339-
detectionEngineConfig = engineConfig
333+
// Build a detection engine config inheriting ID, Model, Version, Env, Config, Args.
334+
// MaxTurns, Concurrency, UserAgent, Firewall, and Agent are intentionally omitted —
335+
// the detection job is a simple threat-analysis invocation and must never run as a
336+
// custom agent (no repo checkout, agent file unavailable).
337+
if detectionEngineConfig == nil {
338+
detectionEngineConfig = &EngineConfig{ID: engineSetting}
340339
} else {
341-
// No model configured - create/update config but don't set model
342-
// This allows the engine execution to use environment variables
343-
if detectionEngineConfig == nil {
344-
detectionEngineConfig = &EngineConfig{
345-
ID: engineSetting,
346-
}
347-
} else {
348-
// Create a copy without setting the model
349-
detectionEngineConfig = &EngineConfig{
350-
ID: detectionEngineConfig.ID,
351-
Model: "", // Explicitly leave empty for env var mechanism
352-
Version: detectionEngineConfig.Version,
353-
MaxTurns: detectionEngineConfig.MaxTurns,
354-
Concurrency: detectionEngineConfig.Concurrency,
355-
UserAgent: detectionEngineConfig.UserAgent,
356-
Env: detectionEngineConfig.Env,
357-
Config: detectionEngineConfig.Config,
358-
Args: detectionEngineConfig.Args,
359-
Firewall: detectionEngineConfig.Firewall,
360-
}
340+
detectionEngineConfig = &EngineConfig{
341+
ID: detectionEngineConfig.ID,
342+
Model: detectionEngineConfig.Model,
343+
Version: detectionEngineConfig.Version,
344+
Env: detectionEngineConfig.Env,
345+
Config: detectionEngineConfig.Config,
346+
Args: detectionEngineConfig.Args,
361347
}
362348
}
363349

‎pkg/workflow/threat_detection_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,68 @@ func TestDetectionJobSkipCondition(t *testing.T) {
937937
}
938938
}
939939

940+
// TestBuildEngineStepsStripsAgentField verifies that the Agent field from the
941+
// main engine config is never propagated to the detection engine config,
942+
// regardless of whether a model is explicitly configured.
943+
func TestBuildEngineStepsStripsAgentField(t *testing.T) {
944+
compiler := NewCompiler()
945+
946+
tests := []struct {
947+
name string
948+
data *WorkflowData
949+
}{
950+
{
951+
name: "agent field stripped when model is explicitly configured",
952+
data: &WorkflowData{
953+
AI: "copilot",
954+
EngineConfig: &EngineConfig{
955+
ID: "copilot",
956+
Model: "claude-opus-4.6",
957+
Agent: "my-agent",
958+
},
959+
SafeOutputs: &SafeOutputsConfig{
960+
ThreatDetection: &ThreatDetectionConfig{},
961+
},
962+
},
963+
},
964+
{
965+
name: "agent field stripped when no model configured",
966+
data: &WorkflowData{
967+
AI: "copilot",
968+
EngineConfig: &EngineConfig{
969+
ID: "copilot",
970+
Agent: "my-agent",
971+
},
972+
SafeOutputs: &SafeOutputsConfig{
973+
ThreatDetection: &ThreatDetectionConfig{},
974+
},
975+
},
976+
},
977+
}
978+
979+
for _, tt := range tests {
980+
t.Run(tt.name, func(t *testing.T) {
981+
steps := compiler.buildEngineSteps(tt.data)
982+
983+
if len(steps) == 0 {
984+
t.Fatal("Expected non-empty steps")
985+
}
986+
987+
allSteps := strings.Join(steps, "")
988+
989+
// The --agent flag must not appear in the threat detection steps
990+
if strings.Contains(allSteps, "--agent") {
991+
t.Errorf("Expected detection steps to NOT contain --agent flag, but found it.\nGenerated steps:\n%s", allSteps)
992+
}
993+
994+
// Ensure the original engine config is not mutated
995+
if tt.data.EngineConfig != nil && tt.data.EngineConfig.Agent != "my-agent" {
996+
t.Errorf("Original EngineConfig.Agent was mutated; expected %q, got %q", "my-agent", tt.data.EngineConfig.Agent)
997+
}
998+
})
999+
}
1000+
}
1001+
9401002
// TestCopilotDetectionDefaultModel verifies that the copilot engine uses the
9411003
// default model gpt-5.1-codex-mini for the detection job when no model is specified
9421004
func TestCopilotDetectionDefaultModel(t *testing.T) {

0 commit comments

Comments
 (0)