Skip to content

Fix docs/release follow-ups from the mcp-types package split#2977

Merged
maxisbey merged 5 commits into
mainfrom
mcp-types-followups
Jun 26, 2026
Merged

Fix docs/release follow-ups from the mcp-types package split#2977
maxisbey merged 5 commits into
mainfrom
mcp-types-followups

Conversation

@maxisbey

@maxisbey maxisbey commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

Follow-ups from the review of #2973 (the review comment) — the items #2973 itself didn't pick up before it merged. Two of them are invisible to every check that ran green on that PR, because neither the docs build nor the publish path runs in PR CI.

Key changes

The published API reference (PR-introduced by #2973, no CI sees it). docs/hooks/gen_ref_pages.py derives the mkdocstrings identifier from the path relative to src/, which for the new package yields ::: mcp-types.mcp_types.* — and griffe resolves modules by walking the filesystem, not importlib, so it happily collects the hyphenated directory as a namespace package. The result is a green mkdocs build --strict that publishes the entire type system under the heading mcp-types.mcp_types (a dotted path that is a syntax error to import) while every existing link to /api/mcp/types/* 404s. Fixed by deriving each module's identifier from its own package root, adding src/mcp-types to the mkdocs paths: list and to the deploy-docs.yml trigger filter (which only listed src/mcp/**), and dropping the mkdocs watch: package list rather than extending it.

The publish path. The release job uploads both distributions in one trusted-publishing step. A partial upload — say, the mcp-types trusted publisher isn't configured yet, or PyPI hiccups on one project — used to be unrecoverable from the workflow alone, because re-running it failed on the files that had already landed. skip-existing: true makes a re-run skip whatever is already on PyPI and upload the rest, so a partial failure is fixed by re-running the job.

Development Status classifier. #2975 set mcp to 5 - Production/Stable but left src/mcp-types/pyproject.toml at 3 - Alpha, and the two distributions are published in lockstep with an exact == pin — so their PyPI metadata would contradict each other, immutably. mcp-types now matches, and RELEASE.md records that the classifier is permanently 5 - Production/Stable and is not part of any release.

A CI check for the package's headline contract. Nothing today fails if mcp_types grows an import httpx: tests, pyright, and ruff all run in the one workspace venv, which always has the full SDK dependency closure installed, so the import just resolves and everything stays green — the breakage only surfaces for the first user who installs mcp-types into a clean environment. A new step in the pre-commit job exercises the contract directly: uv run --isolated --no-project --with ./src/mcp-types resolves only the package's declared dependencies into an empty environment and imports every module, so a stray import of the SDK or anything from its stack is a ModuleNotFoundError and a red, blocking check (the pre-commit job is a hard gate; the test matrix is continue-on-error). Also moves tests/shared/test_version.py to tests/types/ to mirror its source module.

Docs. The migration guide's headline "After (v2)" example, from mcp import Tool, CallToolResult, raises ImportErrorCallToolResult was never in mcp.__all__. Swapped for Resource, which is. Also: name mcp_types.version as the home of the protocol-version constants (they're not in mcp_types.__all__ and the section never said where they went), link the new API reference from the section about the move, correct "depends only on pydantic" to also name typing-extensions, and point schema/README.md at the new codegen output path.

Contributor docs. [project].dependencies becoming dynamic means uv add <runtime-pkg> on the root project now fails (and with a misleading build-backend traceback that never points at the real edit site), so AGENTS.md's "Installation: uv add <package>" gets the exception. RELEASE.md now covers the two-distribution release and the trusted-publisher precondition.

Not done here, deliberately

  • Required before the next release tag, and not expressible in code: the mcp-types PyPI project (currently owned by Kludex alone, vs dsp/jspahrsummers/maxisbey on mcp) needs a Trusted Publisher matching mcp's — this repository, workflow publish-pypi.yml, environment release — plus the same owners. Without it the upload is rejected; the new step ordering bounds the blast radius to "nothing publishes" instead of "pip install mcp breaks", but it doesn't make the release succeed.
  • Recommended follow-up: an mkdocs build --strict job in PR CI is the mechanism that would have caught the gen_ref_pages bug (and its whole class) before merge. Left out of this PR because the build fetches three external objects.inv inventories, so it adds a real external-flakiness surface to every PR — that tradeoff deserves its own decision.
  • mcp_types.version is now load-bearing public API under a name that reads like "the package's version". Renaming (or re-exporting the registry from the top level) is free until the first mcp-types release and not after; raised on Split protocol types into a standalone mcp-types package #2973 and intentionally not decided unilaterally here.

