Upgrade project to .NET 10 #1325
Conversation
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
📝 WalkthroughWalkthroughUpdated project from .NET 8.0 → 10.0 across docs, Docker, CI, project targets and packages; Program startup adjusted (DI registrations, data-protection persistence, logging, routing) and several singleton services were registered. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
BUILDING.md(1 hunks)CLAUDE.md(2 hunks)Dockerfile(2 hunks)src/ESPresense.Companion.csproj(2 hunks)tests/ESPresense.Companion.Tests.csproj(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Place backend C# ASP.NET Core code under src/ (controllers, services, models, utils)
Files:
src/ESPresense.Companion.csproj
🧠 Learnings (1)
📚 Learning: 2025-09-06T14:09:17.125Z
Learnt from: CR
Repo: ESPresense/ESPresense-companion PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-06T14:09:17.125Z
Learning: Run backend and frontend build/test commands as specified (dotnet build/test; pnpm scripts) during development
Applied to files:
CLAUDE.mdBUILDING.md
🪛 GitHub Actions: Build and test
tests/ESPresense.Companion.Tests.csproj
[error] 1-1: dotnet restore failed: NETSDK1045 The current .NET SDK does not support targeting .NET 10.0. Either target .NET 9.0 or lower, or use a version of the .NET SDK that supports .NET 10.0. Download the .NET SDK from https://aka.ms/dotnet/download
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Deploy to Docker add-on with tag
- GitHub Check: Analyze (csharp)
🔇 Additional comments (4)
BUILDING.md (1)
3-3: Documentation accurately reflects .NET 10 SDK requirement.The requirement update aligns with the framework upgrade across all project files and is consistent with the Dockerfile and csproj targets.
Dockerfile (1)
2-2: Container base images correctly updated to .NET 10.Both build and runtime stages are properly updated. The platform-specific image handling and build/publish steps remain consistent.
Also applies to: 27-27
tests/ESPresense.Companion.Tests.csproj (1)
4-4: Test project correctly updated for .NET 10 compatibility.Microsoft.NET.Test.Sdk version 18.0.1 includes specific fixes for .NET 10 SDK, making it the appropriate choice. The Microsoft.Extensions package versions (10.0.0) align with the .NET 10 target framework.
Also applies to: 14-16
CLAUDE.md (1)
9-9: Developer documentation correctly reflects .NET 10 requirements.Updates are consistent with the framework upgrade, and the documented development commands (dotnet watch, dotnet test, etc.) remain valid for .NET 10.
Also applies to: 62-62
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Program.cs (1)
63-69: Refactor HistoryController to use injected DeviceHistoryStore singleton instead of DatabaseFactory.The verification confirms the dual pattern issue:
HistoryController(lines 27, 35 in src/Controllers/HistoryController.cs) creates newDeviceHistoryStoreinstances viaDatabaseFactory.GetDeviceHistory()on each request, whileMultiScenarioLocatoris injected with the singleton. This causes multiple instances with no shared state.Required fix:
HistoryControllershould injectDeviceHistoryStoredirectly (likeMultiScenarioLocatordoes)- Replace
await _databaseFactory.GetDeviceHistory()calls with the injected singleton- After refactoring, remove
DatabaseFactory.GetDeviceHistory()method if no other code uses it
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)src/Program.cs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Place backend C# ASP.NET Core code under src/ (controllers, services, models, utils)
Files:
src/Program.cs
{src,tests}/**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
{src,tests}/**/*.cs: C#: Use spaces with an indent size of 4
C#: Use PascalCase for types and methods
C#: Use camelCase for local variables and parameters
Files:
src/Program.cs
🧠 Learnings (1)
📚 Learning: 2025-09-06T14:09:17.125Z
Learnt from: CR
Repo: ESPresense/ESPresense-companion PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-06T14:09:17.125Z
Learning: Run backend and frontend build/test commands as specified (dotnet build/test; pnpm scripts) during development
Applied to files:
.github/workflows/ci.yml
🧬 Code graph analysis (1)
src/Program.cs (2)
src/Services/DatabaseFactory.cs (1)
DatabaseFactory(6-12)src/Utils/MqttNetLogger.cs (1)
MqttNetLogger(7-15)
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
21-21: input "include-prerelease" is not defined in action "actions/setup-dotnet@v5". available inputs are "cache", "cache-dependency-path", "config-file", "dotnet-quality", "dotnet-version", "global-json-file", "owner", "source-url"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Deploy to Docker add-on with tag
- GitHub Check: build
- GitHub Check: Analyze (csharp)
🔇 Additional comments (10)
src/Program.cs (10)
25-27: LGTM: Logging configuration looks good.The Serilog and MathNet.Numerics logging setup is correct for .NET 10.
29-36: LGTM: Configuration directory and DI setup is correct.The config directory fallback logic and ConfigLoader registrations follow proper patterns for .NET 10.
38-39: LGTM: Data protection persistence is a good improvement.Persisting keys to the filesystem ensures they survive application restarts, which is important for data protection scenarios.
57-59: LGTM: MQTT service registrations follow proper DI patterns.The pattern of registering
MqttCoordinatoras both a concrete type and exposing it via theIMqttCoordinatorinterface is correct.
80-81: LGTM: JSON options configuration is correct.The chained configuration for adding
JsonStringEnumConverteris clean and follows .NET conventions.
87-99: LGTM: WebSockets and Serilog request logging are well-configured.The WebSocket keep-alive interval and Serilog request logging configuration are correct:
- Proper null-safe path checking on Line 96
- Clear log level prioritization logic
- Direct header access is appropriate for .NET 10
101-106: LGTM: Swagger configuration is clear and explicit.The explicit Swagger endpoint and route prefix configuration improves maintainability.
108-109: LGTM: Static asset and controller mapping updated for .NET 10.
MapStaticAssets()is a .NET 9+ feature that provides improved static asset handling with fingerprinting and caching, which is appropriate for this .NET 10 upgrade.
71-78: Code is correct—all services properly implement IHostedService.All five services (
DeviceTracker,DeviceSettingsStore,NodeSettingsStore,NodeTelemetryStore,TelemetryService) inherit fromBackgroundService, which implementsIHostedService. The hosted service registration pattern in Program.cs is valid.
41-51: No action needed—the null-forgiving operator is safe.Given that
storageDiris constructed asPath.Combine(configDir, ".storage")whereconfigDiris either an environment variable or a user profile subdirectory, anddbPathisPath.Combine(storageDir, "history.db"), the resulting path will always be a full path with directory components (e.g.,/home/user/.espresense/.storage/history.db). Path.GetDirectoryName returns null only for root paths, empty strings, or null input—none of which apply here. The null-forgiving operator on line 45 is justified and safe.
DTTerastar
force-pushed
the
codex/upgrade-to-.net-10
branch
from
9d3246c to
26749fc
Compare
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
force-pushed
the
codex/upgrade-to-.net-10
branch
from
26749fc to
f3865e0
Compare
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
force-pushed
the
codex/upgrade-to-.net-10
branch
from
dcd9bd5 to
280f9ab
Compare
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
force-pushed
the
codex/upgrade-to-.net-10
branch
from
cc64e36 to
5865b4c
Compare
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci.yml(1 hunks)BUILDING.md(1 hunks)CLAUDE.md(2 hunks)Dockerfile(2 hunks)src/ESPresense.Companion.csproj(3 hunks)src/Program.cs(4 hunks)tests/ESPresense.Companion.Tests.csproj(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Dockerfile
- BUILDING.md
- tests/ESPresense.Companion.Tests.csproj
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Place backend C# ASP.NET Core code under src/ (controllers, services, models, utils)
Files:
src/Program.cssrc/ESPresense.Companion.csproj
{src,tests}/**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
{src,tests}/**/*.cs: C#: Use spaces with an indent size of 4
C#: Use PascalCase for types and methods
C#: Use camelCase for local variables and parameters
Files:
src/Program.cs
🧠 Learnings (1)
📚 Learning: 2025-09-06T14:09:17.125Z
Learnt from: CR
Repo: ESPresense/ESPresense-companion PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-06T14:09:17.125Z
Learning: Run backend and frontend build/test commands as specified (dotnet build/test; pnpm scripts) during development
Applied to files:
.github/workflows/ci.ymlCLAUDE.md
🧬 Code graph analysis (1)
src/Program.cs (1)
src/Services/MqttCoordinator.cs (2)
MqttCoordinator(29-758)MqttCoordinator(48-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Deploy to Docker add-on with tag
- GitHub Check: build
- GitHub Check: Analyze (csharp)
🔇 Additional comments (14)
.github/workflows/ci.yml (1)
22-25: LGTM!The .NET version update to 10.0.x is correct and aligns with the broader framework upgrade across the repository.
src/ESPresense.Companion.csproj (3)
4-4: LGTM!The target framework upgrade to net10.0 is the foundation of this PR and is correctly implemented.
14-14: LGTM!Explicitly setting the SPA proxy working directory improves configuration clarity.
27-27: No action needed. Microsoft.AspNetCore.SpaProxy version 10.0.1 is available on NuGet and targets .NET 10.0, making it compatible with your project.CLAUDE.md (2)
9-9: LGTM!The backend framework documentation correctly reflects the upgrade to .NET 10.0.
62-66: LGTM!The development setup instructions are correctly updated to reflect the .NET 10.0 requirement and include helpful browsing guidance.
src/Program.cs (8)
26-26: LGTM!The explicit parameter naming improves code readability without changing functionality.
30-33: LGTM!The config directory setup is correctly implemented and the formatting improves readability.
36-37: LGTM!The ConfigLoader registration pattern correctly uses the captured variable and registers it as both a singleton and hosted service.
39-40: LGTM!Persisting data protection keys to the filesystem is an important improvement that ensures encrypted data remains accessible across application restarts.
42-52: LGTM!The SQLite connection setup improvements enhance code clarity and ensure the database directory exists before initialization.
61-75: LGTM!The DI registrations follow established patterns and correctly wire up the new singleton services and interface mappings.
79-83: LGTM!The hosted service registration pattern correctly resolves existing singleton instances, ensuring the same instance is used for both direct injection and background service execution.
104-129: LGTM!The updates to WebSocket options, request logging, Swagger UI configuration, and static asset mapping correctly align with .NET 10 conventions and best practices. The use of MapStaticAssets is the recommended approach for modern ASP.NET Core applications.
| <PackageReference Include="MQTTnet" Version="5.0.1.1416" /> | ||
| <PackageReference Include="MQTTnet.Extensions.ManagedClient" Version="4.3.7.1207" /> |
There was a problem hiding this comment.
Fix incompatible MQTTnet package versions.
MQTTnet v5.0 removed the managed client as a breaking change, and a version 5.x-compatible MQTTnet.Extensions.ManagedClient is not planned. The current combination (MQTTnet 5.0.1.1416 with MQTTnet.Extensions.ManagedClient 4.3.7.1207) will fail at runtime. Either downgrade MQTTnet to v4.3.7 or higher (< 5.0), or remove the Extensions.ManagedClient dependency and handle reconnection logic manually.
🤖 Prompt for AI Agents
In src/ESPresense.Companion.csproj around lines 29-30, the project references
incompatible MQTTnet v5.0.1.1416 together with MQTTnet.Extensions.ManagedClient
v4.3.7.1207 which will fail at runtime; fix by either pinning MQTTnet to a 4.x
release that matches the managed client (e.g., change the MQTTnet
PackageReference Version to 4.3.7.1207 or any <5.0 compatible 4.x) or remove the
MQTTnet.Extensions.ManagedClient PackageReference and update the code to use
MQTTnet v5 APIs with your own reconnection/managed-client logic; apply one of
these two changes so package versions are compatible and restore/build.
4665800 to
5865b4c
Compare
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
Summary
Testing
Codex Task
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.