[PR #5500] [CLOSED] fix(doctor): retain client registration failure reasons #6940

New issue

Closed

opened 2026-11-01 21:44:13 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:13 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5500
Author: @zerojackhear
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: codex/cli-setup-diagnostics

📝 Commits (5)

fcc6aad fix(doctor): retain invalid registration reasons
b455a65 test(windows): make setup and doctor fixtures portable
4fc2988 test(windows): complete client dialect fixture portability
72c15aa chore: merge upstream main into doctor diagnostics
9ed175f chore: retain upstream code outside diagnostics scope

📊 Changes

7 files changed (+287 additions, -74 deletions)

View changed files

📝 src/application/ClientRegistrationStatus.ts (+49 -18)
📝 src/application/Doctor.test.ts (+16 -3)
📝 src/application/MacHopperPaths.test.ts (+34 -7)
📝 src/application/Setup.planning.test.ts (+21 -3)
➕ tests/acceptance/setup/doctorRegistrationDiagnosticsCli.test.ts (+94 -0)
📝 tests/boundary/filesystem/clientConfigurationDialects.test.ts (+35 -22)
📝 tests/boundary/filesystem/clientRegistrationStatus.test.ts (+38 -21)

📄 Description

Summary

Doctor now retains field-level reasons when a readable client configuration contains an invalid REA command, args, type, or transport, instead of returning only generic setup advice. Preserve the existing invalid state, configuration path, and setup remediation. Make the related setup/doctor fixtures portable on Windows.

Change and scope

Return expected parse failures as Result<..., ConfigurationError> at the shared registration boundary, without exposing configuration values. CLI output shape is unchanged; remediation becomes actionable.
Add five public CLI cases covering malformed command, argument element, OpenCode type, transport, and null registration; verify exit status and unchanged configuration files.
Use host-native paths and Windows package-runner commands in positive fixtures, while preserving raw caller roots and stale/foreign/relative/unknown/malformed coverage.
Existing contribution scope; new-provider/target-execution maintainer approval is not applicable. No new provider, execution authority, installation writes, MCP schemas/catalogs, Evidence semantics, cleanup behavior, or package metadata change. No issue is linked; this was found during local diagnostics and regression review.

Upstream conflict resolution

Integrated upstream main at c374c2fe7290b914d73d9e27af38b498753ec593 into the branch while retaining original commits fcc6aadbf3de, b455a65ab0e7, and 4fc2988aff8b. Resolved the three overlapping files while preserving upstream environment snapshots, bigint TOML timeouts, cleanup failure propagation, launcher identity, OpenCode V2, and Cursor JSONC coverage.

Two additional Windows fixture failures exposed by upstream integration were repaired: direct-launcher tests use a real canonical temporary path and cover both missing/present tool timeouts; a Windows junction preserves dangling-entry detection without requiring file-symlink privileges. Production ownership/identity rules were not weakened.

Merge commit: 72c15aa4dcb6c3949ed10e2c4589b0238918d21a. The normal merge commit hook also auto-fixed two unrelated upstream files; follow-up commit 9ed175f076b5470d96e593cd0b3c20213ebd3522 restores them exactly. Its check-only lint-staged hook ran oxfmt --check and oxlint to prevent reapplying those transformations. The final tree equals the validated pre-hook tree; the final PR diff remains seven files. No history was rewritten.

The current upstream main tip still matches this validated integration base.

Validation performed

Windows x64, Node 24.18.0 / npm 11.16.0; final head 9ed175f076b5470d96e593cd0b3c20213ebd3522.

npm ci --no-audit --no-fund — installed the upstream locked dependencies. The first focused attempt stopped before tests because native controls were absent. Restored only the two ignored native files from the exact rea-agents@6.7.0 tarball per the repository guide; npm run verify:windows-native:artifact passed version/ABI/architecture/SHA-256 checks. No binary is committed.
npm run test:local -- src/application/Doctor.test.ts src/application/Setup.planning.test.ts src/application/MacHopperPaths.test.ts — 77 passed.
The 11 focused files below have final successful per-file results of 270 passed, 9 skipped. The first sweep had 266 passed, 3 failed, 9 skipped. After the two fixture fixes (including one added direct-launcher case), rerunning both affected files produced 37 passed, 3 skipped; the nine unchanged files retain their passing results. Together with source tests, 347 distinct tests passed, with 9 platform skips; these are combined runs, not a single all-green invocation.
npm run check — passed after the final fixture edits: typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards. Lint reported 0 errors / 1905 repository-wide warnings. The rebuilt runtime succeeded with two ineffective-dynamic-import warnings.
git diff --check and staged diff checks passed. Normal merge pre-commit checks and the follow-up check-only pre-commit checks passed. The actual default pre-push npm run check:fast passed (typecheck cache hit; lint executed).

Exact focused selection:

npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/boundary/filesystem/clientConfigurationDialects.test.ts tests/boundary/filesystem/windowsNpxRegistration.test.ts tests/process-global/updateMaintenance.test.ts tests/acceptance/setup/doctorRegistrationDiagnosticsCli.test.ts tests/boundary/filesystem/dshClientConfiguration.test.ts tests/boundary/filesystem/clientConfigurationEnvironment.test.ts tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts

Targeted follow-up command:

npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts

Remaining proof gaps

One first-run malformed-UTF-8 CLI case exceeded its 30-second limit while static checks were also running. The full CLI file then passed alone with unchanged timeout/assertions (that case completed in 14.4 seconds). The timeout's cause is unconfirmed; timing sensitivity remains a limitation.
The 9 skips are upstream POSIX permission and alias/traversal cases on Windows. Windows dangling-entry coverage uses a junction; Windows file symlinks remain unverified on this host because creation returned EPERM.
Windows package-runner coverage includes two real local .cmd shim/MCP transport cases. It does not establish installed-agent acceptance or npm/Bun registry installation. DSH/update fixtures do not establish live Harness/update services.
Real installed clients, Hopper/Ghidra, and real macOS/Linux hosts were not exercised. The artifact integrity check is not the full native-controls/provider verification lane. Full test suite, check:pr, generated-document checks, and package verification were not run. No remote CI success is claimed.

Author readiness

Human owner: @zerojackhear (PR author; declarations pending).

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

Human readiness and any required maintainer review are complete.
Required checks for the final revision and outstanding proof gaps have been assessed.

_{🔄 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/5500 **Author:** [@zerojackhear](https://github.com/zerojackhear) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/cli-setup-diagnostics` --- ### 📝 Commits (5) - [`fcc6aad`](https://github.com/morluto/rea/commit/fcc6aadbf3de9019b95fbaaa70ad964814308948) fix(doctor): retain invalid registration reasons - [`b455a65`](https://github.com/morluto/rea/commit/b455a65ab0e7f35a9f5f843fb0ab3beef5d8195c) test(windows): make setup and doctor fixtures portable - [`4fc2988`](https://github.com/morluto/rea/commit/4fc2988aff8bd727f3692b0d98758c78b53722db) test(windows): complete client dialect fixture portability - [`72c15aa`](https://github.com/morluto/rea/commit/72c15aa4dcb6c3949ed10e2c4589b0238918d21a) chore: merge upstream main into doctor diagnostics - [`9ed175f`](https://github.com/morluto/rea/commit/9ed175f076b5470d96e593cd0b3c20213ebd3522) chore: retain upstream code outside diagnostics scope ### 📊 Changes **7 files changed** (+287 additions, -74 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientRegistrationStatus.ts` (+49 -18) 📝 `src/application/Doctor.test.ts` (+16 -3) 📝 `src/application/MacHopperPaths.test.ts` (+34 -7) 📝 `src/application/Setup.planning.test.ts` (+21 -3) ➕ `tests/acceptance/setup/doctorRegistrationDiagnosticsCli.test.ts` (+94 -0) 📝 `tests/boundary/filesystem/clientConfigurationDialects.test.ts` (+35 -22) 📝 `tests/boundary/filesystem/clientRegistrationStatus.test.ts` (+38 -21) </details> ### 📄 Description ## Summary Doctor now retains field-level reasons when a readable client configuration contains an invalid REA `command`, `args`, `type`, or transport, instead of returning only generic setup advice. Preserve the existing invalid state, configuration path, and setup remediation. Make the related setup/doctor fixtures portable on Windows. ## Change and scope - Return expected parse failures as `Result<..., ConfigurationError>` at the shared registration boundary, without exposing configuration values. CLI output shape is unchanged; remediation becomes actionable. - Add five public CLI cases covering malformed command, argument element, OpenCode type, transport, and null registration; verify exit status and unchanged configuration files. - Use host-native paths and Windows package-runner commands in positive fixtures, while preserving raw caller roots and stale/foreign/relative/unknown/malformed coverage. - Existing contribution scope; new-provider/target-execution maintainer approval is not applicable. No new provider, execution authority, installation writes, MCP schemas/catalogs, Evidence semantics, cleanup behavior, or package metadata change. No issue is linked; this was found during local diagnostics and regression review. ## Upstream conflict resolution Integrated upstream `main` at `c374c2fe7290b914d73d9e27af38b498753ec593` into the branch while retaining original commits `fcc6aadbf3de`, `b455a65ab0e7`, and `4fc2988aff8b`. Resolved the three overlapping files while preserving upstream environment snapshots, bigint TOML timeouts, cleanup failure propagation, launcher identity, OpenCode V2, and Cursor JSONC coverage. Two additional Windows fixture failures exposed by upstream integration were repaired: direct-launcher tests use a real canonical temporary path and cover both missing/present tool timeouts; a Windows junction preserves dangling-entry detection without requiring file-symlink privileges. Production ownership/identity rules were not weakened. Merge commit: `72c15aa4dcb6c3949ed10e2c4589b0238918d21a`. The normal merge commit hook also auto-fixed two unrelated upstream files; follow-up commit `9ed175f076b5470d96e593cd0b3c20213ebd3522` restores them exactly. Its check-only lint-staged hook ran `oxfmt --check` and `oxlint` to prevent reapplying those transformations. The final tree equals the validated pre-hook tree; the final PR diff remains seven files. No history was rewritten. The current upstream `main` tip still matches this validated integration base. ## Validation performed Windows x64, Node 24.18.0 / npm 11.16.0; final head `9ed175f076b5470d96e593cd0b3c20213ebd3522`. - `npm ci --no-audit --no-fund` — installed the upstream locked dependencies. The first focused attempt stopped before tests because native controls were absent. Restored only the two ignored native files from the exact `rea-agents@6.7.0` tarball per the repository guide; `npm run verify:windows-native:artifact` passed version/ABI/architecture/SHA-256 checks. No binary is committed. - `npm run test:local -- src/application/Doctor.test.ts src/application/Setup.planning.test.ts src/application/MacHopperPaths.test.ts` — **77 passed**. - The 11 focused files below have final successful per-file results of **270 passed, 9 skipped**. The first sweep had 266 passed, 3 failed, 9 skipped. After the two fixture fixes (including one added direct-launcher case), rerunning both affected files produced **37 passed, 3 skipped**; the nine unchanged files retain their passing results. Together with source tests, **347 distinct tests passed**, with 9 platform skips; these are combined runs, not a single all-green invocation. - `npm run check` — passed after the final fixture edits: typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards. Lint reported 0 errors / 1905 repository-wide warnings. The rebuilt runtime succeeded with two ineffective-dynamic-import warnings. - `git diff --check` and staged diff checks passed. Normal merge pre-commit checks and the follow-up check-only pre-commit checks passed. The actual default pre-push `npm run check:fast` passed (typecheck cache hit; lint executed). Exact focused selection: ```text npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/boundary/filesystem/clientConfigurationDialects.test.ts tests/boundary/filesystem/windowsNpxRegistration.test.ts tests/process-global/updateMaintenance.test.ts tests/acceptance/setup/doctorRegistrationDiagnosticsCli.test.ts tests/boundary/filesystem/dshClientConfiguration.test.ts tests/boundary/filesystem/clientConfigurationEnvironment.test.ts tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts ``` Targeted follow-up command: ```text npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts ``` ## Remaining proof gaps - One first-run malformed-UTF-8 CLI case exceeded its 30-second limit while static checks were also running. The full CLI file then passed alone with unchanged timeout/assertions (that case completed in 14.4 seconds). The timeout's cause is unconfirmed; timing sensitivity remains a limitation. - The 9 skips are upstream POSIX permission and alias/traversal cases on Windows. Windows dangling-entry coverage uses a junction; Windows file symlinks remain unverified on this host because creation returned EPERM. - Windows package-runner coverage includes two real local `.cmd` shim/MCP transport cases. It does not establish installed-agent acceptance or npm/Bun registry installation. DSH/update fixtures do not establish live Harness/update services. - Real installed clients, Hopper/Ghidra, and real macOS/Linux hosts were not exercised. The artifact integrity check is not the full native-controls/provider verification lane. Full test suite, `check:pr`, generated-document checks, and package verification were not run. No remote CI success is claimed. ## Author readiness Human owner: @zerojackhear (PR author; declarations pending). - [ ] 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 - [ ] Human readiness and any required maintainer review are complete. - [ ] Required checks for the final revision and outstanding proof gaps have been assessed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:44:13 +00:00