Verification

  • gen_ref_pages.py executed against the tree: 97 pages, zero hyphenated identifiers, all six mcp_types.* ones resolve via griffe with the new paths; mkdocs build --strict exits 0 with api/mcp and api/mcp_types both emitted.
  • The standalone check was run verbatim, with a negative control: the isolated environment contains only mcp-types plus its declared closure (anyio, httpx, starlette, and mcp are all absent from it), and all six mcp_types modules import.
  • pypa/gh-action-pypi-publish at the pinned ref accepts skip-existing.
  • Targeted coverage on the new test: 100% with branch coverage, 0 partial; strict-no-cover clean; the full pre-commit suite (ruff, pyright strict, markdownlint, the uv.lock check) passes.

AI Disclaimer

This PR was developed with the assistance of Claude. The changes and the verification above were reviewed.

AI Disclaimer

@maxisbey maxisbey marked this pull request as ready for review June 26, 2026 05:33

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

Re-trigger cubic

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found in this review. Deferring to a human since the changes rework the PyPI publish workflow (a release-critical path that PR CI never exercises) and lean on out-of-band PyPI trusted-publisher setup, plus an open API-naming question (mcp_types.version) that maintainers should weigh in on.

Extended reasoning...

Overview

This PR is a follow-up to the mcp-types package split (#2973): it reworks the PyPI publish workflow into two ordered publish steps (mcp-types first, then mcp), fixes the docs reference generator so mcp_types modules get importable identifiers, adds src/mcp-types to the mkdocs paths/watch and the deploy-docs trigger, replaces the sdist-only LICENSE force-include with a real LICENSE file in src/mcp-types (content matches the root LICENSE exactly), adds an AST-based test asserting mcp_types only imports its declared dependencies, moves a test file, and updates several docs (migration guide, RELEASE.md, AGENTS.md, schema/README.md).

Security risks

No direct security-sensitive code is touched (no auth, crypto, or permission logic). The publish-pypi.yml changes do affect the trusted-publishing release path (the job has id-token: write), so they are supply-chain-adjacent: the action ref stays pinned to the same SHA and the new inputs (packages-dir, skip-existing) are standard, but a mistake here surfaces only at release time, not in PR CI.

Level of scrutiny

Most of the diff is documentation and a small, self-contained docs-generation hook change that I would happily approve on its own. However, the publish workflow restructuring is release-critical, cannot be validated by any CI that runs on this PR, and explicitly depends on PyPI-side configuration (the mcp-types trusted publisher and ownership) that only maintainers can confirm. The PR also intentionally leaves open a public-API naming decision (mcp_types.version) that the author flags as needing a maintainer call before the first mcp-types release. Those are exactly the kinds of judgment calls a human owner should make.

Other factors

The new tests/types/test_standalone.py is well-constructed (static AST analysis covers TYPE_CHECKING-only imports) and the author reports mutation-testing it. The docs changes were verified against an actual mkdocs build per the description. I found no correctness issues; the deferral is purely about the release-path ownership and the open design question, not about suspected bugs.

@maxisbey maxisbey force-pushed the mcp-types-followups branch from 20d2832 to 631efd8 Compare June 26, 2026 07:39

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any bugs — the docs/test changes look correct, but the publish-pypi.yml restructuring changes the release pipeline (which never runs in PR CI) and depends on out-of-band Trusted Publisher setup, so a maintainer should look this over before it lands.

Extended reasoning...

Overview

This PR is a set of follow-ups from the mcp-types package split (#2973): a fix to docs/hooks/gen_ref_pages.py so the new package's modules get importable mkdocstrings identifiers, mkdocs paths/watch and deploy-docs.yml trigger updates, a restructured publish-pypi.yml that builds and publishes mcp-types and mcp from separate dist directories with mcp-types first, a new AST-based test asserting mcp_types only imports its declared dependencies, a moved test file, and documentation updates (migration guide, AGENTS.md, RELEASE.md, schema/README.md).

Security risks

No meaningful security risk: no auth, crypto, or input-handling code is touched. The publish workflow keeps the same pinned action SHA and trusted-publishing setup; the only behavioral change is splitting the upload into two ordered steps with skip-existing: true on the mcp-types step.

Level of scrutiny

Most of the PR (docs, mkdocs config, the new static-import test) is low risk and reads correctly — the gen_ref_pages.py loop change is straightforward and the failure mode it fixes is well explained. The part that warrants a human maintainer's eyes is the release path: publish-pypi.yml cannot be exercised in PR CI, the ordering/skip-existing semantics directly determine what happens during a partially failed release, and the PR explicitly calls out a non-code precondition (the mcp-types PyPI project needs a matching Trusted Publisher and owners) that only a maintainer can verify and act on. Release-pipeline changes are exactly the kind of thing that should not be merged on a bot's say-so alone.

Other factors

The PR description is detailed and includes verification of the docs build, the new test's mutation behavior, and the build artifacts; no bugs were found by the bug hunting system, and there are no outstanding reviewer comments in the timeline. The decision to defer is about ownership of the release process and the out-of-band PyPI configuration, not about any defect found in the changes.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread .github/workflows/publish-pypi.yml
@maxisbey maxisbey enabled auto-merge (squash) June 26, 2026 10:17
@maxisbey maxisbey self-assigned this Jun 26, 2026
Comment thread mkdocs.yml
maxisbey added 5 commits June 26, 2026 10:53
- gen_ref_pages.py: derive each module identifier from its own package
  root, so the API reference emits `mcp_types.*` instead of the
  unimportable `mcp-types.mcp_types.*`. Add `src/mcp-types` to the mkdocs
  paths/watch lists and to the deploy-docs trigger filter.
- publish-pypi.yml: build the two distributions into separate directories
  and publish `mcp-types` before `mcp`, so a failed dependency upload can
  never leave `mcp` published with an unsatisfiable exact pin.
  `skip-existing` only on the mcp-types step, so a re-run recovers from a
  partial publish without muting a duplicate `mcp` upload.
- mcp-types: ship a real LICENSE file instead of the sdist-only
  force-include, so the wheel carries License-File in every build mode.
- tests: add a static check that every `mcp_types` module imports only the
  standard library and the dependencies `src/mcp-types/pyproject.toml`
  declares; move test_version.py to mirror its source module.
- docs: the migration guide taught `from mcp import CallToolResult`, which
  raises ImportError (`CallToolResult` is not in `mcp.__all__`) -- use
  `Resource`. Name `mcp_types.version` as the home of the protocol-version
  constants, link the new API reference, correct the "depends only on
  pydantic" wording, and point schema/README.md at the new codegen path.
- AGENTS.md / RELEASE.md: document that the root project's runtime
  dependencies are dynamic and live in the uv-dynamic-versioning hook
  table, and that a release now publishes both distributions.
…fier

- publish-pypi: build both packages into one dist/ and publish in a single
  step with skip-existing, so a partially failed upload is fixed by
  re-running the job rather than by step ordering.
- mkdocs: drop the watch package list instead of extending it.
- The Development Status classifier is permanently 5 - Production/Stable;
  say so in RELEASE.md and set it on mcp-types, which #2975 left at Alpha.
tests/types/test_standalone.py modeled the contract by parsing imports
with ast. A step in the pre-commit job now exercises the contract itself:
`uv run --isolated --no-project --with ./src/mcp-types` resolves only the
package's declared dependencies into an empty environment and imports
every module, so a stray import of the SDK or anything from its stack is
a ModuleNotFoundError. It also sits behind the blocking pre-commit job
rather than the continue-on-error test matrix.
mkdocstrings dropped its own watch hook in 0.23.0 in favour of MkDocs'
top-level watch:, so handlers.python.paths is not auto-watched. Without
this, editing anything under src/ during mkdocs serve would not trigger
a rebuild.
@maxisbey maxisbey force-pushed the mcp-types-followups branch from a8ead8d to 27290cf Compare June 26, 2026 10:55
@maxisbey maxisbey merged commit 3a8da8c into main Jun 26, 2026
32 checks passed
@maxisbey maxisbey deleted the mcp-types-followups branch June 26, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants