Commit f01c2d4
authored
* types(mcp): type the stdio CLI's option plumbing, fixing three crashes it hid (#9773)
`parseOptions` is now typed from CLI_FLAG_SPEC -- repeatable flags as arrays, boolean flags as
booleans, anything else as `string | boolean` behind an index signature, because the parser
genuinely accepts any `--flag` and a closed record would be a lie. That type flows into every
`options` parameter, every argv parameter becomes `readonly string[]`, and the config parameters
take the contract's LoopoverConfig.
Three defects fell out immediately, each reproduced against main before the fix:
TypeError: (options[key] ?? []) is not iterable
repoFullName.includes is not a function
LoopOver API 404: {"error":"not_found"}
The first is `--issue --issue 5`: a bare repeatable flag is stored as `true` by the no-value
branch, and the accumulator then spread it. Anything not already a list now starts a fresh one --
the only sane reading of a flag that carried no value to keep.
The second is `maintain <sub> --repo` with no value. `true` passed the `!repoFullName` truthiness
guard and then died on a string method, where "Pass --repo owner/repo." was intended.
The third is a bare `--login`, read as the literal string "true", so `decision-pack --login`
requested a contributor NAMED "true" and reported them not found instead of saying the value was
missing. Options are read through optionText() now, which treats a valueless flag as absent -- and
every one of those call sites already had an env or profile fallback for absent.
Also: the contract's LoopoverConfig was missing `session`, `telemetryEnabled`, and profile
`createdAt`, all three read and written by the CLI with nothing checking they existed. The legacy
top-level `session` is still written on the default profile so an older CLI reading the same file
keeps working, which is exactly why it cannot be left undeclared.
277 -> 184 `: any` occurrences in the bin. The remainder is a long tail of callbacks over API
payloads that stay untyped for a structural reason worth its own issue: CLI_RESPONSE_SCHEMAS
covers only the 24 STATIC paths, so all 53 parameterised calls fall through to the untyped
overload. #9773 stays open for that.
* types(mcp): type the CLI's parameterised API calls from the published document (#9773)
The second tranche. #9521 built the typed accessors; its scanner rejected any template containing an
interpolation, so every per-repo and per-contributor call -- the majority of the CLI -- missed the
typed overload and read its payload as `any`. The document already described most of them.
Three things had to change for a composed call to resolve.
The path builders now DECLARE their shape. `toolRepoBase` returned `string`, which erases the path
at the type level, so `apiGet(`${toolRepoBase(o, r)}/settings`)` could never match anything; it and
the 24 locally-built bases now carry template-literal types. That is also what lets the generator's
scanner resolve them: it reads the same declarations the type checker does, so the two cannot
disagree about what a base is.
The tables are keyed by METHOD, not by path. `/v1/repos/{owner}/{repo}/agent/pending-actions` lists
on GET and proposes on POST, and those return different shapes -- a path-keyed table had to guess,
and the first version of this guessed `post`, handing the GET call site the POST response type. The
CLI's own `payload.pendingActions` read is what contradicted it, the moment a schema was attached at
all. Caught by the type checker before it shipped.
And the generated copy now carries what a copied schema REFERENCES. `closure` followed only
`*Schema` names, so a schema depending on a plain value beside it (`AGENT_ACTION_CLASS_VALUES`)
emitted a file that would not compile. Values declared in the source are copied; anything else is
imported from the contract's limits.ts, where it is restated and pinned -- and a bound missing there
fails the contract build rather than emitting something broken.
30 parameterised calls are typed now, up from 8, and the guards are in mcp-api-client.test.ts:
method disambiguation, base-path resolution, the copied-value closure, and the prose false-positive
the first cut of the constant scanner hit (it emitted imports for DELETE, REQUIRED and REST, read
out of doc comments).
Still `any` at the fallback overload, for the endpoints whose 200 the document does not describe
with a named schema. Flipping that to `unknown` leaves 72 narrowing sites, and the honest fix for
them is to describe those endpoints -- #9773 stays open for it.
* types(mcp): type what the CLI SENDS, and correct the four request schemas that lied (#9773)
A review found a contributor login being sent to the API as boolean `true` -- a bare `--login`
that my sweep had missed. The first answer was a test that grepped the source for the shape;
that is a guard against one spelling, not against the defect, so it is gone.
The defect is now a compile error. `apiPost`'s body is typed from the request schemas the
published document names, so an option value -- `string | boolean | string[]`, because a bare flag
is `true` -- cannot reach a field the API declares as a string. Verified by reverting one fix and
watching tsc say `Type 'boolean' is not assignable to type 'string'`.
Making the types BINDING mattered as much as adding them. The fallback overloads accepted
`path: string, body: unknown`, so a call that failed a typed overload did not error -- it fell
through and was accepted unchecked. They now refuse any path the typed overloads cover.
That found three more instances of the reviewer's class, each in a different command:
`lint-pr-text` sent `--body` as `true`, `check-slop-risk` sent `--description` as `true`, and
`validate-focus-manifest` sent `--source` as an unchecked free string where the API takes three
literals -- its `.includes()` guard never narrowed, so the body kept the raw value. The last is
now parsed against the contract's own enum, so the accepted values and the error naming them come
from the schema the route validates with.
And four published request schemas were wrong. ValidateLinkedIssueRequest required `owner` and
`repo` in the BODY though both are path params; CheckSlopRiskRequest required `changedFiles` the
handler has optional; ValidateFocusManifestRequest typed an enum as a free string. They were
hand-written parallels of the schemas the handlers actually parse with, and they had drifted --
so they are now built from those schemas. Rebuilt via `z.object(shape)` rather than used directly,
because `.openapi()` exists only after `extendZodWithOpenApi` and the contract must never run it.
The generator carries what a copied schema references, resolved against what each module really
exports: bounds from limits.ts, request schemas from api-requests.ts. A name in neither fails the
contract build instead of emitting a dangling reference.
* types(mcp): narrow the CLI's closed-set guards, and discover the contract's modules (#9773)
Two things the merge with #9762 exposed, now that the action-class and autonomy-level lists are
readonly literal tuples rather than `string[]`:
- The CLI validated `<action>` and `<level>` with `LIST.includes(value)` and then passed the still-
`string` value to a typed request. `includes` returns a boolean and narrows nothing, so the check
ran and the type system learned nothing from it. `isOneOf` is the same check written as a type
predicate, so a validated value arrives at the API as the union it was just proved to be.
- The generator resolved a copied schema's constants against a hardcoded pair of contract modules.
That is a hand-maintained list by another name, and it fails in the quietest way available: a
constant that moves between modules yields a generated file referencing a name it never imported.
It now reads the contract's source directory, so a constant can move -- or a module can appear --
without this script knowing anything about it.
Regression test pins the discovery against wherever PUBLIC_SURFACE_SKIP_REASONS lives, rather than
against the module it happens to live in today.
1 parent 55b6a82 commit f01c2d4
12 files changed
Lines changed: 2305 additions & 278 deletions
File tree
- apps/loopover-ui/public
- packages
- loopover-contract/src
- loopover-mcp/bin
- scripts
- src/openapi
- test/unit
- support
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15869 | 15869 | | |
15870 | 15870 | | |
15871 | 15871 | | |
15872 | | - | |
| 15872 | + | |
| 15873 | + | |
| 15874 | + | |
15873 | 15875 | | |
15874 | 15876 | | |
15875 | | - | |
| 15877 | + | |
| 15878 | + | |
15876 | 15879 | | |
15877 | 15880 | | |
15878 | | - | |
| 15881 | + | |
| 15882 | + | |
15879 | 15883 | | |
15880 | 15884 | | |
15881 | 15885 | | |
15882 | 15886 | | |
15883 | 15887 | | |
15884 | | - | |
| 15888 | + | |
| 15889 | + | |
15885 | 15890 | | |
15886 | 15891 | | |
15887 | | - | |
| 15892 | + | |
| 15893 | + | |
15888 | 15894 | | |
15889 | 15895 | | |
15890 | 15896 | | |
15891 | 15897 | | |
15892 | | - | |
15893 | | - | |
| 15898 | + | |
| 15899 | + | |
| 15900 | + | |
| 15901 | + | |
15894 | 15902 | | |
15895 | 15903 | | |
15896 | 15904 | | |
15897 | 15905 | | |
15898 | | - | |
15899 | | - | |
| 15906 | + | |
| 15907 | + | |
| 15908 | + | |
| 15909 | + | |
15900 | 15910 | | |
15901 | 15911 | | |
15902 | 15912 | | |
15903 | 15913 | | |
15904 | | - | |
15905 | | - | |
| 15914 | + | |
| 15915 | + | |
| 15916 | + | |
| 15917 | + | |
15906 | 15918 | | |
15907 | 15919 | | |
15908 | 15920 | | |
15909 | 15921 | | |
15910 | 15922 | | |
15911 | 15923 | | |
15912 | 15924 | | |
15913 | | - | |
15914 | | - | |
15915 | | - | |
15916 | | - | |
| 15925 | + | |
15917 | 15926 | | |
15918 | 15927 | | |
15919 | 15928 | | |
| |||
16217 | 16226 | | |
16218 | 16227 | | |
16219 | 16228 | | |
16220 | | - | |
| 16229 | + | |
| 16230 | + | |
16221 | 16231 | | |
16222 | 16232 | | |
16223 | 16233 | | |
| |||
16288 | 16298 | | |
16289 | 16299 | | |
16290 | 16300 | | |
16291 | | - | |
16292 | | - | |
16293 | | - | |
16294 | | - | |
16295 | | - | |
16296 | | - | |
16297 | 16301 | | |
16298 | 16302 | | |
16299 | 16303 | | |
| |||
16303 | 16307 | | |
16304 | 16308 | | |
16305 | 16309 | | |
16306 | | - | |
| 16310 | + | |
| 16311 | + | |
| 16312 | + | |
16307 | 16313 | | |
16308 | 16314 | | |
16309 | 16315 | | |
16310 | 16316 | | |
16311 | | - | |
16312 | | - | |
| 16317 | + | |
| 16318 | + | |
| 16319 | + | |
| 16320 | + | |
16313 | 16321 | | |
16314 | 16322 | | |
16315 | | - | |
| 16323 | + | |
| 16324 | + | |
| 16325 | + | |
16316 | 16326 | | |
16317 | 16327 | | |
16318 | 16328 | | |
16319 | 16329 | | |
16320 | 16330 | | |
16321 | | - | |
16322 | | - | |
16323 | 16331 | | |
16324 | 16332 | | |
16325 | 16333 | | |
| |||
16358 | 16366 | | |
16359 | 16367 | | |
16360 | 16368 | | |
16361 | | - | |
16362 | | - | |
16363 | | - | |
16364 | | - | |
16365 | | - | |
16366 | | - | |
16367 | 16369 | | |
16368 | 16370 | | |
16369 | 16371 | | |
16370 | 16372 | | |
16371 | 16373 | | |
16372 | 16374 | | |
16373 | | - | |
| 16375 | + | |
| 16376 | + | |
| 16377 | + | |
16374 | 16378 | | |
16375 | 16379 | | |
16376 | 16380 | | |
16377 | 16381 | | |
16378 | | - | |
16379 | | - | |
| 16382 | + | |
| 16383 | + | |
| 16384 | + | |
| 16385 | + | |
16380 | 16386 | | |
16381 | | - | |
16382 | | - | |
16383 | | - | |
16384 | | - | |
16385 | | - | |
| 16387 | + | |
16386 | 16388 | | |
16387 | 16389 | | |
16388 | 16390 | | |
| |||
0 commit comments