[PR #2693] fix(cli): preserve mcp doctor output choices on argument errors #5316

New issue

Open

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

bsakaguchi commented

2026-10-31 01:05:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2693
Author: @jasonchen505
Created: 10/13/2026
Status: 🔄 Open

Base: main ← Head: contrib/mcp-doctor-full-output

📝 Commits (1)

9816af2 fix(cli): preserve doctor output choices on argument errors

📊 Changes

2 files changed (+228 additions, -40 deletions)

View changed files

📝 src/mcpDoctor.ts (+16 -40)
➕ tests/process-global/mcpDoctorArguments.test.ts (+212 -0)

📄 Description

Summary

The normal MCP doctor full-output path is already on main. This PR now only fixes output selection when argument validation fails, as requested in the maintainer review.

For example, rea mcp doctor --bogus --full-output --json should return a JSON error envelope and exit 1 without starting MCP. Previously, parsing stopped at the first error and dropped the later output options.

Change and scope

The parser keeps the first error while continuing to collect valid output and help options. Tokens consumed by --format remain values, even when they look like flags. The last valid format wins.

The diff is limited to the parser/error path and 35 regression cases. Tests cover option ordering and assert that neither MCP connect nor transport start is called. Normal diagnostic execution is unchanged.

Scope: an argument-parsing fix within the existing CLI. No new provider, target interaction, package change, or migration is introduced; a separate scope decision is not applicable. MCP/tool-catalog, evidence, authority, and cleanup contracts are unchanged.

Validation

On the unmodified base, 25 of the initial 33 regression cases failed; all passed after the fix. Two additional TOON/Markdown cases also pass.

Executed after the final rebase with Node 24.18.0 / npm 11.16.0:

npm run test:focused -- tests/process-global/mcpDoctorArguments.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/cli/mcpDoctorHelpCli.test.ts — 51 passed across 3 files.
npm run check:fast — passed.
npm run check — passed (typecheck, lint, formatting, knip, metadata, architecture guards).
npm run verify:test-discovery — all 1,328 test files discovered exactly once.
git diff --check — passed.
Built CLI smoke checks — unknown options before selectors, missing format, and consumed full-output tokens produced the expected JSON and exit 1, with empty stderr.
GitHub CI — awaiting maintainer approval.

The full suite and real Hopper/Ghidra/browser lanes were not run. Existing lint/build warnings remain.

Author readiness

Human owner: @jasonchen505

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.

_{🔄 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/2693 **Author:** [@jasonchen505](https://github.com/jasonchen505) **Created:** 10/13/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `contrib/mcp-doctor-full-output` --- ### 📝 Commits (1) - [`9816af2`](https://github.com/morluto/rea/commit/9816af29dd9784234b70fbec6bb78bf41b4c2126) fix(cli): preserve doctor output choices on argument errors ### 📊 Changes **2 files changed** (+228 additions, -40 deletions) <details> <summary>View changed files</summary> 📝 `src/mcpDoctor.ts` (+16 -40) ➕ `tests/process-global/mcpDoctorArguments.test.ts` (+212 -0) </details> ### 📄 Description ## Summary The normal MCP doctor full-output path is already on main. This PR now only fixes output selection when argument validation fails, as requested in the [maintainer review](https://github.com/morluto/rea/pull/2693#issuecomment-6186270751). For example, `rea mcp doctor --bogus --full-output --json` should return a JSON error envelope and exit 1 without starting MCP. Previously, parsing stopped at the first error and dropped the later output options. ## Change and scope The parser keeps the first error while continuing to collect valid output and help options. Tokens consumed by `--format` remain values, even when they look like flags. The last valid format wins. The diff is limited to the parser/error path and 35 regression cases. Tests cover option ordering and assert that neither MCP connect nor transport start is called. Normal diagnostic execution is unchanged. Scope: an argument-parsing fix within the existing CLI. No new provider, target interaction, package change, or migration is introduced; a separate scope decision is not applicable. MCP/tool-catalog, evidence, authority, and cleanup contracts are unchanged. ## Validation On the unmodified base, 25 of the initial 33 regression cases failed; all passed after the fix. Two additional TOON/Markdown cases also pass. Executed after the final rebase with Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- tests/process-global/mcpDoctorArguments.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/cli/mcpDoctorHelpCli.test.ts` — 51 passed across 3 files. - `npm run check:fast` — passed. - `npm run check` — passed (typecheck, lint, formatting, knip, metadata, architecture guards). - `npm run verify:test-discovery` — all 1,328 test files discovered exactly once. - `git diff --check` — passed. - Built CLI smoke checks — unknown options before selectors, missing format, and consumed full-output tokens produced the expected JSON and exit 1, with empty stderr. - GitHub CI — [awaiting maintainer approval](https://github.com/morluto/rea/actions/runs/38709332283). The full suite and real Hopper/Ghidra/browser lanes were not run. Existing lint/build warnings remain. ## Author readiness Human owner: @jasonchen505 - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:05:42 +00:00

bsakaguchi referenced this issue

2026-11-01 21:42:16 +00:00

[PR #4941] [MERGED] fix(javascript): preserve runtime path bytes and Electron ownership #6683

bsakaguchi referenced this issue

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

[PR #5316] [MERGED] fix(ci): validate unindexed readiness heads with live PR metadata #6875