Skip to content

Commit 17dff22

Browse files
authored
fix: set supportsNativeAgentFile=false for Codex and Gemini engines; remove awk/AGENT_CONTENT shell code from Codex (#25681)
1 parent a0803a5 commit 17dff22

File tree

4 files changed

+190
-64
lines changed

4 files changed

+190
-64
lines changed

pkg/workflow/codex_engine.go

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func NewCodexEngine() *CodexEngine {
4040
supportsMaxTurns: false, // Codex does not support max-turns feature
4141
supportsMaxContinuations: false, // Codex does not support --max-autopilot-continues-style continuation mode
4242
supportsWebSearch: true, // Codex has built-in web-search support
43-
supportsNativeAgentFile: true, // Codex reads the agent file and prepends it to the prompt at runtime
43+
supportsNativeAgentFile: false, // Codex does not support agent file natively; the compiler prepends the agent file content to prompt.txt
4444
llmGatewayPort: constants.CodexLLMGatewayPort,
4545
},
4646
}
@@ -136,8 +136,8 @@ func (e *CodexEngine) GetAgentManifestPathPrefixes() []string {
136136
func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile string) []GitHubActionStep {
137137
modelConfigured := workflowData.EngineConfig != nil && workflowData.EngineConfig.Model != ""
138138
firewallEnabled := isFirewallEnabled(workflowData)
139-
codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, has_agent_file=%v, firewall=%v",
140-
workflowData.Name, modelConfigured, workflowData.AgentFile != "", firewallEnabled)
139+
codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, firewall=%v",
140+
workflowData.Name, modelConfigured, firewallEnabled)
141141

142142
var steps []GitHubActionStep
143143

@@ -224,20 +224,11 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
224224
// However, npm-installed CLIs (like codex) need hostedtoolcache bin directories in PATH.
225225
npmPathSetup := GetNpmBinPathSetup()
226226

227-
// Codex reads both agent file and prompt inside AWF container (PATH setup + agent file reading + codex command)
228-
var codexCommandWithSetup string
229-
if workflowData.AgentFile != "" {
230-
agentPath, err := ResolveAgentFilePath(workflowData.AgentFile)
231-
if err != nil {
232-
codexEngineLog.Printf("Error resolving agent file path: %v", err)
233-
return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err)
234-
}
235-
// Read agent file and prompt inside AWF container, with PATH setup for npm binaries
236-
codexCommandWithSetup = fmt.Sprintf(`%s && AGENT_CONTENT="$(awk 'BEGIN{skip=1} /^---$/{if(skip){skip=0;next}else{skip=1;next}} !skip' %s)" && INSTRUCTION="$(printf "%%s\n\n%%s" "$AGENT_CONTENT" "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)")" && %s`, npmPathSetup, agentPath, codexCommand)
237-
} else {
238-
// Read prompt inside AWF container to avoid Docker Compose interpolation issues, with PATH setup
239-
codexCommandWithSetup = fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand)
240-
}
227+
// Codex reads prompt inside AWF container (PATH setup + codex command).
228+
// For engines that do not support native agent-file handling (including Codex),
229+
// the compiler prepends the agent file content to prompt.txt so no special
230+
// shell variable juggling is needed here.
231+
codexCommandWithSetup := fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand)
241232

