Skip to content

fix(ci): pin mcp additional_dependency in mypy pre-commit hook - #2302

Open
JSv4 wants to merge 1 commit into
mainfrom
fix/pin-mcp-precommit-mypy-hook
Open

fix(ci): pin mcp additional_dependency in mypy pre-commit hook#2302
JSv4 wants to merge 1 commit into
mainfrom
fix/pin-mcp-precommit-mypy-hook

Conversation

@JSv4

@JSv4 JSv4 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test plan

  • pre-commit run mypy --files opencontractserver/mcp/server.py passes locally with the fix
  • pre-commit run --files .pre-commit-config.yaml (full suite) passes
  • CI green on this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_0199pawBYHjfsWUyPyrhRRtu

The mypy hook's additional_dependencies list pins nearly every entry to
match requirements/base.txt's ceiling, but mcp>=1.0.0 had no ceiling.
pip resolved that to mcp 2.1.1 in the hook's isolated env, whose SDK
renamed/removed fields and methods that opencontractserver/mcp/server.py
(written against the 1.x API, requirements/base.txt pins <2) still uses,
producing 48 mypy errors unrelated to any given PR's actual diff and
failing linter/backend-ci-gate on every open PR that touches backend code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0199pawBYHjfsWUyPyrhRRtu
@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown

Reviewed the diff — this is a clean, minimal, well-targeted fix.

Correctness

  • Verified requirements/base.txt:85 pins mcp>=1.28.1,<2; the new hook constraint mirrors it exactly, so mypy's isolated env and the actual runtime env now resolve the same major version. That's the right fix for the root cause (unbounded mcp>=1.0.0 letting pip pull in mcp 2.x with its renamed Server.list_resources/uriTemplateuri_template/inputSchemainput_schema API).
  • Consistent with the existing convention a few lines above (openai>=2.11.0,<3, anthropic>=0.45.2,<1), so it doesn't introduce a new pattern — just applies the existing one where it was missing.

Code quality

  • The added comment explains why the ceiling exists (a hidden constraint — mismatched hook/runtime mcp versions silently breaking CI on unrelated PRs), which is exactly the kind of non-obvious rationale worth a comment per the repo's comment guidance.
  • Changelog fragment follows the changelog.d/<slug>.<type>.md convention correctly and is appropriately detailed (file, symbols, impact).

Risk / scope

  • Config-only change, no production code touched, blast radius is limited to the mypy pre-commit hook's isolated environment. Low risk.
  • No test coverage gap here — this is infra/tooling config, and the PR description notes local verification (pre-commit run mypy --files opencontractserver/mcp/server.py and the full pre-commit suite) as the practical validation, which is appropriate for this kind of change.

No issues found. One very minor, non-blocking thought: since this class of bug (hook dependency drifting ahead of requirements/base.txt) has now broken CI twice-ish across multiple PRs, it might be worth a follow-up (not this PR) to add a lint/check that diffs pinned additional_dependencies versions against requirements/base.txt for the packages that intentionally mirror it — but that's speculative scope creep, not something this PR needs to solve.

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

1 participant