fix: Codex no longer fails to load its config after uLoopMCP rewrites a codex mcp add entry

[hatayama]

Summary

• Codex configs can end up with a duplicate env definition when a table-style [mcp_servers.uLoopMCP.env] section coexists with uLoopMCP's own inline env = {...} line, causing Codex to fail parsing config.toml entirely.
• This PR removes that table-style env section on every config write/delete path so Codex configs are always valid.

User Impact

• How the duplicate appears: codex mcp add uLoopMCP ... rewrites the whole [mcp_servers.uLoopMCP] section in table style, replacing the inline env = {...} line with a [mcp_servers.uLoopMCP.env] child table. That config on its own is valid. uLoopMCP's AutoConfigure then replaces only what SectionRegex matches, and that regex stops at the child table's own [, so the table survives and the rebuilt block adds an inline env line back — leaving two definitions of the same key.
• Before: with both definitions present, Codex refused the whole config.toml with a duplicate-key parse error, so every MCP server in that file stopped working, not just uLoopMCP.
• After: AutoConfigure, UpdateDevelopmentSettings, and DeleteConfiguration all strip that table before writing, so re-saving the config (which already happens automatically) heals it.

Changes

• Add LegacyEnvTableRegex to match the table-style [mcp_servers.uLoopMCP.env] section (the existing SectionRegex only matches up to the next top-level [, so it never covered this child table).
• Add a pure RemoveLegacyEnvTable(content) helper and apply it in AutoConfigure's content-building step, UpdateDevelopmentSettings, and DeleteConfiguration.
• Extract AutoConfigure's string-building logic into BuildAutoConfiguredContent so the write flow can be covered by tests at the string level, without touching disk.
• Extract BuildDeletedContent and BuildDevelopmentSettingsContent for the same reason, so the strip applied on the delete and development-settings paths is covered by tests instead of only by the write path. BuildDeletedContent keeps the "nothing was removed" short-circuit, so a config holding no uLoopMCP entries still keeps its blank lines untouched; BuildDevelopmentSettingsContent reports whether the section exists, which collapses UpdateDevelopmentSettings' two consecutive AutoConfigure retries into one.
• IsUpdateNeeded now also returns true when the table-style section is present, so an existing bad config gets rewritten. This check is placed after the server-bundle existence guard, so an environment missing the TypeScript server bundle still returns false instead of reaching AutoConfigure's throw path.
• The table is dropped rather than migrated: a duplicate env definition makes Codex reject the whole config, and AutoConfigure / UpdateDevelopmentSettings regenerate the env values uLoopMCP manages. Any custom keys that only existed in the table are intentionally discarded, matching AutoConfigure's existing behavior of rebuilding the section.
• No line-ending normalization was added for the surviving file content, to avoid corrupting unrelated sections or mixing line endings on Windows.

Supersedes #1858

This supersedes #1858, which reported and fixed the same bug but had a few issues this PR avoids:

• fix(codex): migrate legacy env table configuration #1858 applied Regex.Replace(result, @"(\r?\n){3,}", Environment.NewLine + Environment.NewLine) to the entire file content, rewriting blank lines in sections unrelated to uLoopMCP, and using the platform-dependent Environment.NewLine, which can introduce CRLF into an otherwise-LF file on Windows. This PR's RemoveLegacyEnvTable does not touch line endings anywhere in the file.
• fix(codex): migrate legacy env table configuration #1858 placed the table-style check in IsUpdateNeeded before the TypeScript server bundle guard, so an environment without the server bundle present would return true and let AutoConfigure throw. This PR places the check after the guard.
• fix(codex): migrate legacy env table configuration #1858's tests only covered private helper methods in isolation and did not exercise the actual write flow, so a regression in how AutoConfigure assembles the final content would not have been caught. This PR adds a regression test that runs the extracted BuildAutoConfiguredContent write-flow function directly.

Credit

The bug was originally reported and fixed by @rougebleu in #1858. The branch commit carries the
trailer below; because this repository squashes with an empty commit body, it has to be supplied
in the squash message at merge time to survive:

Co-authored-by: rougebleu <rye0126@gmail.com>

Verification

• Added 9 new tests to Assets/Tests/Editor/CodexConfigTests.cs: RemoveLegacyEnvTable (removal, no-op when absent, end-of-file table, CRLF preservation), and write-flow regression tests for BuildAutoConfiguredContent, BuildDeletedContent, and BuildDevelopmentSettingsContent.
• Ran the full CodexConfigTests + CodexConfigDeleteTests suites in Unity 2022.3.62f3 EditMode batch mode: 26/26 passed, 0 failed, 0 compile errors.
• Verified the real write paths against an on-disk {projectRoot}/.codex/config.toml seeded with both an inline env line and a legacy table, driving IsConfigured / IsUpdateNeeded / AutoConfigure, DeleteConfiguration, and UpdateDevelopmentSettings in Unity batch mode. All three produced a single env definition (or removed the section entirely, for delete), and codex mcp list (codex-cli 0.145.0, CODEX_HOME pointed at that directory) listed both servers successfully.
• Ran the same on-disk sequence on origin/main as a control: the legacy table survived, and codex mcp list failed with duplicate key at the [mcp_servers.uLoopMCP.env] line. Same harness, opposite outcome — the healing is this PR's effect.

Summary

Fix Codex TOML configuration parsing for uLoopMCP configs that contain both an inline TOML env = { ... } entry and a legacy [mcp_servers.uLoopMCP.env] table.

• Added LegacyEnvTableRegex and a pure RemoveLegacyEnvTable helper to remove only the legacy env table while preserving the inline env, unrelated sections, and existing line endings (including CRLF).
• Updated IsUpdateNeeded to detect the legacy table (after confirming the TypeScript server bundle exists), and routed write flows through legacy cleanup:
  • BuildAutoConfiguredContent strips the legacy table before updating/inserting the [mcp_servers.uLoopMCP] block.
  • DeleteConfiguration now uses BuildDeletedContent to remove the uLoopMCP section and discard the legacy env table, returning unchanged content when nothing is removed.
  • UpdateDevelopmentSettings now strips the legacy table up front via BuildDevelopmentSettingsContent before applying development/debug log changes.
• Extracted/used content-building helpers to centralize write logic for the auto-config, development-settings, and delete paths.
• Extended NUnit coverage in Assets/Tests/Editor/CodexConfigTests.cs for legacy table removal (including end-of-file), unchanged/no-op cases, CRLF preservation, and ensuring the output contains a single env = definition when both forms exist. Reflection-based helpers and a CountOccurrences utility were added to support the new assertions.

Codex configuration and deletion test suites pass 22/22 tests in Unity 2022.3.62f3.

[coderabbitai]

📝 Walkthrough

Walkthrough

Codex TOML detection and write paths now remove the legacy [mcp_servers.uLoopMCP.env] table. Editor tests cover preservation of inline environment entries, unrelated sections, line endings, unchanged results, and single environment definitions.

Changes

Codex environment table cleanup

Layer / File(s)	Summary
Legacy table detection and removal Packages/src/Editor/Config/CodexTomlConfigService.cs	Detects the legacy environment table and removes it during auto-configuration, deletion, and development-settings updates.
TOML transformation coverage Assets/Tests/Editor/CodexConfigTests.cs	Tests removal, absent and EOF cases, CRLF preservation, section retention, unchanged behavior, and duplicate environment prevention using reflection helpers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• hatayama/unity-cli-loop#1858: Covers the same legacy TOML environment-table migration and related editor tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: preventing Codex config load failures after uLoopMCP rewrites a codex mcp add entry.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/codex-legacy-env-table-removal

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}