Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ai-moderator.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/changeset.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/codex-github-remote-mcp-test.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/daily-fact.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/daily-observability-report.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/duplicate-code-detector.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/grumpy-reviewer.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/issue-arborist.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/schema-feature-coverage.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/smoke-call-workflow.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/smoke-codex.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions pkg/workflow/codex_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,33 @@ func TestCodexEngineRenderMCPConfigOpenAIProxyProvider(t *testing.T) {
if !strings.Contains(result, "awk '") {
t.Errorf("Expected firewall-enabled config append to use awk filtering, got:\n%s", result)
}

normalizedResult := normalizeHeredocDelimiters(result)
syncStart := strings.Index(normalizedResult, "cat > \"/tmp/gh-aw/mcp-config/config.toml\" << GH_AW_CODEX_SHELL_POLICY_NORM_EOF")
if syncStart == -1 {
t.Fatalf("Expected config sync heredoc start in generated config, got:\n%s", normalizedResult)
}

syncBodyStart := strings.Index(normalizedResult[syncStart:], "\n")
if syncBodyStart == -1 {
t.Fatalf("Expected newline after config sync heredoc start, got:\n%s", normalizedResult[syncStart:])
}
syncBodyOffset := syncStart + syncBodyStart + 1

syncEnd := strings.Index(normalizedResult[syncBodyOffset:], "\n GH_AW_CODEX_SHELL_POLICY_NORM_EOF")
if syncEnd == -1 {
t.Fatalf("Expected config sync heredoc end in generated config, got:\n%s", normalizedResult)
}

syncBlock := normalizedResult[syncBodyOffset : syncBodyOffset+syncEnd]
modelProviderIndex := strings.Index(syncBlock, "model_provider = \"openai-proxy\"")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good defensive test: verifying that model_provider index comes before [shell_environment_policy] index in the sync block directly encodes the TOML ordering contract. This prevents future regressions if someone reorders the render calls again.

shellPolicyIndex := strings.Index(syncBlock, "[shell_environment_policy]")
if modelProviderIndex == -1 || shellPolicyIndex == -1 {
t.Fatalf("Expected model_provider and shell_environment_policy in sync block, got:\n%s", syncBlock)
}
if modelProviderIndex > shellPolicyIndex {
t.Errorf("Expected model_provider to be emitted before [shell_environment_policy] in sync block, got:\n%s", syncBlock)
}
})

t.Run("does not inject openai-proxy provider when firewall is disabled", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/codex_mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an

shellPolicyDelimiter := GenerateHeredocDelimiterFromSeed("CODEX_SHELL_POLICY", workflowData.FrontmatterHash)
yaml.WriteString(" cat > \"/tmp/gh-aw/mcp-config/config.toml\" << " + shellPolicyDelimiter + "\n")
e.renderShellEnvironmentPolicyToml(yaml, tools, mcpTools, " ")
if isFirewallEnabled(workflowData) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reordering of renderShellEnvironmentPolicyToml after renderOpenAIProxyProviderToml correctly ensures model_provider appears as a root-level TOML key before any table section ([shell_environment_policy]). This is a good fix — TOML tables scope subsequent keys, so placement order matters.

e.renderOpenAIProxyProviderToml(yaml, " ")
}
e.renderShellEnvironmentPolicyToml(yaml, tools, mcpTools, " ")
yaml.WriteString(" " + shellPolicyDelimiter + "\n")
if isFirewallEnabled(workflowData) {
e.renderAppendConvertedConfigWithoutOpenAIProxy(yaml)
Expand Down
Loading