Skip to content

Commit 31bdcbb

Browse files
committed
types(mcp): finish the option-plumbing sweep, and cover both arms of every fallback (#9824)
The review found `explainReviewRiskCli` reading `options.login` raw. That one was the visible end of a family -- seven more sites read an option without narrowing it, each reachable by an ordinary typo: - `analyze-branch --login` and `review-pr --login` sent boolean `true` as the contributor login. - `login --github-token` with no value took the token branch with `true` as the token, instead of falling through to the device flow. - `Number(options.limit)` on a bare `--limit` is 1, so the flag silently capped an audit at one row rather than leaving the server's default in place. Same for `--window-days`. - `String(options.since)` sent `?since=true`, which the route reads as a cursor. - `validate-config --file` passed a `!options.file` guard (a bare flag is `true`, which is truthy) and then read the manifest from the path "". Rather than add an eighth copy of each chain, the duplicated ones are now single helpers -- `resolveLogin` (flag, profile session, then two env vars, at 14 call sites), `readOptionalTextFile` and `issueNumbersOption` -- so there is one place to get it right and one place to test. `optionalInteger` and `optionalNumber` lose their `any` parameters, which is this issue's subject. `branchEligibilityFromOptions` restated the route's two enums by hand and dropped anything it did not recognise. It parses against `branchEligibilityFields` now, exported from the contract for the purpose: the schema was a pipe, so its members were unreachable through it. Coverage: the diff is at 100% of lines AND branches in the CLI. Every fallback is exercised from both sides in `test/unit/mcp-cli-option-resolution.test.ts`, in-process -- the subprocess harness the rest of the CLI suite uses runs the compiled `dist/`, which v8 cannot instrument, so a path covered only there reads as uncovered and a regression in one of these arms would land unnoticed.
1 parent 9d7f567 commit 31bdcbb

4 files changed

Lines changed: 452 additions & 55 deletions

File tree

packages/loopover-contract/src/api-requests.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,16 +344,28 @@ export const linkedIssueContextSchema = z
344344
})
345345
.strict();
346346

347-
export const branchEligibilitySchema = z
347+
/**
348+
* The FIELDS a caller may send, before the route's normalisation runs (#9773).
349+
*
350+
* Exported separately because `branchEligibilitySchema` is a pipe (it transforms), so its members are not
351+
* reachable through it -- and the stdio CLI needs exactly those members to validate `--branch-eligibility`
352+
* and `--branch-eligibility-source` against the route's own vocabulary instead of restating both lists.
353+
*/
354+
export const branchEligibilityFields = z
348355
.object({
349356
status: z.enum(["eligible", "ineligible", "unknown"]),
350357
source: z.enum(["github_metadata", "local_metadata", "registry", "user_supplied"]).optional(),
351358
reason: z.string().max(MAX_LOCAL_BRANCH_TEXT_CHARS).optional(),
352359
checkedAt: z.string().max(MAX_LOCAL_BRANCH_REF_CHARS).optional(),
353360
stale: z.boolean().optional(),
354361
})
355-
.strict()
356-
.transform((value) => ({ ...value, status: value.status === "eligible" ? ("unknown" as const) : value.status, source: "user_supplied" as const }));
362+
.strict();
363+
364+
export const branchEligibilitySchema = branchEligibilityFields.transform((value) => ({
365+
...value,
366+
status: value.status === "eligible" ? ("unknown" as const) : value.status,
367+
source: "user_supplied" as const,
368+
}));
357369

358370
export const focusManifestInputSchema = z
359371
.record(z.string(), z.unknown())

0 commit comments

Comments
 (0)