Fix Codex threat detection job proxy setup#27974
Merged
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/381da048-e9d5-4055-9374-fd6f95b64ca2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/381da048-e9d5-4055-9374-fd6f95b64ca2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/381da048-e9d5-4055-9374-fd6f95b64ca2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/381da048-e9d5-4055-9374-fd6f95b64ca2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 23, 2026 05:10
View session
Collaborator
|
@copilot recompile |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/010a25c3-5182-443c-9669-b0fe1cb08e8c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Contributor
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
Contributor
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Contributor
|
|
Contributor
|
|
Contributor
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Contributor
Agent Container Tool Check
Result: 11/12 tools available — FAIL (dotnet missing)
|
Contributor
Author
Ran Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
|
Commit pushed:
|
Contributor
|
Smoke test (Codex) for run 24818321037
Warning The following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "chatgpt.com"See Network Configuration for more information.
|
Contributor
|
🧪 Smoke Test Results — §24818321079
Overall: ✅ PASS
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Good regression tests added for Codex detection MCP setup. Two minor suggestions on test coverage completeness — see inline comments.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.4M
|
|
||
| stepsString := strings.Join(steps, "") | ||
| if !strings.Contains(stepsString, "Start MCP Gateway") { | ||
| t.Errorf("Expected Codex detection steps to include MCP setup, got:\n%s", stepsString) |
Contributor
There was a problem hiding this comment.
Good test coverage for the MCP setup inclusion! Consider also asserting that the model_provider appears before the Codex execution step to ensure ordering is correct.
| steps := compiler.buildDetectionJobSteps(data) | ||
| stepsString := strings.Join(steps, "") | ||
|
|
||
| if count := strings.Count(stepsString, "name: Download container images"); count != 1 { |
Contributor
There was a problem hiding this comment.
The duplicate step check uses strings.Count which is good, but consider also verifying the step ordering — the Download container images step should appear after Install AWF binary for detection jobs.
Contributor
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Contributor
Contributor
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 230.7K
| run: npm install --ignore-scripts -g @openai/codex@0.121.0 | ||
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.28 | ||
| - name: Download container images |
Contributor
There was a problem hiding this comment.
🔍 Smoke test review comment #1: The "Download container images" step has been correctly moved to after the AWF binary installation step in the detection job. This ensures proper bootstrap ordering — the MCP gateway image is now available before Codex initialization runs.
| run: npm install --ignore-scripts -g @openai/codex@0.121.0 | ||
| - name: Install AWF binary | ||
| run: bash "${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh" v0.25.28 | ||
| - name: Download container images |
Contributor
There was a problem hiding this comment.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 85/100✅ Excellent
Test Classification Details
Test AnalysisBoth new tests directly correspond to the bug fixed in this PR ("Fix Codex threat detection job proxy setup"):
Minor Observation (non-blocking)
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24818559215
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Codex threat-detection workflow generation so detection jobs consistently bootstrap MCP/OpenAI-proxy configuration (avoiding unauthorized API key failures) and removes a duplicated container image download step.
Changes:
- Ensure MCP setup generation runs for Codex + AWF even when
mcpToolsis empty (to include theopenai-proxymodel provider). - Prepend Codex detection execution with MCP setup (gateway/config bootstrap) and improve failure logging.
- Avoid duplicate
Download container imagesstep generation for Codex detection jobs; add regression tests and refresh lockfiles.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/threat_detection.go |
Skips the standalone AWF pre-pull step for Codex detection and injects MCP setup into Codex detection execution; adds helper to resolve effective detection engine ID. |
pkg/workflow/mcp_setup_generator.go |
Allows MCP setup generation to proceed for Codex+AWF even with no MCP tools configured. |
pkg/workflow/threat_detection_test.go |
Adds regression tests for Codex detection MCP setup inclusion and duplicate container download avoidance. |
.github/workflows/smoke-codex.lock.yml |
Regenerated lockfile reflecting MCP setup in Codex detection and adjusted container download placement. |
.github/workflows/smoke-call-workflow.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/schema-feature-coverage.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/issue-arborist.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/grumpy-reviewer.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/duplicate-code-detector.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/daily-observability-report.lock.yml |
Same as above for this workflow’s detection job. |
.github/workflows/daily-fact.lock.yml |
Same as above for this workflow’s detection job. |
.changeset/patch-fix-codex-threat-detection-proxy.md |
Adds a patch changeset describing the fix. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 12/12 changed files
- Comments generated: 1
| if err := c.generateMCPSetup(&mcpSetup, threatDetectionData.Tools, engine, threatDetectionData); err == nil { | ||
| for line := range strings.SplitSeq(mcpSetup.String(), "\n") { | ||
| if line != "" { | ||
| steps = append(steps, line+"\n") |
There was a problem hiding this comment.
In buildDetectionEngineExecutionStep, the MCP setup lines appended for Codex detection (Download container images / Start MCP Gateway / config writes) are not gated by detectionStepCondition. As a result, these steps will still run even when detection_guard.outputs.run_detection == 'false', defeating the guard and adding unnecessary work (and potential failure modes) to skipped detection runs. Consider injecting if: always() && steps.detection_guard.outputs.run_detection == 'true' into each generated MCP setup step (e.g., after each - name:), or extending generateMCPSetup to support an optional step-level if condition for the detection job path.
Suggested change
| steps = append(steps, line+"\n") | |
| steps = append(steps, line+"\n") | |
| if strings.HasPrefix(strings.TrimSpace(line), "- name:") { | |
| steps = append(steps, fmt.Sprintf(" if: %s\n", detectionStepCondition)) | |
| } |
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.
Summary
Fixes Codex detection job failures where the detection
Execute Codex CLIstep failed with unauthorized OpenAI API key errors while the main agent job succeeded.Root cause
The detection job did not generate Codex MCP/proxy bootstrap when no MCP tools were configured, so Codex did not consistently use the AWF OpenAI proxy provider in detection runs.
Changes
mcpToolsis empty.Download container imagesstep generation for Codex detection jobs.Validation
go test -v -run 'TestCodexEngineWithOutputSteps|TestBuildDetectionEngineExecutionStepCodexIncludesMCPSetup|TestBuildDetectionJobStepsCodexAvoidsDuplicateContainerPullStep' ./pkg/workflow/make fmtmake agent-finish(fails on pre-existing unrelated tests in this branch baseline):TestCopilotDetectionDefaultModel(2 subtests)TestWasmGolden_CompileFixtures(basic-copilot,with-imports)🤖 Smoke CI scheduled run completed — https://github.com/github/gh-aw/actions/runs/24818278489
Changeset
Warning
The following domains were blocked by the firewall during workflow execution:
ab.chatgpt.comchatgpt.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
🤖 Smoke CI scheduled run completed — https://github.com/github/gh-aw/actions/runs/24818464108
✨ PR Review Safe Output Test - Run 24818321063