diff --git a/pkg/workflow/codex_engine_test.go b/pkg/workflow/codex_engine_test.go index 83f2af308fa..5de9038cc34 100644 --- a/pkg/workflow/codex_engine_test.go +++ b/pkg/workflow/codex_engine_test.go @@ -1294,3 +1294,38 @@ func TestCodexEngineExecutionCustomHarness(t *testing.T) { t.Errorf("Expected default harness path to be overridden, got:\n%s", stepContent) } } + +// TestCodexEngineForwardsSafeOutputsInputEnvVars verifies that GH_AW_INPUT_* variables +// from the safe-outputs config are included in the Codex execution step env. +// Codex uses the TOML-based MCP setup; the TOML env_vars list references these vars +// so the safe-outputs container can resolve ${GH_AW_INPUT_…} placeholders in config.json. +// AWF passes them into the sandbox via --env-all, enabling the forwarding chain. +func TestCodexEngineForwardsSafeOutputsInputEnvVars(t *testing.T) { + engine := NewCodexEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + SafeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + TargetRepoSlug: "${{ inputs.owner }}/${{ inputs.repo }}", + }, + }, + SafeOutputsInputEnvVars: map[string]string{ + "GH_AW_INPUT_OWNER": "${{ inputs.owner }}", + "GH_AW_INPUT_REPO": "${{ inputs.repo }}", + }, + } + + steps := engine.GetExecutionSteps(workflowData, "test-log") + if len(steps) != 1 { + t.Fatalf("Expected 1 execution step, got %d", len(steps)) + } + stepContent := strings.Join([]string(steps[0]), "\n") + + if !strings.Contains(stepContent, "GH_AW_INPUT_OWNER: ${{ inputs.owner }}") { + t.Errorf("Expected GH_AW_INPUT_OWNER in step env for TOML env_vars forwarding, got:\n%s", stepContent) + } + if !strings.Contains(stepContent, "GH_AW_INPUT_REPO: ${{ inputs.repo }}") { + t.Errorf("Expected GH_AW_INPUT_REPO in step env for TOML env_vars forwarding, got:\n%s", stepContent) + } +} diff --git a/pkg/workflow/copilot_engine_test.go b/pkg/workflow/copilot_engine_test.go index 0d73dccbd86..8443be4b3ff 100644 --- a/pkg/workflow/copilot_engine_test.go +++ b/pkg/workflow/copilot_engine_test.go @@ -3096,3 +3096,40 @@ func TestCopilotEngineLLMProviderAnthropicAutoBYOK(t *testing.T) { t.Errorf("COPILOT_GITHUB_TOKEN should be omitted in auto-BYOK mode, got:\n%s", stepContent) } } + +// TestCopilotEngineForwardsSafeOutputsInputEnvVars verifies that GH_AW_INPUT_* variables +// extracted from the safe-outputs config are included in the Copilot execution step env. +// These vars must reach the agent step so that the TOML env_vars forwarding chain +// (runner step → AWF sandbox → Copilot CLI → safe-outputs container) can resolve +// ${GH_AW_INPUT_…} placeholders in config.json. Without this, a dynamic target-repo +// like "${{ inputs.owner }}/${{ inputs.repo }}" stays unexpanded in the patch filename, +// causing a mismatch with the consumer's lookup and silently dropping the PR. +func TestCopilotEngineForwardsSafeOutputsInputEnvVars(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + SafeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + TargetRepoSlug: "${{ inputs.owner }}/${{ inputs.repo }}", + }, + }, + SafeOutputsInputEnvVars: map[string]string{ + "GH_AW_INPUT_OWNER": "${{ inputs.owner }}", + "GH_AW_INPUT_REPO": "${{ inputs.repo }}", + }, + } + + steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") + if len(steps) != 1 { + t.Fatalf("Expected 1 execution step, got %d", len(steps)) + } + stepContent := strings.Join([]string(steps[0]), "\n") + + if !strings.Contains(stepContent, "GH_AW_INPUT_OWNER: ${{ inputs.owner }}") { + t.Errorf("Expected GH_AW_INPUT_OWNER in step env for TOML env_vars forwarding, got:\n%s", stepContent) + } + if !strings.Contains(stepContent, "GH_AW_INPUT_REPO: ${{ inputs.repo }}") { + t.Errorf("Expected GH_AW_INPUT_REPO in step env for TOML env_vars forwarding, got:\n%s", stepContent) + } +} diff --git a/pkg/workflow/safe_output_helpers_test.go b/pkg/workflow/safe_output_helpers_test.go index a2b4c12021c..a5ed2104c22 100644 --- a/pkg/workflow/safe_output_helpers_test.go +++ b/pkg/workflow/safe_output_helpers_test.go @@ -272,6 +272,21 @@ func TestApplySafeOutputEnvToMap(t *testing.T) { "GH_AW_ASSETS_ALLOWED_EXTS": "\".png,.jpg,.jpeg\"", }, }, + { + name: "safe outputs input env vars forwarded to agent step", + workflowData: &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{}, + SafeOutputsInputEnvVars: map[string]string{ + "GH_AW_INPUT_OWNER": "${{ inputs.owner }}", + "GH_AW_INPUT_REPO": "${{ inputs.repo }}", + }, + }, + expected: map[string]string{ + "GH_AW_SAFE_OUTPUTS": "${{ steps.set-runtime-paths.outputs.GH_AW_SAFE_OUTPUTS }}", + "GH_AW_INPUT_OWNER": "${{ inputs.owner }}", + "GH_AW_INPUT_REPO": "${{ inputs.repo }}", + }, + }, } for _, tt := range tests { diff --git a/pkg/workflow/safe_outputs_env.go b/pkg/workflow/safe_outputs_env.go index d1f9574596b..76bf3086599 100644 --- a/pkg/workflow/safe_outputs_env.go +++ b/pkg/workflow/safe_outputs_env.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "strconv" "strings" @@ -71,6 +72,16 @@ func applySafeOutputEnvToMap(env map[string]string, data *WorkflowData) { env["GH_AW_ASSETS_MAX_SIZE_KB"] = strconv.Itoa(data.SafeOutputs.UploadAssets.MaxSizeKB) env["GH_AW_ASSETS_ALLOWED_EXTS"] = fmt.Sprintf("%q", strings.Join(data.SafeOutputs.UploadAssets.AllowedExts, ",")) } + + // Forward GH_AW_INPUT_* vars extracted from the safe-outputs config so that + // TOML-based MCP clients (Codex, Copilot) can pass them through to the + // containerised safe-outputs MCP server via the TOML env_vars allowlist. + // Without this, ${GH_AW_INPUT_…} placeholders written into config.json at + // compile time cannot be resolved inside the container, causing a dynamic + // target-repo value (e.g. "${{ inputs.owner }}/${{ inputs.repo }}") to survive + // unexpanded as the patch filename. The consumer's lookup then fails to match, + // silently dropping the pull-request creation. + maps.Copy(env, data.SafeOutputsInputEnvVars) } // buildWorkflowMetadataEnvVars builds workflow name and source environment variables