Bump ModelContextProtocol and ModelContextProtocol.AspNetCore - #2100
Conversation
dependabot
Bot
added
.NET
erikdarlingdata
left a comment
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
dependabot
Bot
changed the title
2cf0150 to
785e082
Compare
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>
785e082 to
b0890f7
Compare
…te lock files Dependabot only bumped PlanAnalysis and Darling.Service; Common, Lite, and the deprecated Dashboard still pinned 2.0.0 directly, which is NU1605 (downgrade, warning-as-error in test projects), and the transitive lock files were never re-evaluated, which is NU1004 in locked-mode restore. One version everywhere plus --force-evaluate lock regeneration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 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.
This adds a brand-new direct PackageReference to ModelContextProtocol on the shared PerformanceMonitor.PlanAnalysis project — it wasn't here before this PR. Two things stand out:
grep -rl "ModelContextProtocol" PerformanceMonitor.PlanAnalysis/only matches this line — no source file in this project actually uses the MCP SDK. It looks unused.- This PR's title/body describe a pure version bump (
ModelContextProtocol/ModelContextProtocol.AspNetCore2.0.0 → 2.1.0) sourced from a Dependabot changelog. Dependabot doesn't add new package references, only bump existing ones — so this addition is out of scope for what the PR claims to do, and it fans out into every consumer'spackages.lock.json(Darling.Tests, Darling.Viewer, Lite, Lite.Tests, deprecated/Dashboard all pick up a new"ModelContextProtocol": "[2.1.0, )"dependency onperformancemonitor.plananalysis).
Every other PackageReference in this repo's .csproj files carries a `` comment (see the AN4 comment a few lines up in the Darling Service csproj, or the comment above Microsoft.Data.SqlClient right here). This new line has none, and I can't find the justification. Was this meant to land in a different PR, or should it be dropped from this one?
| "requested": "[10.0.10, )", | ||
| "resolved": "10.0.10", | ||
| "contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg==" | ||
| "requested": "[10.0.8, )", |
There was a problem hiding this comment.
Unrelated to the ModelContextProtocol bump this PR is about: Microsoft.NET.ILLink.Tasks is being downgraded here, 10.0.10 → 10.0.8 (confirmed against origin/dev, which has 10.0.10). This package isn't referenced by any .csproj/.props in the repo — it's the SDK's implicit trimming-tasks package, so its "requested" version tracks whatever .NET SDK patch generated the lock file. global.json pins the SDK to 10.0.302 with rollForward: latestPatch, so a clean restore on the pinned SDK shouldn't produce 10.0.8 here. This looks like the lock file was regenerated with a different/older SDK than the rest of the repo used, and it's an accidental downgrade riding along with the intended bump rather than something this PR meant to change.
|
Reviewed this as a T-SQL/parity-focused pass, but the actual diff is entirely .NET dependency plumbing (no SQL changes), so the review is scoped accordingly. Two inline comments posted on concrete issues; summary below. Scope creep beyond the stated bump. The PR title/description claim a pure Line-ending inconsistency. Lite/Darling parity: the actual MCP version bump itself is applied consistently to both apps (Lite, Darling Service, and the deprecated Dashboard all move No SQL Server object changes, no security-relevant surface touched (no new I/O, credential, or query-construction code), and no missing-index-DMV concerns apply here since this is exclusively package/lock-file churn. |
…K-artifact lock delta - PerformanceMonitor.PlanAnalysis: remove the ModelContextProtocol reference dependabot materialized — no source in the project uses the SDK, and it forced MCP into every consumer's closure (review round 1). - Rebuild the dependabot-touched csprojs from dev so the diff is the two version-bump lines per file instead of whole-file CRLF rewrites. - deprecated/Installer lock: revert the ILLink.Tasks 10.0.10 -> 10.0.8 downgrade — an artifact of regenerating locks on a 10.0.300 SDK locally; the file's graph is untouched by this bump so dev's entry is correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review round taken, all four findings addressed in the follow-up commit:
|
|
Reviewed. This PR is a mechanical Dependabot bump of
No findings. LGTM. |
Central package management + dependabot major/minor split (#2100 follow-up)
50k vs 200k rows was not enough: a fast runner's fixed per-run cost plus cache warmth accumulating across the two measured runs inverted the pair in the field (d1=279ms, d4=217ms, on PR erikdarlingdata#2100's unrelated dependency bump). 50k vs 500k puts the variable term an order of magnitude above the observed fixed-cost floor, so monotonicity is physics again instead of a coin flip. Test-only change; the telemetry and alert assertions are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gdata#2100 follow-up) Directory.Packages.props now owns every package version; csprojs keep bare PackageReferences. A dependency bump is one line and every consumer moves atomically - the multi-project misalignment dependabot shipped in PR erikdarlingdata#2100 (NU1605 downgrade errors from a partial bump) cannot recur. tools/CompactionRepro keeps its deliberate DuckDB 1.5.3 via VersionOverride. Lock files regenerate as CPM format v2. dependabot.yml: the nuget group takes only minor+patch updates, so a major lands as its own PR instead of hiding in a routine batch (Studio's convention). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
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.