[PR #2769] [CLOSED] fix(setup): validate registration overrides with runtime semantics #5378

New issue

Closed

opened 2026-10-31 01:06:10 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:06:10 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2769
Author: @To3akaRin
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: codex/registration-runtime-validation

📝 Commits (10+)

c1074f6 fix(setup): validate registration overrides with runtime semantics
00ee6e0 fix(setup): validate dependencies after merging client layers
4acf361 fix: reconcile latest main for PR #2769
3d19ef8 fix(setup): merge captured main and preserve monotonic startup deadlines
8695a41 fix(comparison): recheck historical calls-only changes (#2223)
4576282 fix(android): make DEX descriptor pattern portable
620db8a fix(ci): isolate Linux Hopper process fixture inventory
87872b5 chore: fix CI formatting on setup change
d052923 merge: refresh PR #2769 with main
399345d Merge branch 'main' into codex/registration-runtime-validation

📄 Description

Summary

Setup and doctor could report a registration as current even when its explicit environment prevented MCP startup. They now use the same pure environment parsing rules as startup, report the offending setting, and refuse setup writes until the caller repairs it. Valid settings split across OpenCode configuration files remain valid after merging.

Fixes #2665.

Problem and expected behavior

For example, a Cursor entry with HOPPER_LOADER_ARGS_JSON: "{" was accepted as aligned although rea mcp immediately exited with a configuration error. An unpaired REA_GHIDRA_COMPILER_SPEC_ID had the same mismatch. These failures must be diagnosed before creating a backup or changing the registration. OpenCode may put the language identifier in opencode.json and the compiler specification in opencode.jsonc; that pair must be checked in their merged environment, while malformed values in either source must still be reported.

Change and scope

Use two pure parsing stages: parseEnvironmentValues parses individual settings and decoded arrays; parseEnvironment also validates dependencies in a complete environment. Startup consumes the parsed collections instead of decoding them again. Neither stage probes the host.
Validate every client file as a fragment before merging, then validate the effective environment. A valid higher-priority override cannot hide a malformed map, value or JSON array in a lower-priority source.
Retain each parsed layer's source path. Setup, alignment checks and desired-registration construction replace the reread target at its original precedence position before validation, including a missing or non-final write target. Persist only that target's explicit overrides; inherited settings are not copied into a higher-priority file.
Existing client registration readers validate through that owner while retaining the original explicit strings, unknown caller keys and absence of defaults.
DSH carries expression-backed setting identities separately from literal values. A language expression is unknown, not a proven missing companion; a literal override receives normal validation. Expressions remain preserved and are never evaluated by REA's registration reader.
Document the distinction between registration alignment and validation of eventual expression values.

Contract and boundary impact

Semantic owners: pure environment value/dependency parsing and the existing effective client-configuration merger, before projection or persistence.
Scope decision: not applicable; correctness fix within existing setup/diagnostic boundaries, with no provider or execution-authority expansion.
CLI/MCP contracts: existing statuses and configuration errors; setup reports needs_human with empty planned/applied actions, doctor reports the registration as invalid, and startup retains its existing error. Tool schemas and Evidence are unchanged.
Provider/platform compatibility: runtime semantics stay the same. Previously accepted but startup-invalid registration literals are rejected earlier.
Installation: valid strings, unrelated entries, backups and idempotence remain intact. No new setting, dependency, release metadata or committed generated output.
Existing workflow effects: read client configuration; approved setup writes configuration and backups. Verification also launches isolated CLI/MCP processes and uses package-install network access. This change adds no target execution, injection, protection recovery or network behavior.

Evidence and regression coverage

On base f0f09950f34d96e3b1c62fd04821af5e301f80c7, the extended registration boundary tests produced 10 failures / 92 passes before the production change. The failures covered malformed loader JSON, wrong JSON shape, NUL arguments, malformed/empty exclusion patterns, the missing language companion, DSH literal settings and an invalid literal replacing a DSH expression.

Real compiled CLI tests compare MCP startup with setup and doctor for Cursor and DSH, assert unchanged bytes and no backup on refusal, then repair the fixture and verify raw-value preservation, backup readback and repeatable setup. Filesystem boundary controls cover Cursor, Codex, OpenCode and Hermes, including exact environment equality so defaults cannot leak into a registration. DSH controls preserve tagged expressions and distinguish known companions from unresolved ones. Eight additional layered filesystem cases verify split settings, a genuinely missing companion, precedence, unchanged lower-layer bytes, backups, invalid lower-priority fields, missing targets and non-final write targets. A real OpenCode CLI journey verifies setup/doctor accept the valid split pair without writes and reject the same configuration after its required language is removed.

Validation performed

Executed on macOS arm64 with Node 24.19.0 / npm 11.17.0 and locked dependencies. The repository pins 24.18.0 / 11.16.0; the exact pinned pair was not used for these runs.

npm run test:focused -- src/config.test.ts tests/boundary/filesystem/clientConfigurationEnvironment.test.ts tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts tests/boundary/filesystem/clientConfigurationDialects.test.ts tests/boundary/filesystem/dshClientConfiguration.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts tests/acceptance/setup/dshClientSetup.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts — 197 passed / 8 files. Includes the existing DSH setup, actual SDK MCP connection, repeat and uninstall journey.
npm run check — passed; typecheck, format, lint, knip, metadata and architecture guards. Lint reported 0 errors and 1,051 repository warnings.
npm run docs:check — passed, including 2 README consistency tests.
npm run verify:test-discovery — all 1,057 test files discovered exactly once.
npm run jscpd — passed, 2.98% duplicated lines.
npm run verify:package — passed: isolated install/update, CLI/MCP, setup/uninstall, backups, failure recovery and cleanup; 191 tools / 6 prompts, verified lineage with no descendants.
npm pack --dry-run --json — passed, 1,549 package entries.
git diff --check — passed.

Compatibility, safety, and release

No migration for valid configurations. Invalid literal overrides need the named setting repaired before setup can continue. Error settings and startup diagnostics retain the existing contract.

Remaining coverage limits: no live Cursor/Codex/Hermes application or native Hopper/Ghidra execution, no Windows/Linux run, and no evaluation of DSH expressions in a real Harness process. The package verifier's controlled bridges do not establish real-provider behavior. All registration mutations used test-owned temporary homes; no user registration was changed.

Before merge

Concrete issue, focused outcome and conventional title.
Scope and authority are resolved for this existing-boundary correction.
Regression tests cover changed behavior, preservation and failure paths.
Owning installation documentation is updated; no public tool schema changes.
Public CLI behavior and actual verification scope are stated.
Final diff checked for unrelated cleanup, secrets 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/2769 **Author:** [@To3akaRin](https://github.com/To3akaRin) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/registration-runtime-validation` --- ### 📝 Commits (10+) - [`c1074f6`](https://github.com/morluto/rea/commit/c1074f64c9945692a4c38669b986c77291a6536c) fix(setup): validate registration overrides with runtime semantics - [`00ee6e0`](https://github.com/morluto/rea/commit/00ee6e00c030f86679b058fc3755e4acfde0cb99) fix(setup): validate dependencies after merging client layers - [`4acf361`](https://github.com/morluto/rea/commit/4acf3613a3ce97996a6a0a19c3d7689cb9d0398b) fix: reconcile latest main for PR #2769 - [`3d19ef8`](https://github.com/morluto/rea/commit/3d19ef8c4de9648c571aa9c0dde64d64ec20b5f7) fix(setup): merge captured main and preserve monotonic startup deadlines - [`8695a41`](https://github.com/morluto/rea/commit/8695a4153308c19572dd85ba91d9969b76430986) fix(comparison): recheck historical calls-only changes (#2223) - [`4576282`](https://github.com/morluto/rea/commit/4576282883963477b0d83fc73560c7ffc2872aef) fix(android): make DEX descriptor pattern portable - [`620db8a`](https://github.com/morluto/rea/commit/620db8a24f8785f9701f08346b07fb83cdd19f52) fix(ci): isolate Linux Hopper process fixture inventory - [`87872b5`](https://github.com/morluto/rea/commit/87872b5a8fcd41421d43f50ead2d4c46b6baefc7) chore: fix CI formatting on setup change - [`d052923`](https://github.com/morluto/rea/commit/d05292363e2ad29b3640606463cca54672e702d6) merge: refresh PR #2769 with main - [`399345d`](https://github.com/morluto/rea/commit/399345d91e9e85a1d54bd10f4092e0e08f352c12) Merge branch 'main' into codex/registration-runtime-validation ### 📄 Description ## Summary Setup and doctor could report a registration as current even when its explicit environment prevented MCP startup. They now use the same pure environment parsing rules as startup, report the offending setting, and refuse setup writes until the caller repairs it. Valid settings split across OpenCode configuration files remain valid after merging. Fixes #2665. ## Problem and expected behavior For example, a Cursor entry with `HOPPER_LOADER_ARGS_JSON: "{"` was accepted as aligned although `rea mcp` immediately exited with a configuration error. An unpaired `REA_GHIDRA_COMPILER_SPEC_ID` had the same mismatch. These failures must be diagnosed before creating a backup or changing the registration. OpenCode may put the language identifier in `opencode.json` and the compiler specification in `opencode.jsonc`; that pair must be checked in their merged environment, while malformed values in either source must still be reported. ## Change and scope - Use two pure parsing stages: `parseEnvironmentValues` parses individual settings and decoded arrays; `parseEnvironment` also validates dependencies in a complete environment. Startup consumes the parsed collections instead of decoding them again. Neither stage probes the host. - Validate every client file as a fragment before merging, then validate the effective environment. A valid higher-priority override cannot hide a malformed map, value or JSON array in a lower-priority source. - Retain each parsed layer's source path. Setup, alignment checks and desired-registration construction replace the reread target at its original precedence position before validation, including a missing or non-final write target. Persist only that target's explicit overrides; inherited settings are not copied into a higher-priority file. - Existing client registration readers validate through that owner while retaining the original explicit strings, unknown caller keys and absence of defaults. - DSH carries expression-backed setting identities separately from literal values. A language expression is unknown, not a proven missing companion; a literal override receives normal validation. Expressions remain preserved and are never evaluated by REA's registration reader. - Document the distinction between registration alignment and validation of eventual expression values. ## Contract and boundary impact - Semantic owners: pure environment value/dependency parsing and the existing effective client-configuration merger, before projection or persistence. - Scope decision: not applicable; correctness fix within existing setup/diagnostic boundaries, with no provider or execution-authority expansion. - CLI/MCP contracts: existing statuses and configuration errors; setup reports `needs_human` with empty planned/applied actions, doctor reports the registration as `invalid`, and startup retains its existing error. Tool schemas and Evidence are unchanged. - Provider/platform compatibility: runtime semantics stay the same. Previously accepted but startup-invalid registration literals are rejected earlier. - Installation: valid strings, unrelated entries, backups and idempotence remain intact. No new setting, dependency, release metadata or committed generated output. - Existing workflow effects: read client configuration; approved setup writes configuration and backups. Verification also launches isolated CLI/MCP processes and uses package-install network access. This change adds no target execution, injection, protection recovery or network behavior. ## Evidence and regression coverage On base `f0f09950f34d96e3b1c62fd04821af5e301f80c7`, the extended registration boundary tests produced **10 failures / 92 passes** before the production change. The failures covered malformed loader JSON, wrong JSON shape, NUL arguments, malformed/empty exclusion patterns, the missing language companion, DSH literal settings and an invalid literal replacing a DSH expression. Real compiled CLI tests compare MCP startup with setup and doctor for Cursor and DSH, assert unchanged bytes and no backup on refusal, then repair the fixture and verify raw-value preservation, backup readback and repeatable setup. Filesystem boundary controls cover Cursor, Codex, OpenCode and Hermes, including exact environment equality so defaults cannot leak into a registration. DSH controls preserve tagged expressions and distinguish known companions from unresolved ones. Eight additional layered filesystem cases verify split settings, a genuinely missing companion, precedence, unchanged lower-layer bytes, backups, invalid lower-priority fields, missing targets and non-final write targets. A real OpenCode CLI journey verifies setup/doctor accept the valid split pair without writes and reject the same configuration after its required language is removed. ## Validation performed Executed on macOS arm64 with Node **24.19.0** / npm **11.17.0** and locked dependencies. The repository pins 24.18.0 / 11.16.0; the exact pinned pair was not used for these runs. - `npm run test:focused -- src/config.test.ts tests/boundary/filesystem/clientConfigurationEnvironment.test.ts tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts tests/boundary/filesystem/clientConfigurationDialects.test.ts tests/boundary/filesystem/dshClientConfiguration.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts tests/acceptance/setup/dshClientSetup.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts` — **197 passed / 8 files**. Includes the existing DSH setup, actual SDK MCP connection, repeat and uninstall journey. - `npm run check` — passed; typecheck, format, lint, knip, metadata and architecture guards. Lint reported 0 errors and 1,051 repository warnings. - `npm run docs:check` — passed, including 2 README consistency tests. - `npm run verify:test-discovery` — all **1,057** test files discovered exactly once. - `npm run jscpd` — passed, **2.98%** duplicated lines. - `npm run verify:package` — passed: isolated install/update, CLI/MCP, setup/uninstall, backups, failure recovery and cleanup; 191 tools / 6 prompts, verified lineage with no descendants. - `npm pack --dry-run --json` — passed, 1,549 package entries. - `git diff --check` — passed. ## Compatibility, safety, and release No migration for valid configurations. Invalid literal overrides need the named setting repaired before setup can continue. Error settings and startup diagnostics retain the existing contract. Remaining coverage limits: no live Cursor/Codex/Hermes application or native Hopper/Ghidra execution, no Windows/Linux run, and no evaluation of DSH expressions in a real Harness process. The package verifier's controlled bridges do not establish real-provider behavior. All registration mutations used test-owned temporary homes; no user registration was changed. ## Before merge - [x] Concrete issue, focused outcome and conventional title. - [x] Scope and authority are resolved for this existing-boundary correction. - [x] Regression tests cover changed behavior, preservation and failure paths. - [x] Owning installation documentation is updated; no public tool schema changes. - [x] Public CLI behavior and actual verification scope are stated. - [x] Final diff checked for unrelated cleanup, secrets and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:06:10 +00:00