[PR #6910] [CLOSED] fix(cli): enumerate setup and doctor client choices #7441

New issue

Closed

opened 2026-11-01 21:47:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6910
Author: @Bilalmesk
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/setup-client-schema-enum

📝 Commits (1)

e58cae9 fix(cli): enumerate setup and doctor client choices

📊 Changes

2 files changed (+79 additions, -7 deletions)

View changed files

📝 src/cli/setupCommands.ts (+6 -7)
📝 tests/boundary/cli/cliInputSchemaRequired.test.ts (+73 -0)

📄 Description

Summary

Fixes #6838.

rea setup --schema and rea doctor --schema advertised their client array items as arbitrary strings, and setup advertised skillClient as shared or any string, even though the parser accepts only IDs from SUPPORTED_CLIENT_DEFINITIONS. Schema discovery therefore lost the finite choices and admitted requests that parsing rejects.

The three selectors now advertise enums derived from the same client definitions the parser uses. setup.skillClient adds shared. Unsupported IDs remain rejected with the existing Unsupported agent integration error.

Problem and expected behavior

Before this change, all three advertised schemas used items: { "type": "string" } with no enum. An agent could choose an unsupported ID from the schema and then fail during parsing. doctor.provider already demonstrates the expected representation by advertising its five accepted provider IDs as an enum.

Expected: machine-readable schema discovery exposes the parser's finite client choices, while unsupported client and skill-client IDs stay validation errors.

Change and scope

Replaced the string refinement and literal union with z.enum(...) values derived from SUPPORTED_CLIENT_DEFINITIONS. Adding or removing a client definition now updates all three advertised schemas automatically.

Added boundary coverage that:

compares each advertised enum with the canonical client definitions;
checks the production parsers and compiled JSON Schemas accept every supported client ID;
checks both reject unknown-client;
checks setup.skillClient accepts shared plus every supported client ID.

No setup or doctor action, client configuration, provider behavior, target handling, or MCP/tool-catalog contract changes.

Contract and boundary impact

Semantic owner and earliest changed stage: src/cli/setupCommands.ts; CLI input-schema discovery and request parsing.
Contribution scope fit and linked maintainer decision: Ordinary P2 CLI bug fix within existing scope; no new provider or capability, so a separate maintainer scope decision is not applicable.
CLI and MCP/tool-catalog contract: setup.client, setup.skillClient, and doctor.client item schemas now expose enums. No MCP or tool-catalog change.
Provider, bridge, target-format, or platform compatibility: None.
Evidence, artifact, provenance, or reconstruction contract: None.
Target selection, authority, upstream behavior, and cleanup details: Not applicable; the change only inspects and validates CLI schemas.
Generated metadata, package, or installation impact: None.

Expected effects: no target, process, network, file, configuration, or protection interaction.

Evidence and regression coverage

Tests added or updated: tests/boundary/cli/cliInputSchemaRequired.test.ts adds a setup/doctor client-schema regression test covering the advertised enums, parser agreement for all supported IDs, shared for skillClient, and rejection of unknown IDs.
Base reproduction or other evidence: The base source used a string refinement without an enum; the linked issue's executed probe observed items: { "type": "string" } and a schema-valid unknown-client rejected by the parser. I did not separately rerun the base revision. After the change, a production schema probe reported enum[25] for setup.client and doctor.client, enum[26] for setup.skillClient, and still rejected unknown-client with Unsupported agent integration.
User-visible CLI/MCP output:

setup.client.items:       {"type":"string","enum":["claude_code",...,"grok_bot"]}   (25 values)
setup.skillClient.items:  {"type":"string","enum":["shared","claude_code",...,"grok_bot"]} (26 values)
doctor.client.items:      {"type":"string","enum":["claude_code",...,"grok_bot"]}   (25 values)
unknown-client:           Unsupported agent integration

Remaining proof gaps: No setup or doctor action was executed, so no client configuration or provider path was exercised. npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts could not start on this Windows checkout because native/windows/build/manifest.json is absent; the exact test file was run directly with Vitest instead. Full npm run typecheck did not complete: the native compiler exceeded 4 GB and stopped making progress after about ten minutes on this 8 GB host, so I stopped it. A targeted typecheck of the changed source passed; including the changed test in a targeted run surfaced an unrelated Frida type error in unchanged src/frida/FridaInstrumentationManager.ts. CI's Static checks and Build package jobs pass; its Source tests job currently fails in unrelated tests in javascriptStaticAnalysis.mapEndpoints.test.ts, javascriptStaticAnalysisReturnedInvocation.test.ts, BundleResourcesMcp.test.ts, and FridaInstrumentationService.test.ts, none of which this PR changes. A local run of the first three files reproduced those failures as well.

Validation performed

npx vitest run tests/boundary/cli/cliInputSchemaRequired.test.ts --reporter=verbose — 9 tests passed.
node scripts/run-typescript.mjs --noEmit -p <targeted config for src/cli/setupCommands.ts> — passed.
npx oxfmt --check src/cli/setupCommands.ts tests/boundary/cli/cliInputSchemaRequired.test.ts — passed.
npx oxlint src/cli/setupCommands.ts tests/boundary/cli/cliInputSchemaRequired.test.ts — passed.
npm run verify:module-boundaries — passed; checked 2,802 source files.
npm run verify:test-discovery — passed; discovered all 2,178 test files exactly once.

Compatibility, safety, and release

Breaking changes or migration steps: None. The schema now matches the parser's existing accepted values; previously valid supported IDs remain valid, and unsupported IDs are rejected earlier.
Real Hopper/Ghidra, browser, or OS coverage: None; not exercised by this schema-only change.
Package or release metadata impact: None.
Security, privacy, process, or containment review: No target interaction, credential handling, persistence, or configuration writes.

Author readiness

Human owner: @Bilalmesk

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/6910 **Author:** [@Bilalmesk](https://github.com/Bilalmesk) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/setup-client-schema-enum` --- ### 📝 Commits (1) - [`e58cae9`](https://github.com/morluto/rea/commit/e58cae90bfccb7a321431fa60cffabcb2b03b341) fix(cli): enumerate setup and doctor client choices ### 📊 Changes **2 files changed** (+79 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/setupCommands.ts` (+6 -7) 📝 `tests/boundary/cli/cliInputSchemaRequired.test.ts` (+73 -0) </details> ### 📄 Description ## Summary Fixes #6838. `rea setup --schema` and `rea doctor --schema` advertised their `client` array items as arbitrary strings, and `setup` advertised `skillClient` as `shared` or any string, even though the parser accepts only IDs from `SUPPORTED_CLIENT_DEFINITIONS`. Schema discovery therefore lost the finite choices and admitted requests that parsing rejects. The three selectors now advertise enums derived from the same client definitions the parser uses. `setup.skillClient` adds `shared`. Unsupported IDs remain rejected with the existing `Unsupported agent integration` error. ## Problem and expected behavior Before this change, all three advertised schemas used `items: { "type": "string" }` with no `enum`. An agent could choose an unsupported ID from the schema and then fail during parsing. `doctor.provider` already demonstrates the expected representation by advertising its five accepted provider IDs as an enum. Expected: machine-readable schema discovery exposes the parser's finite client choices, while unsupported client and skill-client IDs stay validation errors. ## Change and scope Replaced the string refinement and literal union with `z.enum(...)` values derived from `SUPPORTED_CLIENT_DEFINITIONS`. Adding or removing a client definition now updates all three advertised schemas automatically. Added boundary coverage that: - compares each advertised enum with the canonical client definitions; - checks the production parsers and compiled JSON Schemas accept every supported client ID; - checks both reject `unknown-client`; - checks `setup.skillClient` accepts `shared` plus every supported client ID. No setup or doctor action, client configuration, provider behavior, target handling, or MCP/tool-catalog contract changes. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/cli/setupCommands.ts`; CLI input-schema discovery and request parsing. - Contribution scope fit and linked maintainer decision: Ordinary P2 CLI bug fix within existing scope; no new provider or capability, so a separate maintainer scope decision is not applicable. - CLI and MCP/tool-catalog contract: `setup.client`, `setup.skillClient`, and `doctor.client` item schemas now expose enums. No MCP or tool-catalog change. - Provider, bridge, target-format, or platform compatibility: None. - Evidence, artifact, provenance, or reconstruction contract: None. - Target selection, authority, upstream behavior, and cleanup details: Not applicable; the change only inspects and validates CLI schemas. - Generated metadata, package, or installation impact: None. Expected effects: no target, process, network, file, configuration, or protection interaction. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/cliInputSchemaRequired.test.ts` adds a setup/doctor client-schema regression test covering the advertised enums, parser agreement for all supported IDs, `shared` for `skillClient`, and rejection of unknown IDs. - Base reproduction or other evidence: The base source used a string refinement without an enum; the linked issue's executed probe observed `items: { "type": "string" }` and a schema-valid `unknown-client` rejected by the parser. I did not separately rerun the base revision. After the change, a production schema probe reported `enum[25]` for `setup.client` and `doctor.client`, `enum[26]` for `setup.skillClient`, and still rejected `unknown-client` with `Unsupported agent integration`. - User-visible CLI/MCP output: ```text setup.client.items: {"type":"string","enum":["claude_code",...,"grok_bot"]} (25 values) setup.skillClient.items: {"type":"string","enum":["shared","claude_code",...,"grok_bot"]} (26 values) doctor.client.items: {"type":"string","enum":["claude_code",...,"grok_bot"]} (25 values) unknown-client: Unsupported agent integration ``` - Remaining proof gaps: No setup or doctor action was executed, so no client configuration or provider path was exercised. `npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts` could not start on this Windows checkout because `native/windows/build/manifest.json` is absent; the exact test file was run directly with Vitest instead. Full `npm run typecheck` did not complete: the native compiler exceeded 4 GB and stopped making progress after about ten minutes on this 8 GB host, so I stopped it. A targeted typecheck of the changed source passed; including the changed test in a targeted run surfaced an unrelated Frida type error in unchanged `src/frida/FridaInstrumentationManager.ts`. CI's Static checks and Build package jobs pass; its Source tests job currently fails in unrelated tests in `javascriptStaticAnalysis.mapEndpoints.test.ts`, `javascriptStaticAnalysisReturnedInvocation.test.ts`, `BundleResourcesMcp.test.ts`, and `FridaInstrumentationService.test.ts`, none of which this PR changes. A local run of the first three files reproduced those failures as well. ## Validation performed - `npx vitest run tests/boundary/cli/cliInputSchemaRequired.test.ts --reporter=verbose` — 9 tests passed. - `node scripts/run-typescript.mjs --noEmit -p <targeted config for src/cli/setupCommands.ts>` — passed. - `npx oxfmt --check src/cli/setupCommands.ts tests/boundary/cli/cliInputSchemaRequired.test.ts` — passed. - `npx oxlint src/cli/setupCommands.ts tests/boundary/cli/cliInputSchemaRequired.test.ts` — passed. - `npm run verify:module-boundaries` — passed; checked 2,802 source files. - `npm run verify:test-discovery` — passed; discovered all 2,178 test files exactly once. ## Compatibility, safety, and release - Breaking changes or migration steps: None. The schema now matches the parser's existing accepted values; previously valid supported IDs remain valid, and unsupported IDs are rejected earlier. - Real Hopper/Ghidra, browser, or OS coverage: None; not exercised by this schema-only change. - Package or release metadata impact: None. - Security, privacy, process, or containment review: No target interaction, credential handling, persistence, or configuration writes. ## Author readiness Human owner: @Bilalmesk - [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 - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:53 +00:00