Fix: engine.agent propagates to threat detection job causing "No such agent" failure#17949
Fix: engine.agent propagates to threat detection job causing "No such agent" failure#17949
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
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>
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>
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>
|
📰 DEVELOPING STORY: Smoke Copilot reports was cancelled. Our correspondents are investigating the incident... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
There was a problem hiding this comment.
Pull request overview
This pull request fixes a bug where the engine.agent field from the main workflow configuration was being propagated to the threat detection job, causing it to fail with "No such agent" errors. The threat detection job runs without repository checkout, making agent files unavailable.
Changes:
- Refactored
buildEngineSteps()inpkg/workflow/threat_detection.goto explicitly copy only necessary fields (ID, Model, Version, Env, Config, Args) to the detection engine config, intentionally omitting Agent, MaxTurns, Concurrency, UserAgent, and Firewall - Added comprehensive test coverage in
TestBuildEngineStepsStripsAgentFieldto verify the Agent field is never propagated - Updated auto-generated
.lock.ymlworkflow files to reflect the removal of--max-turnsflags and addition ofstorage.googleapis.comto allowed domains
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/workflow/threat_detection.go | Replaced pointer copy with explicit field copying to prevent Agent field propagation to detection job |
| pkg/workflow/threat_detection_test.go | Added test verifying Agent field is stripped in both model-configured and non-configured scenarios |
| .github/workflows/*.lock.yml | Auto-generated workflow files reflecting MaxTurns removal and domain additions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Smoke test results:
|
|
Commit pushed: |
|
🤖 Smoke test §22319427353 results for
Overall: PASS ✅
|
There was a problem hiding this comment.
Good fix for the Agent field leak into the threat detection job. The minimal-copy approach is cleaner than the previous two-branch pattern. Left two inline comments — one on a potential simplification and one suggesting a clarifying comment on the intentionally omitted fields.
📰 BREAKING: Report filed by Smoke Copilot
| @@ -330,34 +330,20 @@ func (c *Compiler) buildEngineSteps(data *WorkflowData) []string { | |||
| // 3. Default detection model from the engine (as environment variable fallback) | |||
| detectionEngineConfig := engineConfig | |||
There was a problem hiding this comment.
The initial detectionEngineConfig := engineConfig assignment (which sets a full reference) is immediately replaced in the if/else below, so it's a no-op. Could simplify to var detectionEngineConfig *EngineConfig to make the intent clearer from the start.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
When
engine.agentis set alongside an explicitmodel, the threat detection job inherits the--agentflag via a pointer copy ofEngineConfig. Since the detection job never checks out the repo, the agent file is missing and the job fails immediately withNo such agent: <name>, available:.Changes
pkg/workflow/threat_detection.go— InbuildEngineSteps(), the two-branch logic is collapsed into a single minimal-copy pattern. The detection engine config now inherits onlyID,Model,Version,Env,Config, andArgsfrom the main engine config.MaxTurns,Concurrency,UserAgent,Firewall, andAgentare intentionally omitted — the detection job is a simple threat-analysis invocation and must never run as a custom agent.pkg/workflow/threat_detection_test.go— AddedTestBuildEngineStepsStripsAgentFieldcovering both the model-configured and non-model-configured paths, asserting--agentnever appears in detection steps and that the originalEngineConfigis not mutated.Original prompt
engine.agentpropagates to threat detection job, causing "No such agent" failure #17943✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Changeset
Warning
The following domains were blocked by the firewall during workflow execution:
codeload.github.comgithub.com✨ PR Review Safe Output Test - Run 22319427382