[PR #5115] [CLOSED] fix(setup): accept and preserve Codex per-tool settings (#3862, #3863) #6771

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5115
Author: @HarsH-1877
Created: 10/18/2026
Status: ❌ Closed

Base: main ← Head: fix/codex-per-tool-settings-3862-3863


📝 Commits (10+)

  • 7b30f0d fix(doctor): accept Codex per-tool approval settings in registration status (#3862)
  • 63a779e fix(setup): preserve Codex per-tool approval settings during refresh (#3863)
  • d1101d2 fix(doctor): enforce array tools validation on OpenCode registrations
  • 94c9efb fix(setup): validate recognized Codex tool settings and preserve valid entries in mixed tables
  • b92e37b fix(setup): preserve valid tool fields and retain empty tool tables (#3863)
  • 9b7e618 fix(setup): validate recognized Codex output_token_limit and preserve valid tool fields (#3863)
  • f25822a fix(setup): reject TOML floats for Codex output_token_limit (#3863)
  • d5165ac fix(setup): preserve Codex default_tools_approval_mode and accept writes mode (#3863)
  • d5d3608 fix(setup): eliminate duplicate schema export for Codex approval mode
  • cda49c5 fix(setup): accept sequence-form Codex tool settings and scope default approval mode (#3862, #3863)

📊 Changes

5 files changed (+1178 additions, -20 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.ts (+206 -16)
📝 src/application/ClientRegistrationStatus.ts (+41 -4)
📝 src/application/SetupClientConfiguration.ts (+66 -0)
📝 tests/boundary/filesystem/clientLifecycle.test.ts (+615 -0)
📝 tests/boundary/filesystem/clientRegistrationStatus.test.ts (+250 -0)

📄 Description

Summary

Fixes #3862
Fixes #3863

This PR ensures OpenAI Codex MCP server registrations configured with per-tool approval settings (under [mcp_servers.rea.tools.<name>]) are accepted as valid by rea doctor and preserved during rea setup inspections and refreshes, while strictly maintaining Copilot CLI's string[] tools validation.

Problem and expected behavior

  1. Issue #3862: When a user configures Codex with per-tool approval policies (e.g. [mcp_servers.rea.tools.binary_session]\napproval_mode = "approve"), rea doctor (via readClientRegistrationStatuses) marks the registration as invalid. This occurred because registrationSchema in src/application/ClientRegistrationStatus.ts strictly required tools: z.array(z.string()).optional() (tailored for Copilot CLI), failing on TOML tables which parse as nested objects (Record<string, Record<string, unknown>>).
  2. Issue #3863: When rea setup --client codex inspects or refreshes an existing Codex registration, clientConfigurationDesired in src/application/SetupClientConfiguration.ts preserves startup_timeout_sec and tool_timeout_sec from the previous entry, but drops previous.tools. Consequently, refreshing the registration wipes out caller-selected per-tool approval settings, and inspectClientConfiguration never recognizes a current registration with per-tool settings as already_current.

Expected behavior:

  • rea doctor identifies Codex registrations containing valid per-tool settings tables as aligned (or stale if the launcher/version requires update), while reporting malformed tools values as invalid.
  • copilot_cli continues to enforce string[] validation for tools.
  • rea setup --client codex preserves existing valid caller-selected [mcp_servers.rea.tools.*] tables during refresh and remains idempotent (status: "unchanged" when already current).
  • Malformed Codex tools values (e.g. tools = ["*"] or primitives) are safely replaced with clean registrations during repair setup.

Change and scope

  • Single Owning Schema (ClientConfigurationDocument.ts): Defined and exported codexServerToolsSchema = z.record(z.string(), objectSchema) in src/application/ClientConfigurationDocument.ts so all consumers share the same parsed representation.
  • Registration Status (ClientRegistrationStatus.ts):
    • Updated registrationSchema.tools to z.union([z.array(z.string()), codexServerToolsSchema]).optional().
    • In parseRegistration, enforced that Codex rejects array-shaped tools and non-Codex clients reject non-array tools.
  • Setup Preservation (SetupClientConfiguration.ts):
    • Added codexTools helper utilizing codexServerToolsSchema.safeParse and Object.hasOwn(registration, "tools").
    • Preserved valid previousCodexTools in clientConfigurationDesired for codex registrations.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ClientConfigurationDocument.ts (codexServerToolsSchema), consumed at ClientRegistrationStatus.ts and SetupClientConfiguration.ts.
  • Contribution scope fit and linked maintainer decision: not applicable (bugfix for existing client configuration handling).
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: OpenAI Codex TOML configuration (config.toml).
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): none.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:
    • tests/boundary/filesystem/clientRegistrationStatus.test.ts: Added tests verifying Codex per-tool approval settings report aligned, malformed Codex tools settings report invalid, and non-array Copilot CLI tools report invalid.
    • tests/boundary/filesystem/clientLifecycle.test.ts: Added tests verifying rea setup preserves Codex per-tool approval settings during refresh with idempotency and backup creation, and replaces malformed tools values with clean registrations.
  • Base reproduction or other evidence: On base 29910d16b, Codex registrations with per-tool settings failed with AssertionError: expected 'invalid' to deeply equal 'aligned' and rea setup failed with missing tools in mcp_servers.rea.
  • User-visible CLI/MCP output (if applicable): rea doctor outputs state: "aligned" instead of state: "invalid" for Codex with per-tool approval settings.
  • Remaining proof gaps: none.

Validation performed

  • npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts — passed
  • npm run test:focused -- tests/boundary/filesystem/clientLifecycle.test.ts — passed
  • npm run test:focused -- tests/boundary/filesystem/clientConfigurationTransactions.test.ts — passed
  • npm run check:fast — passed (turbo typecheck & lint, 0 errors)
  • npm run docs:check — passed (6/6 turbo docs:check:built & readmeConsistency, 0 errors)

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: cross-platform filesystem configuration tests on Windows (win32).
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: additive and idempotent configuration updates with .rea.backup creation preserved.

Author readiness

Human owner: @HarsH-1877

  • 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/5115 **Author:** [@HarsH-1877](https://github.com/HarsH-1877) **Created:** 10/18/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/codex-per-tool-settings-3862-3863` --- ### 📝 Commits (10+) - [`7b30f0d`](https://github.com/morluto/rea/commit/7b30f0d325bfc89f5237f1180bc95e5ef456e282) fix(doctor): accept Codex per-tool approval settings in registration status (#3862) - [`63a779e`](https://github.com/morluto/rea/commit/63a779e5836c3603633ca776cbdc9cd181fa96fa) fix(setup): preserve Codex per-tool approval settings during refresh (#3863) - [`d1101d2`](https://github.com/morluto/rea/commit/d1101d2e852a8d1d5d0ffe0cee351b296648302b) fix(doctor): enforce array tools validation on OpenCode registrations - [`94c9efb`](https://github.com/morluto/rea/commit/94c9efb9191080d397445c56801a397fc5773e7a) fix(setup): validate recognized Codex tool settings and preserve valid entries in mixed tables - [`b92e37b`](https://github.com/morluto/rea/commit/b92e37ba4bbb5434a484939e9ae2921fbd88d9bd) fix(setup): preserve valid tool fields and retain empty tool tables (#3863) - [`9b7e618`](https://github.com/morluto/rea/commit/9b7e6187b5334fba1278279f4b0b10a5a3497eb4) fix(setup): validate recognized Codex output_token_limit and preserve valid tool fields (#3863) - [`f25822a`](https://github.com/morluto/rea/commit/f25822aa94039f4738733b73dbaa6014c8a838e0) fix(setup): reject TOML floats for Codex output_token_limit (#3863) - [`d5165ac`](https://github.com/morluto/rea/commit/d5165ac75027a440896b7c1d78e9ce0c486415e7) fix(setup): preserve Codex default_tools_approval_mode and accept writes mode (#3863) - [`d5d3608`](https://github.com/morluto/rea/commit/d5d36080f6c64f7ab262e89bc29f517b55d69b18) fix(setup): eliminate duplicate schema export for Codex approval mode - [`cda49c5`](https://github.com/morluto/rea/commit/cda49c55ee1fcafb93b69d83aef2d3491aafe763) fix(setup): accept sequence-form Codex tool settings and scope default approval mode (#3862, #3863) ### 📊 Changes **5 files changed** (+1178 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.ts` (+206 -16) 📝 `src/application/ClientRegistrationStatus.ts` (+41 -4) 📝 `src/application/SetupClientConfiguration.ts` (+66 -0) 📝 `tests/boundary/filesystem/clientLifecycle.test.ts` (+615 -0) 📝 `tests/boundary/filesystem/clientRegistrationStatus.test.ts` (+250 -0) </details> ### 📄 Description ## Summary Fixes #3862 Fixes #3863 This PR ensures OpenAI Codex MCP server registrations configured with per-tool approval settings (under `[mcp_servers.rea.tools.<name>]`) are accepted as valid by `rea doctor` and preserved during `rea setup` inspections and refreshes, while strictly maintaining Copilot CLI's `string[]` tools validation. ## Problem and expected behavior 1. **Issue #3862**: When a user configures Codex with per-tool approval policies (e.g. `[mcp_servers.rea.tools.binary_session]\napproval_mode = "approve"`), `rea doctor` (via `readClientRegistrationStatuses`) marks the registration as `invalid`. This occurred because `registrationSchema` in `src/application/ClientRegistrationStatus.ts` strictly required `tools: z.array(z.string()).optional()` (tailored for Copilot CLI), failing on TOML tables which parse as nested objects (`Record<string, Record<string, unknown>>`). 2. **Issue #3863**: When `rea setup --client codex` inspects or refreshes an existing Codex registration, `clientConfigurationDesired` in `src/application/SetupClientConfiguration.ts` preserves `startup_timeout_sec` and `tool_timeout_sec` from the previous entry, but drops `previous.tools`. Consequently, refreshing the registration wipes out caller-selected per-tool approval settings, and `inspectClientConfiguration` never recognizes a current registration with per-tool settings as `already_current`. **Expected behavior**: - `rea doctor` identifies Codex registrations containing valid per-tool settings tables as `aligned` (or `stale` if the launcher/version requires update), while reporting malformed tools values as `invalid`. - `copilot_cli` continues to enforce `string[]` validation for `tools`. - `rea setup --client codex` preserves existing valid caller-selected `[mcp_servers.rea.tools.*]` tables during refresh and remains idempotent (`status: "unchanged"` when already current). - Malformed Codex `tools` values (e.g. `tools = ["*"]` or primitives) are safely replaced with clean registrations during repair setup. ## Change and scope - **Single Owning Schema (`ClientConfigurationDocument.ts`)**: Defined and exported `codexServerToolsSchema = z.record(z.string(), objectSchema)` in `src/application/ClientConfigurationDocument.ts` so all consumers share the same parsed representation. - **Registration Status (`ClientRegistrationStatus.ts`)**: - Updated `registrationSchema.tools` to `z.union([z.array(z.string()), codexServerToolsSchema]).optional()`. - In `parseRegistration`, enforced that Codex rejects array-shaped `tools` and non-Codex clients reject non-array `tools`. - **Setup Preservation (`SetupClientConfiguration.ts`)**: - Added `codexTools` helper utilizing `codexServerToolsSchema.safeParse` and `Object.hasOwn(registration, "tools")`. - Preserved valid `previousCodexTools` in `clientConfigurationDesired` for `codex` registrations. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ClientConfigurationDocument.ts` (`codexServerToolsSchema`), consumed at `ClientRegistrationStatus.ts` and `SetupClientConfiguration.ts`. - Contribution scope fit and linked maintainer decision: not applicable (bugfix for existing client configuration handling). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: OpenAI Codex TOML configuration (`config.toml`). - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): none. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [x] Contact network services. - [x] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [x] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/filesystem/clientRegistrationStatus.test.ts`: Added tests verifying Codex per-tool approval settings report `aligned`, malformed Codex tools settings report `invalid`, and non-array Copilot CLI tools report `invalid`. - `tests/boundary/filesystem/clientLifecycle.test.ts`: Added tests verifying `rea setup` preserves Codex per-tool approval settings during refresh with idempotency and backup creation, and replaces malformed tools values with clean registrations. - Base reproduction or other evidence: On base `29910d16b`, Codex registrations with per-tool settings failed with `AssertionError: expected 'invalid' to deeply equal 'aligned'` and `rea setup` failed with missing `tools` in `mcp_servers.rea`. - User-visible CLI/MCP output (if applicable): `rea doctor` outputs `state: "aligned"` instead of `state: "invalid"` for Codex with per-tool approval settings. - Remaining proof gaps: none. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts` — passed - `npm run test:focused -- tests/boundary/filesystem/clientLifecycle.test.ts` — passed - `npm run test:focused -- tests/boundary/filesystem/clientConfigurationTransactions.test.ts` — passed - `npm run check:fast` — passed (turbo typecheck & lint, 0 errors) - `npm run docs:check` — passed (6/6 turbo docs:check:built & readmeConsistency, 0 errors) ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: cross-platform filesystem configuration tests on Windows (`win32`). - Package or release metadata impact: none. - Security, privacy, process, or containment review: additive and idempotent configuration updates with `.rea.backup` creation preserved. ## Author readiness Human owner: @HarsH-1877 - [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:56 +00:00
bsakaguchi changed title from [PR #5115] [CLOSED] fix(setup): accept and preserve Codex per-tool settings (#3862, #3863) to [PR #5115] fix(setup): accept and preserve Codex per-tool settings (#3862, #3863) 2026-11-03 13:34:15 +00:00
bsakaguchi changed title from [PR #5115] fix(setup): accept and preserve Codex per-tool settings (#3862, #3863) to [PR #5115] [CLOSED] fix(setup): accept and preserve Codex per-tool settings (#3862, #3863) 2026-11-03 14:36:01 +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#6771
No description provided.