242233
command = BuildAWFCommand(AWFCommandConfig{
243234
EngineName: "codex",
@@ -255,29 +246,16 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
255246
ExcludeEnvVarNames: ComputeAWFExcludeEnvVarNames(workflowData, []string{"CODEX_API_KEY", "OPENAI_API_KEY"}),
256247
})
257248
} else {
258-
// Build the command without AWF wrapping
259-
// Reuse commandName already determined above
260-
if workflowData.AgentFile != "" {
261-
agentPath, err := ResolveAgentFilePath(workflowData.AgentFile)
262-
if err != nil {
263-
codexEngineLog.Printf("Error resolving agent file path: %v", err)
264-
return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err)
265-
}
266-
command = fmt.Sprintf(`set -o pipefail
267-
touch %s
268-
(umask 177 && touch %s)
269-
AGENT_CONTENT="$(awk 'BEGIN{skip=1} /^---$/{if(skip){skip=0;next}else{skip=1;next}} !skip' %s)"
270-
INSTRUCTION="$(printf "%%s\n\n%%s" "$AGENT_CONTENT" "$(cat "$GH_AW_PROMPT")")"
271-
mkdir -p "$CODEX_HOME/logs"
272-
%s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, agentPath, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile)
273-
} else {
274-
command = fmt.Sprintf(`set -o pipefail
249+
// Build the command without AWF wrapping.
250+
// For engines that do not support native agent-file handling (including Codex),
251+
// the compiler prepends the agent file content to prompt.txt so no special
252+
// shell variable juggling is needed here.
253+
command = fmt.Sprintf(`set -o pipefail
275254
touch %s
276255
(umask 177 && touch %s)
277256
INSTRUCTION="$(cat "$GH_AW_PROMPT")"
278257
mkdir -p "$CODEX_HOME/logs"
279258
%s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile)
280-
}
281259
}
282260

283261
// Get effective GitHub token based on precedence: custom token > default

pkg/workflow/compiler_yaml.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,8 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre
387387
// - Agent files WITHOUT inputs: path is in data.ImportPaths → included by Step 1b.
388388
// - Agent files WITH inputs: content is in data.ImportedMarkdown → included by Step 1a.
389389
// - inlined-imports mode: data.AgentFile is cleared; content is in data.ImportPaths.
390-
// Engines that DO support native agent-file handling (e.g. Codex) continue to read the
391-
// file themselves in GetExecutionSteps alongside the runtime-import.
390+
// All current engines (Claude, Codex, Gemini, Copilot) use this mechanism: SupportsNativeAgentFile()
391+
// returns false and they read the fully-assembled prompt.txt in GetExecutionSteps.
392392

393393
var userPromptChunks []string
394394
var expressionMappings []*ExpressionMapping

pkg/workflow/engine_agent_import_test.go

Lines changed: 174 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ func TestClaudeEngineWithoutAgentFile(t *testing.T) {
186186
}
187187
}
188188

189-
// TestCodexEngineWithAgentFromImports tests that codex engine prepends agent file content to prompt
189+
// TestCodexEngineWithAgentFromImports tests that codex engine does NOT handle agent file natively
190+
// and instead relies on the compiler to include the agent file content in prompt.txt
190191
func TestCodexEngineWithAgentFromImports(t *testing.T) {
191192
engine := NewCodexEngine()
192193
workflowData := &WorkflowData{
@@ -205,23 +206,23 @@ func TestCodexEngineWithAgentFromImports(t *testing.T) {
205206

206207
stepContent := strings.Join([]string(steps[0]), "\n")
207208

208-
// Check that agent content extraction is present
209-
if !strings.Contains(stepContent, `AGENT_CONTENT="$(awk`) {
210-
t.Errorf("Expected agent content extraction in codex command, got:\n%s", stepContent)
209+
// Codex does not handle the agent file natively — no awk or AGENT_CONTENT variable
210+
// juggling should appear in the step.
211+
if strings.Contains(stepContent, "AGENT_CONTENT") {
212+
t.Errorf("Codex must NOT handle agent file natively (AGENT_CONTENT found in step); the compiler handles it:\n%s", stepContent)
211213
}
212-
213-
// Check that agent file path is referenced with quoted GITHUB_WORKSPACE prefix
214-
if !strings.Contains(stepContent, `"${GITHUB_WORKSPACE}/.github/agents/test-agent.md"`) {
215-
t.Errorf("Expected agent file path with quoted GITHUB_WORKSPACE prefix in codex command, got:\n%s", stepContent)
214+
if strings.Contains(stepContent, "awk") {
215+
t.Errorf("Codex must NOT invoke awk for agent file reading (found in step); the compiler handles it:\n%s", stepContent)
216216
}
217217

218-
// Check that agent content is prepended to prompt using printf
219-
if !strings.Contains(stepContent, `INSTRUCTION="$(printf`) {
220-
t.Errorf("Expected printf with INSTRUCTION in codex command, got:\n%s", stepContent)
218+
// The engine still reads the standard prompt.txt (which has agent content prepended by the compiler).
219+
if !strings.Contains(stepContent, `INSTRUCTION="$(cat "$GH_AW_PROMPT")"`) {
220+
t.Errorf("Expected standard prompt.txt reading in codex command, got:\n%s", stepContent)
221221
}
222222

223-
if !strings.Contains(stepContent, "$AGENT_CONTENT") {
224-
t.Errorf("Expected $AGENT_CONTENT variable in codex command, got:\n%s", stepContent)
223+
// The engine reports that it does not support native agent file handling.
224+
if engine.SupportsNativeAgentFile() {
225+
t.Errorf("Codex engine should return false for SupportsNativeAgentFile()")
225226
}
226227
}
227228

@@ -254,6 +255,154 @@ func TestCodexEngineWithoutAgentFile(t *testing.T) {
254255
}
255256
}
256257

258+
// TestCodexEngineDoesNotSupportNativeAgentFile verifies that the Codex engine declares
259+
// it does not handle agent files natively, so the compiler knows to prepend the agent file
260+
// content to prompt.txt during the activation job instead.
261+
func TestCodexEngineDoesNotSupportNativeAgentFile(t *testing.T) {
262+
engine := NewCodexEngine()
263+
if engine.SupportsNativeAgentFile() {
264+
t.Errorf("Codex engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection")
265+
}
266+
}
267+
268+
// TestCodexEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used
269+
// with the firewall (AWF) enabled, the codex command reads from prompt.txt (not from a
270+
// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt
271+
// in the activation job.
272+
func TestCodexEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) {
273+
engine := NewCodexEngine()
274+
275+
agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF}
276+
workflowData := &WorkflowData{
277+
Name: "test-workflow",
278+
EngineConfig: &EngineConfig{
279+
ID: "codex",
280+
},
281+
AgentFile: ".github/agents/test-agent.md",
282+
SandboxConfig: &SandboxConfig{
283+
Agent: agentSandbox,
284+
},
285+
}
286+
287+
steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log")
288+
if len(steps) == 0 {
289+
t.Fatal("Expected at least one step")
290+
}
291+
292+
stepContent := strings.Join([]string(steps[0]), "\n")
293+
294+
// No AGENT_CONTENT shell variable anywhere in the step.
295+
if strings.Contains(stepContent, "AGENT_CONTENT") {
296+
t.Errorf("AGENT_CONTENT must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent)
297+
}
298+
if strings.Contains(stepContent, "awk") {
299+
t.Errorf("awk must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent)
300+
}
301+
302+
// The container command must still read from prompt.txt.
303+
if !strings.Contains(stepContent, `INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
304+
t.Errorf("Expected codex to read from prompt.txt in AWF mode, got:\n%s", stepContent)
305+
}
306+
}
307+
308+
// TestGeminiEngineDoesNotSupportNativeAgentFile verifies that the Gemini engine declares
309+
// it does not handle agent files natively, so the compiler knows to prepend the agent file
310+
// content to prompt.txt during the activation job instead.
311+
func TestGeminiEngineDoesNotSupportNativeAgentFile(t *testing.T) {
312+
engine := NewGeminiEngine()
313+
if engine.SupportsNativeAgentFile() {
314+
t.Errorf("Gemini engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection")
315+
}
316+
}
317+
318+
// TestGeminiEngineWithAgentFromImports tests that Gemini engine does NOT handle agent file natively
319+
// and instead relies on the compiler to include the agent file content in prompt.txt
320+
func TestGeminiEngineWithAgentFromImports(t *testing.T) {
321+
engine := NewGeminiEngine()
322+
workflowData := &WorkflowData{
323+
Name: "test-workflow",
324+
EngineConfig: &EngineConfig{
325+
ID: "gemini",
326+
},
327+
AgentFile: ".github/agents/test-agent.md",
328+
Tools: map[string]any{},
329+
}
330+
331+
steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
332+
333+
// GetExecutionSteps returns a settings step + execution step for Gemini
334+
if len(steps) == 0 {
335+
t.Fatal("Expected at least one execution step")
336+
}
337+
338+
// Combine all step content for inspection
339+
var allContent strings.Builder
340+
for _, step := range steps {
341+
allContent.WriteString(strings.Join([]string(step), "\n"))
342+
allContent.WriteString("\n")
343+
}
344+
combined := allContent.String()
345+
346+
// Gemini does not handle the agent file natively — no awk or AGENT_CONTENT
347+
if strings.Contains(combined, "AGENT_CONTENT") {
348+
t.Errorf("Gemini must NOT handle agent file natively (AGENT_CONTENT found in steps); the compiler handles it:\n%s", combined)
349+
}
350+
if strings.Contains(combined, "awk") {
351+
t.Errorf("Gemini must NOT invoke awk for agent file reading (found in steps); the compiler handles it:\n%s", combined)
352+
}
353+
354+
// The execution step must read the prompt from prompt.txt
355+
if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
356+
t.Errorf("Expected Gemini to read from prompt.txt, got:\n%s", combined)
357+
}
358+
}
359+
360+
// TestGeminiEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used
361+
// with the firewall (AWF) enabled, the gemini command reads from prompt.txt (not from a
362+
// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt
363+
// in the activation job.
364+
func TestGeminiEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) {
365+
engine := NewGeminiEngine()
366+
367+
agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF}
368+
workflowData := &WorkflowData{
369+
Name: "test-workflow",
370+
EngineConfig: &EngineConfig{
371+
ID: "gemini",
372+
},
373+
AgentFile: ".github/agents/test-agent.md",
374+
SandboxConfig: &SandboxConfig{
375+
Agent: agentSandbox,
376+
},
377+
Tools: map[string]any{},
378+
}
379+
380+
steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log")
381+
if len(steps) == 0 {
382+
t.Fatal("Expected at least one step")
383+
}
384+
385+
var allContent strings.Builder
386+
for _, step := range steps {
387+
allContent.WriteString(strings.Join([]string(step), "\n"))
388+
allContent.WriteString("\n")
389+
}
390+
combined := allContent.String()
391+
392+
// No AGENT_CONTENT shell variable anywhere in the steps.
393+
if strings.Contains(combined, "AGENT_CONTENT") {
394+
t.Errorf("AGENT_CONTENT must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined)
395+
}
396+
if strings.Contains(combined, "awk") {
397+
t.Errorf("awk must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined)
398+
}
399+
400+
// The command must still read from prompt.txt.
401+
if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
402+
t.Errorf("Expected gemini to read from prompt.txt in AWF mode, got:\n%s", combined)
403+
}
404+
}
405+
257406
// TestAgentFileValidation tests compile-time validation of agent file existence
258407
func TestAgentFileValidation(t *testing.T) {
259408
// Create a temporary directory structure that mimics a repository
@@ -353,15 +502,16 @@ This is a test agent file.
353502
})
354503
}
355504

356-
// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that handle agent files
357-
// natively emit a clearly-failing step (rather than silently skipping execution) when the
358-
// agent file path contains shell metacharacters.
359-
// Engines that do NOT support native agent files (e.g. Claude) rely on the compiler's
360-
// validateAgentFile to reject malicious paths at compile time instead.
505+
// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that do NOT handle agent files
506+
// natively (Claude, Codex, Gemini) rely on the compiler's validateAgentFile to reject malicious
507+
// paths at compile time. Engine steps should proceed normally and never reference agent file paths.
361508
func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) {
362509
maliciousPath := `.github/agents/a";id;"b.md`
363510

364-
t.Run("codex_emits_failing_step_for_invalid_path", func(t *testing.T) {
511+
// Codex does not handle agent files natively; path validation is done by the compiler
512+
// at compile time (validateAgentFile). The engine step should proceed normally and never
513+
// reference the agent file path directly.
514+
t.Run("codex_ignores_agent_path_in_step_for_invalid_path", func(t *testing.T) {
365515
engine := NewCodexEngine()
366516
workflowData := &WorkflowData{
367517
Name: "test-workflow",
@@ -370,18 +520,15 @@ func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) {
370520
steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log")
371521

372522
if len(steps) != 1 {
373-
t.Fatalf("Expected exactly 1 failing step, got %d", len(steps))
523+
t.Fatalf("Expected exactly 1 step, got %d", len(steps))
374524
}
375525
content := strings.Join([]string(steps[0]), "\n")
376-
if !strings.Contains(content, "exit 1") {
377-
t.Errorf("Expected failing step with 'exit 1', got:\n%s", content)
378-
}
379-
if !strings.Contains(content, "Error") {
380-
t.Errorf("Expected error message in failing step, got:\n%s", content)
526+
// Must NOT reference the malicious path at all in the generated step
527+
if strings.Contains(content, maliciousPath) {
528+
t.Errorf("Codex step must not reference the agent file path directly, got:\n%s", content)
381529
}
382-
// Must NOT invoke awk (that would mean the path was used for real execution)
383530
if strings.Contains(content, "awk") {
384-
t.Errorf("Failing step must not invoke awk with the invalid path, got:\n%s", content)
531+
t.Errorf("Codex step must not invoke awk for agent file reading, got:\n%s", content)
385532
}
386533
})
387534

pkg/workflow/gemini_engine.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func NewGeminiEngine() *GeminiEngine {
2626
supportsMaxTurns: false,
2727
supportsMaxContinuations: false, // Gemini CLI does not support --max-autopilot-continues-style continuation mode
2828
supportsWebSearch: false,
29+
supportsNativeAgentFile: false, // Gemini does not support agent file natively; the compiler prepends the agent file content to prompt.txt
2930
llmGatewayPort: constants.GeminiLLMGatewayPort,
3031
},
3132
}

0 commit comments

Comments
 (0)