Skip to content

types(mcp): the option-plumbing sweep left seven raw reads, and the CLI restates two route enums by hand #9824

Description

@JSONbored

Context

#9773 (merged as #9785) typed parseOptions from CLI_FLAG_SPEC and introduced optionText(), which treats a flag given with no value as absent. The review of that PR caught one site the sweep missed (explainReviewRiskCli's contributorLogin). That was the visible end of a family — seven more sites read an option without narrowing it, each reachable by an ordinary typo:

Invocation What happens today
analyze-branch --login boolean true is sent to /v1/local/branch-analysis as the contributor login
review-pr --login same
login --github-token true is truthy, so the token branch is taken with true as the token instead of falling through to the device flow
maintain selftune-audit --limit Number(true) is 1 — the audit is silently capped at one row instead of using the server default
maintain precision --window-days same, a one-day window
maintain audit-feed --since String(true) sends ?since=true, which the route reads as a cursor
validate-config --file !options.file passes (true is truthy) and the manifest is read from the path ""

Two other things in the same code

  1. The chains are copied. optionText(options.login) ?? activeProfile.session?.login ?? process.env.LOOPOVER_LOGIN ?? process.env.GITHUB_LOGIN appears at 14 call sites, readCliTextFile(optionText(options.bodyFile) ?? "", "Body") at three, and the --issue array/scalar/absent dance at three more. Each copy is its own chance to miss a step — which is exactly how the two --login sites above ended up different from the other twelve.

  2. branchEligibilityFromOptions restates two route enums by hand (["eligible", "ineligible", "unknown"] and the four sources) and silently drops anything it does not recognise, so a status the route accepts can be discarded by the CLI with no error. branchEligibilitySchema already declares both, but it is a zod pipe (it transforms), so its members are not reachable through it.

  3. optionalInteger and optionalNumber still take any — the annotation this whole issue exists to remove.

Requirements

  1. Every remaining raw read is narrowed, so a valueless flag is absent everywhere rather than at most sites.
  2. The three duplicated chains become single helpers, so there is one place to be correct and one place to test.
  3. branchEligibilityFromOptions parses against the route's own schema. The contract exports the pre-transform object so its members are reachable.
  4. optionalInteger/optionalNumber lose their any parameters.
  5. 100% patch coverage, branch-counted — including both arms of every fallback. The CLI suite runs the compiled dist/ through a subprocess harness, which v8 cannot instrument, so these must be exercised in-process or they will read as uncovered.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions