[PR #1178] [MERGED] fix(setup): keep Codex TOML comments when adding or removing REA #2702

New issue

Closed

opened 2026-10-17 21:09:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:09:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1178
Author: @kaoru0822-kitauji
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/codex-toml-preserve-comments

📝 Commits (1)

e9c48a6 fix(setup): keep Codex TOML comments when adding or removing REA

📊 Changes

3 files changed (+175 additions, -29 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.test.ts (+55 -0)
📝 src/application/ClientConfigurationDocument.ts (+91 -29)
📝 tests/boundary/filesystem/clientLifecycle.test.ts (+29 -0)

📄 Description

Summary

rea setup --client codex and rea uninstall rewrote the whole Codex config.toml with smol-toml's stringify. Every comment was lost, and unrelated values were respelled (["a", "b"] → [ "a", "b" ], 'C:\demo\path' → "C:\\demo\\path"), even though the setup plan says it will "preserve unrelated configuration". Grok Build's TOML and the JSON/JSONC clients already kept their comments.

Fixes #1163

Change (`src/application/ClientConfigurationDocument.ts`)

Codex now uses the same table editor as Grok Build (renamed from upsertGrokServerSection to upsertTomlServerSection). It removes REA's [mcp_servers.rea] tables, sub-tables, dotted assignments, and inline entries, and appends the new table, leaving every other line as written. Comments that introduce REA's own table go with it, as they already do for Grok.
The editor's root disabled_mcp_servers update is Grok Build behavior, so it runs only for Grok. A Codex file with such a list is left alone.
serializeCodexConfiguration keeps the original final line break (removing a final table used to drop it), then parses the edited text and compares it with the requested document. Only if they differ does it fall back to the previous full rewrite, which keeps every value but not comments or spelling. An empty mcp_servers table after removing REA counts as equal to no table. A missing or empty original is still written with stringify.

Issue reproduction (built branch, disposable `HOME`)

Setup:

# Keep this explanation.
notify = ["a", "b"]
literal = 'C:\demo\path'

[mcp_servers.rea]
command = "…/rea.mjs"
args = [ "mcp" ]
startup_timeout_sec = 30

Uninstall then restores the original three lines byte for byte. I also checked these by hand: other servers plus [profiles.*] with comments, an [mcp_servers.rea.env] sub-table, inline mcp_servers = { rea = {…}, other = {…} }, root dotted mcp_servers.rea.command = …, and CRLF files (line endings kept on setup and uninstall).

Tests

src/application/ClientConfigurationDocument.test.ts (new Codex TOML serialization): adding keeps the original text as a prefix; removing returns the original byte for byte; disabled_mcp_servers is left alone; a requested value the text edit cannot hold falls back to a rewrite that parses to the document. The first three fail on main.
tests/boundary/filesystem/clientLifecycle.test.ts (Codex configuration comments): configureClientConfiguration followed by runUninstall on a commented Codex file leaves the original bytes. Fails on main.

Verification

npm run check:fast, npm run jscpd (2.49%), npm run docs:check
npm run test:focused -- every test file touching client configuration serialization, Codex config.toml, or Grok (11 files, 181 passed, 1 skipped)

No Hopper/Ghidra verification was needed: the change is limited to client configuration serialization.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1178 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/codex-toml-preserve-comments` --- ### 📝 Commits (1) - [`e9c48a6`](https://github.com/morluto/rea/commit/e9c48a61e4c2c05c5b77ef4a815c5b2a65a9337e) fix(setup): keep Codex TOML comments when adding or removing REA ### 📊 Changes **3 files changed** (+175 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.test.ts` (+55 -0) 📝 `src/application/ClientConfigurationDocument.ts` (+91 -29) 📝 `tests/boundary/filesystem/clientLifecycle.test.ts` (+29 -0) </details> ### 📄 Description ## Summary `rea setup --client codex` and `rea uninstall` rewrote the whole Codex `config.toml` with `smol-toml`'s `stringify`. Every comment was lost, and unrelated values were respelled (`["a", "b"]` → `[ "a", "b" ]`, `'C:\demo\path'` → `"C:\\demo\\path"`), even though the setup plan says it will "preserve unrelated configuration". Grok Build's TOML and the JSON/JSONC clients already kept their comments. Fixes #1163 ## Change (`src/application/ClientConfigurationDocument.ts`) - Codex now uses the same table editor as Grok Build (renamed from `upsertGrokServerSection` to `upsertTomlServerSection`). It removes REA's `[mcp_servers.rea]` tables, sub-tables, dotted assignments, and inline entries, and appends the new table, leaving every other line as written. Comments that introduce REA's own table go with it, as they already do for Grok. - The editor's root `disabled_mcp_servers` update is Grok Build behavior, so it runs only for Grok. A Codex file with such a list is left alone. - `serializeCodexConfiguration` keeps the original final line break (removing a final table used to drop it), then parses the edited text and compares it with the requested document. Only if they differ does it fall back to the previous full rewrite, which keeps every value but not comments or spelling. An empty `mcp_servers` table after removing REA counts as equal to no table. A missing or empty original is still written with `stringify`. ## Issue reproduction (built branch, disposable `HOME`) Setup: ```toml # Keep this explanation. notify = ["a", "b"] literal = 'C:\demo\path' [mcp_servers.rea] command = "…/rea.mjs" args = [ "mcp" ] startup_timeout_sec = 30 ``` Uninstall then restores the original three lines byte for byte. I also checked these by hand: other servers plus `[profiles.*]` with comments, an `[mcp_servers.rea.env]` sub-table, inline `mcp_servers = { rea = {…}, other = {…} }`, root dotted `mcp_servers.rea.command = …`, and CRLF files (line endings kept on setup and uninstall). ## Tests - `src/application/ClientConfigurationDocument.test.ts` (new `Codex TOML serialization`): adding keeps the original text as a prefix; removing returns the original byte for byte; `disabled_mcp_servers` is left alone; a requested value the text edit cannot hold falls back to a rewrite that parses to the document. The first three fail on `main`. - `tests/boundary/filesystem/clientLifecycle.test.ts` (`Codex configuration comments`): `configureClientConfiguration` followed by `runUninstall` on a commented Codex file leaves the original bytes. Fails on `main`. ## Verification - `npm run check:fast`, `npm run jscpd` (2.49%), `npm run docs:check` - `npm run test:focused --` every test file touching client configuration serialization, Codex `config.toml`, or Grok (11 files, 181 passed, 1 skipped) No Hopper/Ghidra verification was needed: the change is limited to client configuration serialization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:09:14 +00:00