Prevent Codex MCP gateway startup failures from config.toml self-copy

[Copilot]

Recent Codex workflows using MCP CLI servers (serena, mempalace) were failing in Start MCP Gateway despite successful gateway health and MCP connectivity checks. The step was aborting on a same-file cp operation when syncing Codex MCP config.

• Root cause

  • The generated Codex MCP sync script always copied:
    • source: /tmp/gh-aw/mcp-config/config.toml
    • destination: ${CODEX_HOME}/config.toml
  • In affected runs, CODEX_HOME=/tmp/gh-aw/mcp-config, so cp attempted a self-copy and exited non-zero.

• Compiler change (runtime script generation)

  • Updated Codex MCP config rendering to guard the copy and skip it when source and destination are identical.
  • File: pkg/workflow/codex_mcp.go

• Test updates

  • Updated expected generated script content for Codex MCP rendering.
  • File: pkg/workflow/codex_engine_test.go

# before
cp "/tmp/gh-aw/mcp-config/config.toml" "${CODEX_HOME}/config.toml"

# after
if [ "/tmp/gh-aw/mcp-config/config.toml" != "${CODEX_HOME}/config.toml" ]; then
  cp "/tmp/gh-aw/mcp-config/config.toml" "${CODEX_HOME}/config.toml"
fi

[pelikhan]

@copilot recompile

[Copilot]

Pull request overview

Prevents Codex MCP gateway startup from failing when the generated MCP config sync script attempts to cp a file onto itself (same source/destination path).

Changes:

• Add a conditional guard around copying /tmp/gh-aw/mcp-config/config.toml into ${CODEX_HOME}/config.toml during Codex MCP config sync.
• Update the Codex MCP config rendering test expectations to match the newly generated script line.

Show a summary per file

File	Description
pkg/workflow/codex_mcp.go	Updates generated shell script to skip cp when source/destination paths match.
pkg/workflow/codex_engine_test.go	Adjusts expected rendered script lines for MCP config generation.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Copilot]

The self-copy guard compares path strings, which won’t catch cases where ${CODEX_HOME} resolves to the same file via a trailing slash (/tmp/.../mcp-config/), .., or a symlink; cp will still treat the files as identical and exit non-zero. Consider using a filesystem identity check (e.g., test ... -ef ... / [ ... -ef ... ] with a negation) instead of string inequality, and emit the if/then/cp/fi as a multi-line block like other generated run scripts for readability/consistency (see pkg/workflow/compiler_yaml_ai_execution.go:161-164).

Suggested change

	yaml.WriteString(" if [ \"/tmp/gh-aw/mcp-config/config.toml\" != \"${CODEX_HOME}/config.toml\" ]; then cp \"/tmp/gh-aw/mcp-config/config.toml\" \"${CODEX_HOME}/config.toml\"; fi\n")
	yaml.WriteString(" if ! [ \"/tmp/gh-aw/mcp-config/config.toml\" -ef \"${CODEX_HOME}/config.toml\" ]; then\n")
	yaml.WriteString(" cp \"/tmp/gh-aw/mcp-config/config.toml\" \"${CODEX_HOME}/config.toml\"\n")
	yaml.WriteString(" fi\n")

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	No (1:1 ratio)
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
TestCodexEngineRenderMCPConfig	pkg/workflow/codex_engine_test.go:191	✅ Design	None

---

Analysis

This PR makes a single targeted change: the cp command that copies config.toml to \$\{CODEX_HOME} is now wrapped in an if guard to prevent a self-copy when source and destination paths resolve to the same file.

The corresponding test update in TestCodexEngineRenderMCPConfig updates the expected shell-script output to match. This is a behavioral contract test — it directly verifies the exact shell commands emitted by RenderMCPConfig, which is exactly the observable output that matters. The test:

• Uses a table-driven structure with t.Run() — preferred pattern ✓
• Includes t.Fatalf on the error return of RenderMCPConfig — error path coverage ✓
• Has the required //go:build !integration build tag on line 1 ✓
• Uses no mock libraries ✓
• 1:1 test-to-production line ratio (1 line changed in each) — no test inflation ✓

What invariant does this test enforce? When RenderMCPConfig generates the MCP config setup script, it must emit a conditional guard around the cp command so the file is only copied when source ≠ destination. Deleting or reverting this test update would allow the self-copy regression to re-enter silently.

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 1 test — unit (//go:build !integration)
• 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

---

Verdict

✅ Check passed. 0% of new/modified tests are implementation tests (threshold: 30%). The test update faithfully captures the behavioral contract enforced by the bug fix.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24729659653

🧪 Test quality analysis by Test Quality Sentinel · ● 474.5K · ◷

[github-actions]

✅ Test Quality Sentinel: 100/100. Test quality is acceptable — 0% of new/modified tests are implementation tests (threshold: 30%). The single modified test (TestCodexEngineRenderMCPConfig) is a well-structured behavioral contract test that correctly captures the self-copy guard introduced by this fix.