diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 0e473474dcc..253d5aa4d60 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -40,7 +40,7 @@ func NewCodexEngine() *CodexEngine { supportsMaxTurns: false, // Codex does not support max-turns feature supportsMaxContinuations: false, // Codex does not support --max-autopilot-continues-style continuation mode supportsWebSearch: true, // Codex has built-in web-search support - supportsNativeAgentFile: true, // Codex reads the agent file and prepends it to the prompt at runtime + supportsNativeAgentFile: false, // Codex does not support agent file natively; the compiler prepends the agent file content to prompt.txt llmGatewayPort: constants.CodexLLMGatewayPort, }, } @@ -136,8 +136,8 @@ func (e *CodexEngine) GetAgentManifestPathPrefixes() []string { func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile string) []GitHubActionStep { modelConfigured := workflowData.EngineConfig != nil && workflowData.EngineConfig.Model != "" firewallEnabled := isFirewallEnabled(workflowData) - codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, has_agent_file=%v, firewall=%v", - workflowData.Name, modelConfigured, workflowData.AgentFile != "", firewallEnabled) + codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, firewall=%v", + workflowData.Name, modelConfigured, firewallEnabled) var steps []GitHubActionStep @@ -224,20 +224,11 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri // However, npm-installed CLIs (like codex) need hostedtoolcache bin directories in PATH. npmPathSetup := GetNpmBinPathSetup() - // Codex reads both agent file and prompt inside AWF container (PATH setup + agent file reading + codex command) - var codexCommandWithSetup string - if workflowData.AgentFile != "" { - agentPath, err := ResolveAgentFilePath(workflowData.AgentFile) - if err != nil { - codexEngineLog.Printf("Error resolving agent file path: %v", err) - return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err) - } - // Read agent file and prompt inside AWF container, with PATH setup for npm binaries - 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) - } else { - // Read prompt inside AWF container to avoid Docker Compose interpolation issues, with PATH setup - codexCommandWithSetup = fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand) - } + // Codex reads prompt inside AWF container (PATH setup + codex command). + // For engines that do not support native agent-file handling (including Codex), + // the compiler prepends the agent file content to prompt.txt so no special + // shell variable juggling is needed here. + codexCommandWithSetup := fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand) command = BuildAWFCommand(AWFCommandConfig{ EngineName: "codex", @@ -255,29 +246,16 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri ExcludeEnvVarNames: ComputeAWFExcludeEnvVarNames(workflowData, []string{"CODEX_API_KEY", "OPENAI_API_KEY"}), }) } else { - // Build the command without AWF wrapping - // Reuse commandName already determined above - if workflowData.AgentFile != "" { - agentPath, err := ResolveAgentFilePath(workflowData.AgentFile) - if err != nil { - codexEngineLog.Printf("Error resolving agent file path: %v", err) - return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err) - } - command = fmt.Sprintf(`set -o pipefail -touch %s -(umask 177 && touch %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 "$GH_AW_PROMPT")")" -mkdir -p "$CODEX_HOME/logs" -%s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, agentPath, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile) - } else { - command = fmt.Sprintf(`set -o pipefail + // Build the command without AWF wrapping. + // For engines that do not support native agent-file handling (including Codex), + // the compiler prepends the agent file content to prompt.txt so no special + // shell variable juggling is needed here. + command = fmt.Sprintf(`set -o pipefail touch %s (umask 177 && touch %s) INSTRUCTION="$(cat "$GH_AW_PROMPT")" mkdir -p "$CODEX_HOME/logs" %s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile) - } } // Get effective GitHub token based on precedence: custom token > default diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index aef655f9b85..280ace0d91e 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -387,8 +387,8 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre // - Agent files WITHOUT inputs: path is in data.ImportPaths → included by Step 1b. // - Agent files WITH inputs: content is in data.ImportedMarkdown → included by Step 1a. // - inlined-imports mode: data.AgentFile is cleared; content is in data.ImportPaths. - // Engines that DO support native agent-file handling (e.g. Codex) continue to read the - // file themselves in GetExecutionSteps alongside the runtime-import. + // All current engines (Claude, Codex, Gemini, Copilot) use this mechanism: SupportsNativeAgentFile() + // returns false and they read the fully-assembled prompt.txt in GetExecutionSteps. var userPromptChunks []string var expressionMappings []*ExpressionMapping diff --git a/pkg/workflow/engine_agent_import_test.go b/pkg/workflow/engine_agent_import_test.go index b4be21eaee4..1443ed881f6 100644 --- a/pkg/workflow/engine_agent_import_test.go +++ b/pkg/workflow/engine_agent_import_test.go @@ -186,7 +186,8 @@ func TestClaudeEngineWithoutAgentFile(t *testing.T) { } } -// TestCodexEngineWithAgentFromImports tests that codex engine prepends agent file content to prompt +// TestCodexEngineWithAgentFromImports tests that codex engine does NOT handle agent file natively +// and instead relies on the compiler to include the agent file content in prompt.txt func TestCodexEngineWithAgentFromImports(t *testing.T) { engine := NewCodexEngine() workflowData := &WorkflowData{ @@ -205,23 +206,23 @@ func TestCodexEngineWithAgentFromImports(t *testing.T) { stepContent := strings.Join([]string(steps[0]), "\n") - // Check that agent content extraction is present - if !strings.Contains(stepContent, `AGENT_CONTENT="$(awk`) { - t.Errorf("Expected agent content extraction in codex command, got:\n%s", stepContent) + // Codex does not handle the agent file natively — no awk or AGENT_CONTENT variable + // juggling should appear in the step. + if strings.Contains(stepContent, "AGENT_CONTENT") { + t.Errorf("Codex must NOT handle agent file natively (AGENT_CONTENT found in step); the compiler handles it:\n%s", stepContent) } - - // Check that agent file path is referenced with quoted GITHUB_WORKSPACE prefix - if !strings.Contains(stepContent, `"${GITHUB_WORKSPACE}/.github/agents/test-agent.md"`) { - t.Errorf("Expected agent file path with quoted GITHUB_WORKSPACE prefix in codex command, got:\n%s", stepContent) + if strings.Contains(stepContent, "awk") { + t.Errorf("Codex must NOT invoke awk for agent file reading (found in step); the compiler handles it:\n%s", stepContent) } - // Check that agent content is prepended to prompt using printf - if !strings.Contains(stepContent, `INSTRUCTION="$(printf`) { - t.Errorf("Expected printf with INSTRUCTION in codex command, got:\n%s", stepContent) + // The engine still reads the standard prompt.txt (which has agent content prepended by the compiler). + if !strings.Contains(stepContent, `INSTRUCTION="$(cat "$GH_AW_PROMPT")"`) { + t.Errorf("Expected standard prompt.txt reading in codex command, got:\n%s", stepContent) } - if !strings.Contains(stepContent, "$AGENT_CONTENT") { - t.Errorf("Expected $AGENT_CONTENT variable in codex command, got:\n%s", stepContent) + // The engine reports that it does not support native agent file handling. + if engine.SupportsNativeAgentFile() { + t.Errorf("Codex engine should return false for SupportsNativeAgentFile()") } } @@ -254,6 +255,154 @@ func TestCodexEngineWithoutAgentFile(t *testing.T) { } } +// TestCodexEngineDoesNotSupportNativeAgentFile verifies that the Codex engine declares +// it does not handle agent files natively, so the compiler knows to prepend the agent file +// content to prompt.txt during the activation job instead. +func TestCodexEngineDoesNotSupportNativeAgentFile(t *testing.T) { + engine := NewCodexEngine() + if engine.SupportsNativeAgentFile() { + t.Errorf("Codex engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection") + } +} + +// TestCodexEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used +// with the firewall (AWF) enabled, the codex command reads from prompt.txt (not from a +// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt +// in the activation job. +func TestCodexEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) { + engine := NewCodexEngine() + + agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF} + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "codex", + }, + AgentFile: ".github/agents/test-agent.md", + SandboxConfig: &SandboxConfig{ + Agent: agentSandbox, + }, + } + + steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log") + if len(steps) == 0 { + t.Fatal("Expected at least one step") + } + + stepContent := strings.Join([]string(steps[0]), "\n") + + // No AGENT_CONTENT shell variable anywhere in the step. + if strings.Contains(stepContent, "AGENT_CONTENT") { + t.Errorf("AGENT_CONTENT must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent) + } + if strings.Contains(stepContent, "awk") { + t.Errorf("awk must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent) + } + + // The container command must still read from prompt.txt. + if !strings.Contains(stepContent, `INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) { + t.Errorf("Expected codex to read from prompt.txt in AWF mode, got:\n%s", stepContent) + } +} + +// TestGeminiEngineDoesNotSupportNativeAgentFile verifies that the Gemini engine declares +// it does not handle agent files natively, so the compiler knows to prepend the agent file +// content to prompt.txt during the activation job instead. +func TestGeminiEngineDoesNotSupportNativeAgentFile(t *testing.T) { + engine := NewGeminiEngine() + if engine.SupportsNativeAgentFile() { + t.Errorf("Gemini engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection") + } +} + +// TestGeminiEngineWithAgentFromImports tests that Gemini engine does NOT handle agent file natively +// and instead relies on the compiler to include the agent file content in prompt.txt +func TestGeminiEngineWithAgentFromImports(t *testing.T) { + engine := NewGeminiEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "gemini", + }, + AgentFile: ".github/agents/test-agent.md", + Tools: map[string]any{}, + } + + steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") + + // GetExecutionSteps returns a settings step + execution step for Gemini + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + // Combine all step content for inspection + var allContent strings.Builder + for _, step := range steps { + allContent.WriteString(strings.Join([]string(step), "\n")) + allContent.WriteString("\n") + } + combined := allContent.String() + + // Gemini does not handle the agent file natively — no awk or AGENT_CONTENT + if strings.Contains(combined, "AGENT_CONTENT") { + t.Errorf("Gemini must NOT handle agent file natively (AGENT_CONTENT found in steps); the compiler handles it:\n%s", combined) + } + if strings.Contains(combined, "awk") { + t.Errorf("Gemini must NOT invoke awk for agent file reading (found in steps); the compiler handles it:\n%s", combined) + } + + // The execution step must read the prompt from prompt.txt + if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) { + t.Errorf("Expected Gemini to read from prompt.txt, got:\n%s", combined) + } +} + +// TestGeminiEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used +// with the firewall (AWF) enabled, the gemini command reads from prompt.txt (not from a +// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt +// in the activation job. +func TestGeminiEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) { + engine := NewGeminiEngine() + + agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF} + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "gemini", + }, + AgentFile: ".github/agents/test-agent.md", + SandboxConfig: &SandboxConfig{ + Agent: agentSandbox, + }, + Tools: map[string]any{}, + } + + steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log") + if len(steps) == 0 { + t.Fatal("Expected at least one step") + } + + var allContent strings.Builder + for _, step := range steps { + allContent.WriteString(strings.Join([]string(step), "\n")) + allContent.WriteString("\n") + } + combined := allContent.String() + + // No AGENT_CONTENT shell variable anywhere in the steps. + if strings.Contains(combined, "AGENT_CONTENT") { + t.Errorf("AGENT_CONTENT must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined) + } + if strings.Contains(combined, "awk") { + t.Errorf("awk must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined) + } + + // The command must still read from prompt.txt. + if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) { + t.Errorf("Expected gemini to read from prompt.txt in AWF mode, got:\n%s", combined) + } +} + // TestAgentFileValidation tests compile-time validation of agent file existence func TestAgentFileValidation(t *testing.T) { // Create a temporary directory structure that mimics a repository @@ -353,15 +502,16 @@ This is a test agent file. }) } -// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that handle agent files -// natively emit a clearly-failing step (rather than silently skipping execution) when the -// agent file path contains shell metacharacters. -// Engines that do NOT support native agent files (e.g. Claude) rely on the compiler's -// validateAgentFile to reject malicious paths at compile time instead. +// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that do NOT handle agent files +// natively (Claude, Codex, Gemini) rely on the compiler's validateAgentFile to reject malicious +// paths at compile time. Engine steps should proceed normally and never reference agent file paths. func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) { maliciousPath := `.github/agents/a";id;"b.md` - t.Run("codex_emits_failing_step_for_invalid_path", func(t *testing.T) { + // Codex does not handle agent files natively; path validation is done by the compiler + // at compile time (validateAgentFile). The engine step should proceed normally and never + // reference the agent file path directly. + t.Run("codex_ignores_agent_path_in_step_for_invalid_path", func(t *testing.T) { engine := NewCodexEngine() workflowData := &WorkflowData{ Name: "test-workflow", @@ -370,18 +520,15 @@ func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) { steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log") if len(steps) != 1 { - t.Fatalf("Expected exactly 1 failing step, got %d", len(steps)) + t.Fatalf("Expected exactly 1 step, got %d", len(steps)) } content := strings.Join([]string(steps[0]), "\n") - if !strings.Contains(content, "exit 1") { - t.Errorf("Expected failing step with 'exit 1', got:\n%s", content) - } - if !strings.Contains(content, "Error") { - t.Errorf("Expected error message in failing step, got:\n%s", content) + // Must NOT reference the malicious path at all in the generated step + if strings.Contains(content, maliciousPath) { + t.Errorf("Codex step must not reference the agent file path directly, got:\n%s", content) } - // Must NOT invoke awk (that would mean the path was used for real execution) if strings.Contains(content, "awk") { - t.Errorf("Failing step must not invoke awk with the invalid path, got:\n%s", content) + t.Errorf("Codex step must not invoke awk for agent file reading, got:\n%s", content) } }) diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 19520601305..7cd0cc2b5e5 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -26,6 +26,7 @@ func NewGeminiEngine() *GeminiEngine { supportsMaxTurns: false, supportsMaxContinuations: false, // Gemini CLI does not support --max-autopilot-continues-style continuation mode supportsWebSearch: false, + supportsNativeAgentFile: false, // Gemini does not support agent file natively; the compiler prepends the agent file content to prompt.txt llmGatewayPort: constants.GeminiLLMGatewayPort, }, }