Conversation
`test/helpers`, `test/conformance` and `test/integration` each define `check` as `npm run typecheck && npm run lint`, but none of them defines a `typecheck` script. `pnpm -r typecheck` skips a package that lacks the script rather than failing, so `pnpm check:all` — the CI gate in `.github/workflows/main.yml` — silently typechecked 52 of 53 workspace projects and these three were never covered. Running `npm run check` in any of them fails outright with `Missing script: "typecheck"`, so the per-package entry point was broken too. Add the script, matching the `tsgo -p tsconfig.json --noEmit` form every `packages/*` project already uses, and the `@typescript/native-preview` devDependency that provides `tsgo`. Each package already has a `tsconfig.json`, so nothing else was needed to make them checkable. Enabling the check surfaces 35 pre-existing type errors that the gap had been hiding; they are fixed in the following commit so this one stays reviewable. Signed-off-by: Sharvil Saxena <sharvil.saxena@gmail.com>
Enabling `typecheck` on `test/conformance` and `test/integration` surfaces 35
type errors that CI had never seen. Each is a fault in the test code, not in
the SDK, and none changes what a test asserts:
- `noUncheckedIndexedAccess` violations: `tools[0].x` and `registered[2].x`
now use `?.`/`!`, matching how `packages/*/test` already writes these.
- `everythingServer.ts` casts the `x-mcp-header` property to
`Record<string, unknown>`, as `mcpParamValidation.test.ts` and
`scopeChallengeModern.test.ts` do — the annotation is not in the
`JSONSchema` property type.
- `dualEraStdio.test.ts` wrapped `onmessage`/`send` with a second parameter.
`StdioClientTransport` declares neither, so the extra arguments were
inert; the wrappers now match the real signatures.
- `mcp.test.ts` called `registerResource`/`registerPrompt` without the
required `config` argument, and built a flat `ServerContext` that no
longer matches the type.
- elicitation content values are `string | number | boolean | string[]`, so
`server.test.ts` omits an absent `username` instead of passing `undefined`,
and `elicitation.test.ts` annotates the handler return as `ElicitResult` so
each branch widens to the shared type.
- `client.test.ts`'s `roots/list` handler returns `{ roots: [] }`; the
assertion is that registration throws, so the value is never produced.
`pnpm -r typecheck` and `pnpm check:all` now pass across the whole workspace,
and `test/integration` stays at 371/371.
Signed-off-by: Sharvil Saxena <sharvil.saxena@gmail.com>
When `registerPrompt` is given a config with no `argsSchema`, `createPromptHandler` takes its `else` branch and invokes the callback as `callback(ctx)` — the server context is the only argument. Neither public overload describes that: both constrain `Args` to a schema type, so the argument-less form fell through to the deprecated raw-shape signature and typed the first parameter as the arguments record. A callback that reads `ctx.mcpReq` therefore failed to compile against behaviour that works at runtime, and there was no annotation that fixed it, since an explicit `PromptCallback` also failed to match either overload. Add an `argsSchema?: undefined` overload typing the callback as `PromptCallback`, and widen the implementation signature to accept it. Callbacks that declare no parameters already compiled and are unaffected, as is every schema-bearing form. This was invisible because the only caller in the repo lives in `test/integration`, which was not typechecked until the preceding commits. Signed-off-by: Sharvil Saxena <sharvil.saxena@gmail.com>
🦋 Changeset detectedLatest commit: 5a74dc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
Enabling `typecheck` on `test/conformance` passes against a tree that has
already been built, but fails from a clean checkout:
../../packages/client/src/client/auth.ts(1,34): error TS2307:
Cannot find module '@modelcontextprotocol/client/_shims'
or its corresponding type declarations.
(and the same for four more files across `client` and `server`).
`test/conformance/tsconfig.json` redirects `@modelcontextprotocol/client` and
`@modelcontextprotocol/server` to their `src/index.ts` so the conformance suite
typechecks against sources rather than build output. Those sources import the
`/_shims` subpath, which is only resolvable through the package's `exports`
map, and every target there is under `dist/`. With no `paths` entry for the
subpath, resolution falls through to `exports` and needs a build to exist.
CI runs `pnpm run check:all` before `pnpm run build:all`, so `dist/` is absent
and the typecheck fails. It passed locally only because a previous build had
left `dist/` behind.
Map the subpath to `src/shimsNode.ts`, which is what every other project that
redirects these packages to source already does: `test/e2e`,
`test/integration`, `examples/*` and all four `packages/middleware/*`
tsconfigs. `test/conformance` was the only one missing it.
Verified by removing every `packages/*/dist` directory to reproduce a clean
checkout: the five errors above appear before this change and none after.
Signed-off-by: Sharvil Saxena <sharvil.saxena@gmail.com>
|
The Enabling plus the same for four more files across Cause.
Fix. Map the subpath to Verification. I reproduced CI's clean checkout by deleting every Happy to split the tsconfig change into the first commit ( |
This branch has not been deployed
1 participant
Fixes #2840
What
pnpm -r typechecksilently skipstest/conformance,test/helpers, andtest/integrationbecause none of them defines atypecheckscript. All three definecheckasnpm run typecheck && npm run lint, so the intent was there; the script simply did not exist, and a missing script in a recursive run is a no-op rather than an error.35 type errors were sitting in the repo with CI green.
Three commits, deliberately separable
1.
build: run typecheck on the three test workspace packagesAdds the missing
typecheckscript and the@typescript/native-previewdevDependency it needs. This is the commit that makes the problem visible.2.
test: fix the type errors hidden by the missing typecheck scriptsThe 34 in
test/integrationand 1 intest/conformance. Test-side type drift; no runtime behavior changes.3.
fix(server): type the context-only registerPrompt callbackThe one real API defect among them, described below. Includes a changeset since it changes public types.
If you would rather take 1+2 now and discuss 3 separately, dropping the last commit is clean.
The API defect
When
registerPromptgets a config with noargsSchema,createPromptHandlertakes itselsebranch and callscallback(ctx)— the server context is the only argument. Neither public overload describes that: both constrainArgsto a schema type, so the argument-less form resolves to the deprecated raw-shape signature and types the first parameter as the arguments record.So this fails to compile while working correctly at runtime:
An explicit
PromptCallbackannotation does not help either, since it matches neither overload.The fix adds an
argsSchema?: undefinedoverload typing the callback asPromptCallback, and widens the implementation signature to accept it. Callbacks that declare no parameters already compiled and are unaffected, as is every schema-bearing form.This stayed invisible because the only caller of that form lives in
test/integration— the package that was never typechecked. Commit 1 and commit 3 are the same bug at two levels.Verification
test/conformancealone had 1 error,test/integration34,test/helpers0 (it was clean, but should still be guarded).I could not run
pnpm test:conformance:serveron my machine; it fails at browser launch there, and the identical failure reproduces on a clean checkout, so it is unrelated to this change.