Bump ModelContextProtocol and ModelContextProtocol.AspNetCore - #2100
Conversation
erikdarlingdata
left a comment
There was a problem hiding this comment.
Summary
Dependabot grouped bump of ModelContextProtocol + ModelContextProtocol.AspNetCore from 2.0.0 → 2.1.0. Targets dev — good.
What the PR does
- Bumps
ModelContextProtocol/ModelContextProtocol.AspNetCoreinDarling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj:30-31(the only project that legitimately holds these references ondev). - Regenerates
Lite/packages.lock.jsonanddeprecated/Dashboard/packages.lock.json. - Also adds a NEW
ModelContextProtocolPackageReferenceto two projects that did not have it:Darling/PerformanceMonitor.Darling.Analysis/PerformanceMonitor.Darling.Analysis.csproj:20andPerformanceMonitor.PlanAnalysis/PerformanceMonitor.PlanAnalysis.csproj:25. Confirmed against base3022f3a.
Needs attention
- Blocker — dependency scope creep. The two new
PackageReferenceadditions are not version bumps; they widen the dependency graph.PlanAnalysisis the shared library that Lite, Dashboard, and Darling all consume, so pulling MCP into it forces MCP into every consumer's closure for no code reason.Darling.Analysisis the pure analysis library; MCP hosting already lives inDarling.Service. Both additions should be reverted before merge. Inline comments on each line. - CI is red.
buildandDarling PostgreSQL testsboth failed on this PR (Darling Linux build,review, andcheck-branchesare green). Given the two new PackageReference additions, this is likely related; worth reading the failing job logs before force-rebasing. - CRLF↔LF flip on every touched csproj. All four csproj files show every line as changed even where the text is identical — Dependabot's writer flipped line endings. Not a merge blocker, but if
.csprojshould be CRLF-locked here, tighten.gitattributesso future grouped bumps don't repaint every line. deprecated/is being tracked by Dependabot.deprecated/Dashboard/packages.lock.jsonwas regenerated. Consider excludingdeprecated/**from the nuget group in.github/dependabot.yml— inline comment on the file.
What's fine
- Base branch is
dev, per.github/dependabot.yml. - No SQL install/upgrade scripts, no
PlanAnalyzer.cs, nobuild.ymlchanges — none of the PlanAnalyzer-sync / upgrade-path / SignPath gates apply here.
If Dependabot's grouped update logic is going to keep introducing new PackageReferences on 2.x → 2.y bumps, that is worth flagging upstream / considering a groups.nuget.update-types restriction.
Generated by Claude Code
| <!-- The plan fetcher connects to the MONITORED SQL Server (same client the Service uses); | ||
| Npgsql and Logging.Abstractions flow transitively from the Storage/Notifications references. --> | ||
| <PackageReference Include="Microsoft.Data.SqlClient" Version="7.0.2" /> | ||
| <PackageReference Include="ModelContextProtocol" Version="2.1.0" /> |
There was a problem hiding this comment.
Not a version bump — this is a brand-new PackageReference. ModelContextProtocol was not declared on dev for this project (confirmed against base 3022f3a), so Dependabot's grouped update materialized it here as a side effect of bumping the reference that legitimately lives in PerformanceMonitor.Darling.Service.
Darling.Analysis is the analysis library (DarlingAnalysisService/PgPlanFetcher) — MCP is a hosting concern that already lives in Darling.Service (line 30 of that csproj). Adding it here expands the dependency surface of a plain library for no code reason and is likely contributing to the failing build/Postgres-tests checks on this PR.
This addition should be reverted before merge; only the version numbers on the projects that already declared ModelContextProtocol/ModelContextProtocol.AspNetCore should change.
Generated by Claude Code
| XML), so it needs the SqlClient runtime connection surface. Version matches Lite/Dashboard/the Darling | ||
| service (7.0.1) so there is no transitive version skew. --> | ||
| <PackageReference Include="Microsoft.Data.SqlClient" Version="7.0.2" /> | ||
| <PackageReference Include="ModelContextProtocol" Version="2.1.0" /> |
There was a problem hiding this comment.
Same problem as the Darling.Analysis csproj, and worse here: PerformanceMonitor.PlanAnalysis is the SHARED plan-analysis library referenced by Lite, Dashboard, and the Darling service (via InternalsVisibleTo on PerformanceMonitorLite/PerformanceMonitorDashboard/Lite.Tests/Dashboard.Tests/Darling.Tests, plus the direct ProjectReference from Darling.Service.csproj).
Adding ModelContextProtocol here forces MCP into every consumer's closure — Lite, Dashboard, Darling — for a library whose stated purpose (per the comment two lines above) is running ActualPlanExecutor against SqlClient. There is no MCP surface in this project; the MCP tools that USE the shared parser live in Darling.Service and in Lite/Dashboard.
Confirmed against base 3022f3a: this line did not exist before. Revert this addition; only bump the versions in projects that already declared the reference.
Generated by Claude Code
There was a problem hiding this comment.
Dependabot is regenerating the lockfile for code under deprecated/. The folder name signals this is not shipped, so bumping it produces review noise and, worse, could keep pinning deprecated code to new package graphs indefinitely.
Consider excluding deprecated/** from the nuget update in .github/dependabot.yml (either move to directories: scoping or add an ignore: entry / paths filter), so grouped bumps only touch the active projects.
Generated by Claude Code
| <InternalsVisibleTo Include="Darling.Tests" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| <Project Sdk="Microsoft.NET.Sdk.Worker"> |
There was a problem hiding this comment.
All four touched csproj files show every line as changed even where the text is identical (-<Project Sdk=...> / +<Project Sdk=...> on line 1, and so on). That is the signature of a CRLF↔LF flip introduced by Dependabot's writer.
If the repo's convention is CRLF for .csproj (typical for .NET/Windows tooling), add or tighten .gitattributes — e.g. *.csproj text eol=crlf — so future bumps don't retouch every line and drown the real diff.
Generated by Claude Code
Bumps ModelContextProtocol from 2.0.0 to 2.1.0 Bumps ModelContextProtocol.AspNetCore from 2.0.0 to 2.1.0 --- updated-dependencies: - dependency-name: ModelContextProtocol dependency-version: 2.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget - dependency-name: ModelContextProtocol.AspNetCore dependency-version: 2.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget ... Signed-off-by: dependabot[bot] <support@github.com>
2cf0150 to
785e082
Compare
Pinned ModelContextProtocol at 2.1.0.
Release notes
Sourced from ModelContextProtocol's releases.
2.1.0
This release adds an opt-in
subscriptions/listenhandler for custom server-side notification streams, improves HTTP transport fallback reliability, and expands guidance and samples for telemetry and binary resources.What's Changed
Documentation Updates
Test Improvements
Repository Infrastructure Updates
> [!NOTE]alert example #1771 by @PranavSenthilnathan (co-authored by @Copilot)Acknowledgements
Full Changelog: modelcontextprotocol/csharp-sdk@v2.0.0...v2.1.0
Commits viewable in compare view.
Updated ModelContextProtocol.AspNetCore from 2.0.0 to 2.1.0.