[PR #6331] [MERGED] fix(doctor): read each client's own activation key #7289

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6331
Author: @cpruijsen
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/doctor-client-activation-keys


📝 Commits (2)

  • a4efb9e fix(doctor): read each client's own activation key
  • 8962909 fix(tests): resolve the Devin config root per platform in activation cases

📊 Changes

2 files changed (+83 additions, -2 deletions)

View changed files

📝 src/application/ClientRegistrationStatus.ts (+9 -1)
📝 tests/boundary/filesystem/clientRegistrationStatus.test.ts (+74 -1)

📄 Description

Summary

rea doctor treated both disabled: true and enabled: false as a disabled REA registration for every client except Grok. The real clients each read only one of these keys. Codex and Kimi Code still load a server marked disabled, and Devin still loads a server marked enabled: false. For those loaded registrations, doctor reported stale, rea doctor --client codex exited 1 with config_drift, and the remediation asked the user to run setup again.

This makes doctor skip the key that each of these three clients ignores. Every other client keeps the existing rule. It is the same kind of per-client activation rule that already exists for Grok and ZCode (#1564).

No issue is linked. I found this by running each client's own mcp list against a registration that REA had written.

Problem and expected behavior

Each line was checked against the installed client, in a temporary HOME, with no login and no model session:

Client REA entry Client's own view Doctor on base
Codex 0.163.0-alpha disabled = true codex mcp list: rea ... enabled stale, config_drift
Codex enabled = false disabled stale (correct)
Devin 3000.11.3 "enabled": false devin mcp list: • rea (active) stale, config_drift
Devin "disabled": true ✗ rea (disabled) stale (correct)
Kimi Code 2.1.1 "disabled": true local API GET /api/v2/mcp/servers: entry without disabled, so active stale
Kimi Code "enabled": false entry carries "enabled": false stale (correct)

devin mcp disable --scope user rea writes disabled: true, and devin mcp enable removes only disabled and leaves an enabled: false key in place, so Devin does not read enabled. Kimi Code's server schema (McpServerCommonFields) has enabled and strips disabled as an unknown key.

Expected: doctor reports aligned when the client loads the server, and stale when the client skips it.

Change and scope

  • src/application/ClientRegistrationStatus.ts: two small sets, ignoresDisabledKey (codex, kimi) and ignoresEnabledKey (devin), checked in registrationActivation next to the existing Grok rule.

Not included:

  • Cursor. What I chose: keep the generic rule for cursor. The alternative was adding cursor to both sets, since cursor-agent mcp list ignores both keys. I did not do that because REA's cursor client also covers the Cursor desktop app, and I did not check how the app reads them; a wrong flip there would report a registration the IDE disables as aligned, which hides real drift instead of noise. A follow-up can add cursor once the app's behavior is verified.
  • Other clients. I did not run them, so they keep the current rule.

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

Contract and boundary impact

  • Semantic owner and earliest changed stage: registration status (registrationActivation), read by doctor and setup inspection. Update maintenance (existingMaintenanceScope) selects the same registrations doctor now reports as aligned, which matches what the client loads.

  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows the Grok and ZCode activation rules).

  • CLI and MCP/tool-catalog contract: for the three cases above, doctor's registration state changes from stale to aligned, and the config_drift failure goes away. No tool catalog changes.

  • Provider, bridge, target-format, or platform compatibility: Codex, Kimi Code and Devin on every platform. The Devin test case follows the client's per-platform config root (%APPDATA%/devin on Windows, $XDG_CONFIG_HOME/devin elsewhere), so the new coverage runs on Windows hosts too.

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

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

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/filesystem/clientRegistrationStatus.test.ts gets a "client activation keys" block. It writes a Codex, Kimi Code and Devin registration with each key and asserts aligned for the key the client ignores and stale for the key it honors. The Kimi entry is the shape REA's own setup writes.
  • Base reproduction: with the src/ change reverted, the three ignored-key cases fail (stale received) and the three honored-key cases pass. With the change, all 31 tests in the file pass.
  • Remaining proof gaps: the client behavior above comes from each client's CLI or local API, not from a full agent session that called an REA tool.

Validation performed

  • npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts: 31 passed. With the src/ change reverted: 3 failed, 28 passed.
  • npx vitest run --testTimeout=180000 on updateMaintenance and setupConfigurationRollback (process-global), mcpDoctor (boundary/mcp), clientLifecycle, kimiClientConfiguration, clientConfigurationDialects, setupConfig (boundary/filesystem), clientProfileLifecycle and mcpDoctorHelpCli (boundary/cli): 9 files, 244 passed.
  • npm run check:fast: passed.
  • npm run verify:test-discovery: passed (1986 files).
  • All checks above ran on the branch rebased onto main at c8ed32853.
  • Client cross-check: all six table rows reproduced against the installed clients in a temporary HOME (codex mcp list, devin mcp list, Kimi loopback GET /api/v2/mcp/servers); doctor exit codes match the table (0 for aligned, 1 for stale).
  • Platform: macOS (arm64), Node 26. Client checks ran on macOS.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. A registration that the client loads is no longer reported as stale.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. Client checks ran on macOS.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none. Doctor reads the same files as before.

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/6331 **Author:** [@cpruijsen](https://github.com/cpruijsen) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/doctor-client-activation-keys` --- ### 📝 Commits (2) - [`a4efb9e`](https://github.com/morluto/rea/commit/a4efb9ec80ead6e117c17d9738ef35be5bcc56a6) fix(doctor): read each client's own activation key - [`8962909`](https://github.com/morluto/rea/commit/89629090eedb573f805b72aba1aacbea65b733da) fix(tests): resolve the Devin config root per platform in activation cases ### 📊 Changes **2 files changed** (+83 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientRegistrationStatus.ts` (+9 -1) 📝 `tests/boundary/filesystem/clientRegistrationStatus.test.ts` (+74 -1) </details> ### 📄 Description ## Summary `rea doctor` treated both `disabled: true` and `enabled: false` as a disabled REA registration for every client except Grok. The real clients each read only one of these keys. Codex and Kimi Code still load a server marked `disabled`, and Devin still loads a server marked `enabled: false`. For those loaded registrations, doctor reported `stale`, `rea doctor --client codex` exited 1 with `config_drift`, and the remediation asked the user to run setup again. This makes doctor skip the key that each of these three clients ignores. Every other client keeps the existing rule. It is the same kind of per-client activation rule that already exists for Grok and ZCode (#1564). No issue is linked. I found this by running each client's own `mcp list` against a registration that REA had written. ## Problem and expected behavior Each line was checked against the installed client, in a temporary HOME, with no login and no model session: | Client | REA entry | Client's own view | Doctor on base | |---|---|---|---| | Codex 0.163.0-alpha | `disabled = true` | `codex mcp list`: `rea ... enabled` | `stale`, `config_drift` | | Codex | `enabled = false` | `disabled` | `stale` (correct) | | Devin 3000.11.3 | `"enabled": false` | `devin mcp list`: `• rea` (active) | `stale`, `config_drift` | | Devin | `"disabled": true` | `✗ rea (disabled)` | `stale` (correct) | | Kimi Code 2.1.1 | `"disabled": true` | local API `GET /api/v2/mcp/servers`: entry without `disabled`, so active | `stale` | | Kimi Code | `"enabled": false` | entry carries `"enabled": false` | `stale` (correct) | `devin mcp disable --scope user rea` writes `disabled: true`, and `devin mcp enable` removes only `disabled` and leaves an `enabled: false` key in place, so Devin does not read `enabled`. Kimi Code's server schema (`McpServerCommonFields`) has `enabled` and strips `disabled` as an unknown key. Expected: doctor reports `aligned` when the client loads the server, and `stale` when the client skips it. ## Change and scope - `src/application/ClientRegistrationStatus.ts`: two small sets, `ignoresDisabledKey` (`codex`, `kimi`) and `ignoresEnabledKey` (`devin`), checked in `registrationActivation` next to the existing Grok rule. Not included: - Cursor. What I chose: keep the generic rule for `cursor`. The alternative was adding cursor to both sets, since `cursor-agent mcp list` ignores both keys. I did not do that because REA's `cursor` client also covers the Cursor desktop app, and I did not check how the app reads them; a wrong flip there would report a registration the IDE disables as aligned, which hides real drift instead of noise. A follow-up can add cursor once the app's behavior is verified. - Other clients. I did not run them, so they keep the current rule. Scope: an ordinary fix inside the existing doctor boundary. A maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: registration status (`registrationActivation`), read by doctor and setup inspection. Update maintenance (`existingMaintenanceScope`) selects the same registrations doctor now reports as aligned, which matches what the client loads. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix, follows the Grok and ZCode activation rules). - CLI and MCP/tool-catalog contract: for the three cases above, doctor's registration `state` changes from `stale` to `aligned`, and the `config_drift` failure goes away. No tool catalog changes. - Provider, bridge, target-format, or platform compatibility: Codex, Kimi Code and Devin on every platform. The Devin test case follows the client's per-platform config root (`%APPDATA%/devin` on Windows, `$XDG_CONFIG_HOME/devin` elsewhere), so the new coverage runs on Windows hosts too. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/filesystem/clientRegistrationStatus.test.ts` gets a "client activation keys" block. It writes a Codex, Kimi Code and Devin registration with each key and asserts `aligned` for the key the client ignores and `stale` for the key it honors. The Kimi entry is the shape REA's own setup writes. - Base reproduction: with the `src/` change reverted, the three ignored-key cases fail (`stale` received) and the three honored-key cases pass. With the change, all 31 tests in the file pass. - Remaining proof gaps: the client behavior above comes from each client's CLI or local API, not from a full agent session that called an REA tool. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts`: 31 passed. With the `src/` change reverted: 3 failed, 28 passed. - `npx vitest run --testTimeout=180000` on `updateMaintenance` and `setupConfigurationRollback` (process-global), `mcpDoctor` (boundary/mcp), `clientLifecycle`, `kimiClientConfiguration`, `clientConfigurationDialects`, `setupConfig` (boundary/filesystem), `clientProfileLifecycle` and `mcpDoctorHelpCli` (boundary/cli): 9 files, 244 passed. - `npm run check:fast`: passed. - `npm run verify:test-discovery`: passed (1986 files). - All checks above ran on the branch rebased onto `main` at c8ed32853. - Client cross-check: all six table rows reproduced against the installed clients in a temporary HOME (`codex mcp list`, `devin mcp list`, Kimi loopback `GET /api/v2/mcp/servers`); doctor exit codes match the table (0 for aligned, 1 for stale). - Platform: macOS (arm64), Node 26. Client checks ran on macOS. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A registration that the client loads is no longer reported as stale. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Client checks ran on macOS. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. Doctor reads the same files as before. ## 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:46:47 +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#7289
No description provided.