Skip to content

Conversation

@felixweinberger
Copy link
Contributor

@felixweinberger felixweinberger commented Oct 22, 2025

Motivation and Context

The CI was incorrectly using --frozen with both resolution strategies.
The --frozen flag should only be used with the highest resolution
strategy (which uses the lockfile), while lowest-direct resolution
needs to resolve dependencies dynamically using --resolution lowest-direct.

This change restructures the matrix to properly pass the correct flags
for each resolution strategy, fixing the CI pipeline.

Github-Issue: #1325

How Has This Been Tested?

CI

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@felixweinberger felixweinberger force-pushed the fweinberger/fix-pipeline branch 5 times, most recently from 59642be to 63363bf Compare October 23, 2025 03:08
run: uv run --frozen --no-sync pytest
run: uv run ${{ matrix.dep-resolution.install-flags }} --no-sync pytest
env:
UV_RESOLUTION: ${{ matrix.dep-resolution.name == 'lowest-direct' && 'lowest-direct' || 'highest' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed to ensure subprocesses use the same resolution strategy when they run uv sync rather than overwrite the venv as in @maxisbey's comment: #1325 (comment)

@felixweinberger felixweinberger marked this pull request as ready for review October 23, 2025 05:03
@felixweinberger felixweinberger marked this pull request as draft October 23, 2025 05:03
@felixweinberger felixweinberger force-pushed the fweinberger/fix-pipeline branch from 9c7c2a4 to fc7836d Compare October 23, 2025 05:06
- Restructure dep-resolution matrix with name and install-flags
- Use uv sync with appropriate flags for each resolution strategy
- Set UV_RESOLUTION env var to ensure test subprocesses use correct strategy

This fixes the issue where tests spawning uv run subprocesses were
re-resolving dependencies using the lockfile instead of respecting
the lowest-direct resolution strategy.
@felixweinberger felixweinberger force-pushed the fweinberger/fix-pipeline branch from fc7836d to 0a22bb6 Compare October 23, 2025 16:38
@felixweinberger felixweinberger marked this pull request as ready for review October 23, 2025 16:42
Kludex
Kludex previously approved these changes Oct 23, 2025
@felixweinberger felixweinberger force-pushed the fweinberger/fix-pipeline branch from 48280fe to aed48b3 Compare October 23, 2025 17:06
@Kludex
Copy link
Member

Kludex commented Oct 23, 2025

Are my reviews being dismissed on any code change or on force-pushes? 🤔

@felixweinberger
Copy link
Contributor Author

Are my reviews being dismissed on any code change or on force-pushes? 🤔

Should be on any code change, see: https://github.com/modelcontextprotocol/python-sdk/rules/3187917

CleanShot 2025-10-23 at 10 25 16

@felixweinberger felixweinberger merged commit 9f9dbad into main Oct 23, 2025
21 checks passed
@felixweinberger felixweinberger deleted the fweinberger/fix-pipeline branch October 23, 2025 17:25
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.

3 participants