satellite-provider/_docs/03_implementation/reviews/batch_01_cycle4_review.md

Code Review Report

Batch: 1 (cycle 4) — AZ-500_dotnet10_migration Date: 2026-05-12 Verdict: PASS_WITH_WARNINGS

Findings

#	Severity	Category	File:Line	Title
1	Medium	Maintainability	SatelliteProvider.Api/Program.cs:174–219	WithOpenApi(...) extension is deprecated in ASP.NET Core 10 (ASPDEPR002) — slated for removal
2	Medium	Maintainability	SatelliteProvider.Api/Swagger/ParameterDescriptionFilter.cs:25	CS8604 — parameter.Name is nullable in Microsoft.OpenApi 2.x; missing null guard
3	Low	Scope	scripts/run-performance-tests.sh:49	Fix to bin/Release/netX.0/ path was implicit in AC-5 but not explicitly listed in task spec's Scope/Included

Finding Details

F1: WithOpenApi(...) extension is deprecated in ASP.NET Core 10 (ASPDEPR002) (Medium / Maintainability)

• Location: SatelliteProvider.Api/Program.cs:174,178,182,187,199,207,211,219 (8 endpoint mappings)
• Description: OpenApiEndpointConventionBuilderExtensions.WithOpenApi<TBuilder>(TBuilder, Func<OpenApiOperation, OpenApiOperation>) is marked obsolete with diagnostic ID ASPDEPR002 ("WithOpenApi is deprecated and will be removed in a future release"). The build emits 8 deprecation warnings — one per endpoint that calls .WithOpenApi(op => new(op) { Summary = ... }). The migration intentionally kept the existing surface to minimise behavioural risk (Risk #2 in AZ-500 task spec), but this is a known follow-up. See https://aka.ms/aspnet/deprecate/002 for the recommended replacement (.WithSummary(...) / .WithDescription(...) / .WithName(...) minimal-API metadata extensions).
• Suggestion: Open a follow-up PBI to migrate the 8 callsites to the new minimal-API metadata extensions (WithSummary, WithDescription, WithName, WithTags). Out of AZ-500 scope per coderule.mdc "scope discipline" — AZ-500's contract is "preserve behaviour during runtime/SDK migration, do not adopt new APIs."
• Task: AZ-500

F2: CS8604 — parameter.Name is nullable in Microsoft.OpenApi 2.x (Medium / Maintainability)

• Location: SatelliteProvider.Api/Swagger/ParameterDescriptionFilter.cs:25

  if (parameterDescriptions.TryGetValue(parameter.Name, out var description))
  

• Description: Microsoft.OpenApi 2.x added stricter nullable annotations — OpenApiParameter.Name is now declared string?, while Dictionary<string, string>.TryGetValue(string key, ...) requires non-null. The build emits one CS8604 warning. At runtime any parameter without a Name would NRE here, although in practice every Swashbuckle-emitted parameter has a non-null Name. Trivial defensive fix.
• Suggestion (one-line, can land in this batch if user prefers):

  if (parameter.Name is { } name && parameterDescriptions.TryGetValue(name, out var description))
  

  Otherwise defer to a follow-up nullable-cleanup PBI.
• Task: AZ-500

F3: scripts/run-performance-tests.sh:49 path fix was implicit, not explicitly listed in task scope (Low / Scope)

• Location: scripts/run-performance-tests.sh:49 — PERF_DLL=".../bin/Release/net8.0/..." → .../bin/Release/net10.0/...
• Description: AZ-500's Scope/Included lists scripts/run-tests.sh and .woodpecker/01-test.yml for image bumps but does NOT explicitly mention scripts/run-performance-tests.sh. However, AC-5 requires the perf-bootstrap to succeed; without this path fix, dotnet "$PERF_DLL" would always fail (file not found at the old path). This is necessary scope creep, not opportunistic — AC-5 cannot pass without it. Recorded as Low/Scope rather than Spec-Gap because the fix is required by AC-5's letter.
• Suggestion: Accept as in-scope. No code change needed — this finding is documentary.
• Task: AZ-500

Phase Summary

Phase	Result	Notes
1. Context Loading	OK	Task spec read; 21 changed files mapped to AZ-500
2. Spec Compliance	PASS	All 8 ACs satisfied (verified by grep + build + test run + manual smoke)
3. Code Quality	PASS_WITH_WARNINGS	F1 + F2 flagged; otherwise pure version-bump diff
4. Security Quick-Scan	PASS	No new code paths; JWT validation contract preserved (AC-6 covers AZ-487/AZ-494 regression)
5. Performance Scan	PASS	No new logic; .NET 10 ASP.NET pipeline expected to improve, not regress (gated at Step 15)
6. Cross-Task Consistency	N/A	Single-task batch
7. Architecture Compliance	PASS	No layer-direction violations; no new cross-component imports; using Microsoft.OpenApi; is external

AC Verification Trace

AC	Verification	Result
AC-1 (every csproj net10.0)	grep -r "<TargetFramework>" --include="*.csproj" → 9/9 net10.0, 0 net8.0	PASS
AC-2 (global.json sdk.version=10.0.0)	cat global.json → "version": "10.0.0", "rollForward": "latestMinor"	PASS
AC-3 (all docker images :10.0)	grep -rE "mcr.microsoft.com/dotnet/" --include="*Dockerfile" --include="*.yml" --include="*.sh" → 7/7 references on :10.0	PASS
AC-4 (M.AspNetCore.* / M.Extensions.* on 10.0.7)	grep over csprojs → 19 references on 10.0.7; Serilog.AspNetCore at 8.0.3 (documented per Risk #4 in AGENTS.md:244)	PASS
AC-5 (perf-bootstrap succeeds, leftover updated)	PERF_REPEAT_COUNT=2 PERF_UAV_BATCH_SIZE=2 ./scripts/run-performance-tests.sh → exit code 1 (NOT 3 — bootstrap succeeded, build OK, JWT mint OK, PT-01..PT-07 PASS, PT-08 crashed on pre-existing grep-pipefail bug). Leftover file updated with non-SDK failure detail. Per AZ-500 Constraints last bullet, leftover stays in place — closed by Step 15 of cycle 4 after the script grep fix lands.	PASS
AC-6 (./scripts/run-tests.sh --full green)	Step 0 dotnet format → PASS; Step 1 unit tests → 271/271 passed in 4.2s; Step 2 integration tests → docker-compose with --abort-on-container-exit --exit-code-from integration-tests exited 0; visible PASSED markers cover JWT/UAV/Tile/Region/Route + 4 complex/extended route scenarios	PASS
AC-7 (docker-compose build succeeds, no downgrade warnings)	Both run-tests.sh Step 2 build AND a separate docker compose up -d --build succeeded; only warnings emitted are F1 (ASPDEPR002) + F2 (CS8604) — neither is "package downgrade", "framework conflict", or "missing base image"	PASS
AC-8 (docs reflect .NET 10)	_docs/02_document/architecture.md:5 ".NET 10", line 67 Tech Stack table "10.0"; AGENTS.md:9 ".NET 10", lines 240–244 packages list updated with Serilog fallback note	PASS

Auto-Fix Eligibility

Per implement/SKILL.md Step 10 matrix:

• F1 (Medium / Maintainability) → eligible, but deferred — fix would expand scope beyond AZ-500's "preserve behaviour" contract; needs a follow-up PBI to migrate 8 endpoints to the new metadata API. Surfaced for user decision.
• F2 (Medium / Maintainability) → eligible auto-fix. Trivial 1-line null guard. User can opt to fold into this batch or defer to a nullable-cleanup PBI.
• F3 (Low / Scope) → no fix needed, documentary only.

Verdict Logic

• 0 Critical, 0 High → no FAIL gate triggered.
• 2 Medium + 1 Low → PASS_WITH_WARNINGS.

Recommended Follow-Up PBIs (out of AZ-500 scope)

1. scripts/run-performance-tests.sh:416-417 grep-pipefail fix (1 point) — replace grep -o ... | wc -l with grep -c ... || true; unblocks Step 15 perf full-run + closes the perf-cycle3 leftover. See _docs/_process_leftovers/2026-05-12_perf-cycle3-harness-execution.md for the trace.
2. Migrate WithOpenApi(...) callsites to ASP.NET Core 10 minimal-API metadata extensions (3 points) — 8 callsites in Program.cs; covers F1.
3. Microsoft.OpenApi 2.x nullable cleanup (1 point) — covers F2 plus any other nullable annotations exposed by the migration (none others detected in this batch, but a project-wide grep is recommended).