[PR #5054] [CLOSED] fix(setup): admit only syntax Gemini CLI loads in settings.json #6762

Closed
opened 2026-11-01 21:42:51 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5054
Author: @cpruijsen
Created: 10/18/2026
Status: ❌ Closed

Base: main ← Head: fix/setup-gemini-settings-syntax


📝 Commits (2)

  • 9a7c0b2 fix(setup): admit only syntax Gemini CLI loads in settings.json
  • 0dda76a Merge branch 'main' into fix/setup-gemini-settings-syntax

📊 Changes

4 files changed (+31 additions, -4 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.shape.test.ts (+21 -0)
📝 src/application/ClientConfigurationDocument.ts (+6 -1)
📝 src/application/GeminiClientSettings.ts (+2 -2)
📝 src/application/SupportedClients.ts (+2 -1)

📄 Description

Summary

Gemini CLI loads settings.json with JSON.parse after it strips comments. A trailing comma or a UTF-8 BOM makes that parse fail, and Gemini then loads no settings, so no MCP servers, from that file. REA parsed Gemini settings as JSONC that allows trailing commas and a BOM. For such a file, setup reported ready and wrote into it, and doctor reported the registration aligned.

This gives Gemini CLI its own gemini format with the syntax Gemini accepts. It is the same class of fix as #4944 (Claude Code), but Gemini accepts comments, so this format cannot be strict JSON.

No issue is linked. I found this while comparing each client's native settings reader with REA's parser.

Problem and expected behavior

Smallest trigger: ~/.gemini/settings.json containing {"mcpServers": {"peer": {"command": "/usr/bin/true"}},}.

  • Base: rea setup --client gemini_cli --yes returns ready, writes the registration and keeps the trailing comma. rea doctor --client gemini_cli reports aligned. Gemini CLI cannot parse the result.
  • Expected: setup refuses the file as malformed before any backup or write, and doctor reports invalid. A file with comments still configures, because Gemini CLI strips them.

Change and scope

  • SupportedClients.ts: a gemini format value, used by gemini_cli.
  • ClientConfigurationDocument.ts: for gemini, the JSONC parse runs with allowTrailingComma: false, and a leading BOM is refused. Every other format is unchanged.
  • GeminiClientSettings.ts: the defaults, system and workspace settings reads use gemini instead of json, because Gemini CLI loads all of its settings scopes with the same load function.

Not included: Qwen Code, a Gemini CLI fork. I checked its published package (@qwen-code/qwen-code@0.25.2): it parses settings with JSON.parse(stripJsonComments(stripUtf8Bom(content))), so unlike Gemini CLI it accepts a UTF-8 BOM. Giving Qwen the BOM-refusing gemini format would reject files Qwen can load; a Qwen dialect is a separate change. A BOM-only settings.json still counts as empty, as for every other format.

Scope: an ordinary fix inside the existing setup boundary. A maintainer scope decision is not applicable.

What I chose and the alternative: the regression test is a module-level shape test on parseClientConfiguration with the format taken from the real gemini_cli client definition (21 lines), and the end-to-end setup/doctor/uninstall behavior was verified by hand on the CLI (output below). The alternative, a CLI acceptance test mirroring claudeCodeStrictJsonSetup.test.ts from #4944, would add roughly 70-130 lines; I kept the diff minimal. I can add it if you want parity with the precedent.

Contract and boundary impact

  • Semantic owner and earliest changed stage: client configuration parsing (parseDocument in ClientConfigurationDocument.ts), before planning.

  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows #4944).

  • CLI and MCP/tool-catalog contract: setup's client format for Gemini CLI changes from "json" to "gemini". For a trailing comma or a BOM, setup and uninstall now return needs_human / failed with a malformed remediation. No tool catalog changes.

  • Provider, bridge, target-format, or platform compatibility: Gemini CLI on every platform.

  • Evidence, artifact, provenance, or reconstruction contract: none.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata: the generated product catalog's setup_clients[].format for gemini_cli becomes "gemini" (regenerated, not committed). npm run docs:check passes.

  • Read existing files or captured data.

  • Change process memory, registers, files, or configuration. (Setup now refuses a write it used to make.)

Evidence and regression coverage

  • Tests added or updated: src/application/ClientConfigurationDocument.shape.test.ts gets a "Gemini CLI settings syntax" block. It takes the format from the real gemini_cli client definition and checks that comments parse and that a trailing comma and a BOM throw SyntaxError.
  • Base reproduction: with the three src/ files at the merge base and the test file kept, the trailing-comma and BOM cases fail (AssertionError: expected function to throw an error, but it didn't) and the other 8 tests pass. With the change, 10 of 10 pass.
  • End-to-end CLI checks on this change (isolated HOME, macOS): a trailing-comma file and a BOM file each make setup return needs_human with exit 1, leave the file byte-identical, create no .rea.backup, and make doctor report invalid; a comment-only file configures, keeps the comment, and reports aligned; a BOM-only file counts as empty and configures; a trailing comma in the system settings file (via GEMINI_CLI_SYSTEM_SETTINGS_PATH) makes setup refuse and name the system file; uninstall with a malformed user file reports failed and does not change it.
  • Gemini CLI reader (verified against @google/gemini-cli@0.64.0 from npm, the latest on 2026-10-18): one settings load function reads each file with readFileSync(filePath, "utf-8") and parses it with JSON.parse(stripJsonComments(content)); user and workspace scopes call it directly, and system and system-defaults reach it through loadSystemFile. On a parse error it records a settings error and loads {} for that file, so no MCP servers. The bundled strip-json-comments has no trailing-comma option. Running that bundled function: a comment is accepted, a trailing comma fails with "Expected double-quoted property name", and a BOM fails with "Unexpected token".
  • Remaining proof gaps: I did not run the Gemini CLI binary against a file that REA wrote; the client behavior above is source-derived from the published package. The system, defaults and workspace read change has no dedicated new test; the existing workspace-trust and profile suites below exercise those reads. Tests ran on macOS only.

Validation performed

  • npm run test:focused -- src/application/ClientConfigurationDocument.shape.test.ts: 10 passed. With the src/ change at the merge base (isolated worktree, test file kept): 2 failed, 8 passed.
  • npx vitest run --testTimeout=180000 on clientRegistrationStatus, clientLifecycle, setupClientConfigCleanupOwnership, clientConfigurationBom, clientConfigurationDialects (boundary/filesystem), clientProfiles, clientProfileLifecycle (boundary/cli), geminiWorkspaceTrust and clientConfigurationDiagnosticsCli (acceptance): all pass. A first combined run had 4 timeout failures (Test process exceeded its 20000ms deadline from the process fixture, on a loaded machine); each affected file passed in an isolated re-run (clientProfiles 15/15, clientProfileLifecycle 5/5, geminiWorkspaceTrust 5/5).
  • npm run check:fast: passed, re-run uncached with npx turbo run typecheck lint --force.
  • npm run verify:test-discovery: passed.
  • npm run docs:check: passed.
  • npx oxfmt --check on the changed files: passed.
  • Platform: macOS (arm64), Node 26.

Compatibility, safety, and release

  • Breaking changes or migration steps: none for files Gemini CLI can load. A Gemini settings file with a trailing comma or a BOM was already unreadable by Gemini CLI. Setup now asks the user to fix it instead of writing into it.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tests ran on macOS only.
  • Package or release metadata impact: the generated product catalog format value for gemini_cli changes (regenerated, not committed).
  • Security, privacy, process, or containment review: none. The change only refuses a write.

Author readiness

Human owner: @cpruijsen

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/5054 **Author:** [@cpruijsen](https://github.com/cpruijsen) **Created:** 10/18/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/setup-gemini-settings-syntax` --- ### 📝 Commits (2) - [`9a7c0b2`](https://github.com/morluto/rea/commit/9a7c0b27a99d14a9a6c7bc0e21bca0c6199eb1dd) fix(setup): admit only syntax Gemini CLI loads in settings.json - [`0dda76a`](https://github.com/morluto/rea/commit/0dda76a38eca35858cae918d733b57baff0a4816) Merge branch 'main' into fix/setup-gemini-settings-syntax ### 📊 Changes **4 files changed** (+31 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.shape.test.ts` (+21 -0) 📝 `src/application/ClientConfigurationDocument.ts` (+6 -1) 📝 `src/application/GeminiClientSettings.ts` (+2 -2) 📝 `src/application/SupportedClients.ts` (+2 -1) </details> ### 📄 Description ## Summary Gemini CLI loads `settings.json` with `JSON.parse` after it strips comments. A trailing comma or a UTF-8 BOM makes that parse fail, and Gemini then loads no settings, so no MCP servers, from that file. REA parsed Gemini settings as JSONC that allows trailing commas and a BOM. For such a file, setup reported `ready` and wrote into it, and doctor reported the registration `aligned`. This gives Gemini CLI its own `gemini` format with the syntax Gemini accepts. It is the same class of fix as #4944 (Claude Code), but Gemini accepts comments, so this format cannot be strict JSON. No issue is linked. I found this while comparing each client's native settings reader with REA's parser. ## Problem and expected behavior Smallest trigger: `~/.gemini/settings.json` containing `{"mcpServers": {"peer": {"command": "/usr/bin/true"}},}`. - Base: `rea setup --client gemini_cli --yes` returns `ready`, writes the registration and keeps the trailing comma. `rea doctor --client gemini_cli` reports `aligned`. Gemini CLI cannot parse the result. - Expected: setup refuses the file as malformed before any backup or write, and doctor reports `invalid`. A file with comments still configures, because Gemini CLI strips them. ## Change and scope - `SupportedClients.ts`: a `gemini` format value, used by `gemini_cli`. - `ClientConfigurationDocument.ts`: for `gemini`, the JSONC parse runs with `allowTrailingComma: false`, and a leading BOM is refused. Every other format is unchanged. - `GeminiClientSettings.ts`: the defaults, system and workspace settings reads use `gemini` instead of `json`, because Gemini CLI loads all of its settings scopes with the same `load` function. Not included: Qwen Code, a Gemini CLI fork. I checked its published package (`@qwen-code/qwen-code@0.25.2`): it parses settings with `JSON.parse(stripJsonComments(stripUtf8Bom(content)))`, so unlike Gemini CLI it accepts a UTF-8 BOM. Giving Qwen the BOM-refusing `gemini` format would reject files Qwen can load; a Qwen dialect is a separate change. A BOM-only `settings.json` still counts as empty, as for every other format. Scope: an ordinary fix inside the existing setup boundary. A maintainer scope decision is not applicable. What I chose and the alternative: the regression test is a module-level shape test on `parseClientConfiguration` with the format taken from the real `gemini_cli` client definition (21 lines), and the end-to-end setup/doctor/uninstall behavior was verified by hand on the CLI (output below). The alternative, a CLI acceptance test mirroring `claudeCodeStrictJsonSetup.test.ts` from #4944, would add roughly 70-130 lines; I kept the diff minimal. I can add it if you want parity with the precedent. ## Contract and boundary impact - Semantic owner and earliest changed stage: client configuration parsing (`parseDocument` in `ClientConfigurationDocument.ts`), before planning. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows #4944). - CLI and MCP/tool-catalog contract: setup's client `format` for Gemini CLI changes from `"json"` to `"gemini"`. For a trailing comma or a BOM, setup and uninstall now return `needs_human` / `failed` with a `malformed` remediation. No tool catalog changes. - Provider, bridge, target-format, or platform compatibility: Gemini CLI on every platform. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata: the generated product catalog's `setup_clients[].format` for `gemini_cli` becomes `"gemini"` (regenerated, not committed). `npm run docs:check` passes. - [x] Read existing files or captured data. - [x] Change process memory, registers, files, or configuration. (Setup now refuses a write it used to make.) ## Evidence and regression coverage - Tests added or updated: `src/application/ClientConfigurationDocument.shape.test.ts` gets a "Gemini CLI settings syntax" block. It takes the format from the real `gemini_cli` client definition and checks that comments parse and that a trailing comma and a BOM throw `SyntaxError`. - Base reproduction: with the three `src/` files at the merge base and the test file kept, the trailing-comma and BOM cases fail (`AssertionError: expected function to throw an error, but it didn't`) and the other 8 tests pass. With the change, 10 of 10 pass. - End-to-end CLI checks on this change (isolated HOME, macOS): a trailing-comma file and a BOM file each make setup return `needs_human` with exit 1, leave the file byte-identical, create no `.rea.backup`, and make doctor report `invalid`; a comment-only file configures, keeps the comment, and reports `aligned`; a BOM-only file counts as empty and configures; a trailing comma in the system settings file (via `GEMINI_CLI_SYSTEM_SETTINGS_PATH`) makes setup refuse and name the system file; uninstall with a malformed user file reports `failed` and does not change it. - Gemini CLI reader (verified against `@google/gemini-cli@0.64.0` from npm, the latest on 2026-10-18): one settings `load` function reads each file with `readFileSync(filePath, "utf-8")` and parses it with `JSON.parse(stripJsonComments(content))`; user and workspace scopes call it directly, and system and system-defaults reach it through `loadSystemFile`. On a parse error it records a settings error and loads `{}` for that file, so no MCP servers. The bundled strip-json-comments has no trailing-comma option. Running that bundled function: a comment is accepted, a trailing comma fails with "Expected double-quoted property name", and a BOM fails with "Unexpected token". - Remaining proof gaps: I did not run the Gemini CLI binary against a file that REA wrote; the client behavior above is source-derived from the published package. The system, defaults and workspace read change has no dedicated new test; the existing workspace-trust and profile suites below exercise those reads. Tests ran on macOS only. ## Validation performed - `npm run test:focused -- src/application/ClientConfigurationDocument.shape.test.ts`: 10 passed. With the `src/` change at the merge base (isolated worktree, test file kept): 2 failed, 8 passed. - `npx vitest run --testTimeout=180000` on `clientRegistrationStatus`, `clientLifecycle`, `setupClientConfigCleanupOwnership`, `clientConfigurationBom`, `clientConfigurationDialects` (boundary/filesystem), `clientProfiles`, `clientProfileLifecycle` (boundary/cli), `geminiWorkspaceTrust` and `clientConfigurationDiagnosticsCli` (acceptance): all pass. A first combined run had 4 timeout failures (`Test process exceeded its 20000ms deadline` from the process fixture, on a loaded machine); each affected file passed in an isolated re-run (clientProfiles 15/15, clientProfileLifecycle 5/5, geminiWorkspaceTrust 5/5). - `npm run check:fast`: passed, re-run uncached with `npx turbo run typecheck lint --force`. - `npm run verify:test-discovery`: passed. - `npm run docs:check`: passed. - `npx oxfmt --check` on the changed files: passed. - Platform: macOS (arm64), Node 26. ## Compatibility, safety, and release - Breaking changes or migration steps: none for files Gemini CLI can load. A Gemini settings file with a trailing comma or a BOM was already unreadable by Gemini CLI. Setup now asks the user to fix it instead of writing into it. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tests ran on macOS only. - Package or release metadata impact: the generated product catalog format value for `gemini_cli` changes (regenerated, not committed). - Security, privacy, process, or containment review: none. The change only refuses a write. ## Author readiness Human owner: @cpruijsen - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:42:51 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6762
No description provided.