Maintenance pass 2026-07: dependency bumps, zero-warning build, CI + hygiene (#1643) - #1645
Merged
Merged
Conversation
…hygiene (#1643) Dependencies (all patch/minor; zero vulnerable and zero deprecated across all 20 projects, verified with --vulnerable --include-transitive): Microsoft.Data.SqlClient 7.0.1 -> 7.0.2 Microsoft.Data.SqlClient.Extensions.Azure 1.0.0 -> 7.0.2 Microsoft.Extensions.* 10.0.9 -> 10.0.10 System.Security.Cryptography.ProtectedData 10.0.0 -> 10.0.10 ModelContextProtocol(+AspNetCore) 1.4.0 -> 1.4.1 ScottPlot.WPF 5.1.58 -> 5.1.59 Microsoft.NET.Test.Sdk 18.6.0 -> 18.8.1 DuckDB.NET (Bindings.Full + Data) 1.5.2 -> 1.5.3 Extensions.Azure 1.0.0 -> 7.0.2 is not a real major: Microsoft re-versioned the companion package onto the SqlClient 7.x line. DuckDB 1.5.3 was validated by running tools/CompactionRepro --synthetic (the real ParquetCompaction merge code) against the new engine: SUCCESS, 30k rows exact, peak WS 364 MB, no spill. Lock files regenerated with --force-evaluate. Zero-warning build restored (~280 accumulated warnings -> 0 across the whole solution). The dominant one was CS8629 at ~100 MCP tool call sites in all three apps: ServerResolver.ResolveOrError returned a nullable tuple, so the "error is null implies resolved is non-null" invariant was invisible to the compiler and every call site dereferenced .Value. All three resolvers now return a non-nullable resolved that is default when error is set, and the call sites drop .Value. Also: CA1834 single-char Append via char alias consts, CA1859 concrete return types (including the four parallel BrushFromHex/MakeFrozen copies), CA1863 cached CompositeFormats, CA1416 platform-guard restructuring so the DPAPI and Event Log calls are provably Windows-only, CA1310/CA2249/CA1865 ordinal comparisons, CA1716 Get -> Find, CS0108 redundant HasReportXml, a CS8602 null guard in the Dashboard actual-plan path, and xUnit assertion forms. Two documented suppressions rather than silences: CA1720 on the collector type enums (naming SQL types is their purpose) and xUnit1051 in the test projects. CI ran neither the build nor the tests for the deprecated Dashboard: #1612 defined a 'dashboard' path filter but no step ever consumed it. That is why the warnings above accumulated unseen, and why all three ThemeParityTests had been hard-failing since the move (FindRepoRoot still looked for Dashboard\Themes instead of deprecated\Dashboard\Themes, so each run died on the repo-root assert without comparing anything). The test now resolves the deprecated path from one constant and genuinely compares the palettes again, and the workflow gains the missing restore/build/test steps. Dashboard.Tests gets a packages.lock.json so it restores --locked-mode like every other CI root. This matters beyond the deprecated app: the palettes that guard checks are Lite's too. Test results: Lite 1444/1444, Darling 2995/2995 (147 gated-live skipped), Installer 190/190 (CI filter), Dashboard 768/768. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The entry asserted the Actions pins were verified current. They are not: checkout v5 (latest v7), setup-dotnet v5 (v6), cache v4 (v6), upload-artifact v6 (v7), paths-filter v3 (v4). Only the SignPath action is current at v2. Verified against the releases API, not a search summary. They stay unbumped here on purpose: these are major bumps and upload-artifact feeds artifact ids directly into the SignPath signing steps, so the blast radius reaches the release path. Tracked separately per the maintenance rule that majors and release-critical changes get their own effort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 25, 2026
erikdarlingdata
added a commit
that referenced
this pull request
Jul 25, 2026
…ntended-lockfiles Remove six lock files added unintentionally by #1645
argpna
pushed a commit
to argpna/PerformanceMonitor
that referenced
this pull request
Jul 26, 2026
erikdarlingdata#1645 took the repo from 8 packages.lock.json files to 15. Only one of those seven was deliberate: deprecated/Dashboard.Tests needed a lock file to become a CI --locked-mode restore root. The other six -- Alerting, Analysis, Common, Notifications, PlanAnalysis, Ui -- were collateral and went unmentioned in that PR's description. Cause, confirmed by experiment rather than assumption: `--use-lock-file` propagates transitively across ProjectReferences. Running it once against Dashboard.Tests generated lock files for every project Dashboard references, which is exactly those six. `--force-evaluate` was innocent -- re-running `dotnet restore PerformanceMonitor.sln --force-evaluate` after deleting them does NOT bring them back, so this revert is stable and the next maintenance pass will not silently re-add them. Reverting rather than keeping them, because with CI restoring --locked-mode every lock file has to move in step on a dependency change or restore fails NU1004, and Dependabot is about to start opening weekly grouped NuGet PRs against all of them. Fewer lock files, confined to the projects CI actually restores, is the convention this repo already had. Verified: all six CI --locked-mode restore roots (Lite, Lite.Tests, Installer.Tests, Dashboard.Tests, Darling.Tests, Darling.Viewer) restore clean with zero errors and regenerate nothing; full solution builds 0 warnings, 0 errors. No CHANGELOG entry: nothing user-facing changed, this never shipped, and erikdarlingdata#1645's "lock files regenerated with --force-evaluate" wording stays true. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First formal
/maintenancepass on this repo. Closes #1643.Dependencies
Zero vulnerable (
--vulnerable --include-transitive) and zero deprecated packages across all 20 projects.tools/CompactionReprois not in the solution, so it was scanned separately.All patch/minor. The Extensions.Azure
1.0.0 -> 7.0.2jump is not a real major — Microsoft re-versioned the companion package onto the SqlClient 7.x line (verified on nuget.org). Per the engine-wrapped-bindings rule, DuckDB 1.5.3 was validated by runningtools/CompactionRepro --synthetic— the actual productionParquetCompactionmerge code, not a reimplementation — against the new engine: SUCCESS, 30,000 rows exact, peak working set 364 MB, no spill. Lock files regenerated with--force-evaluate.Zero-warning build restored
The solution built with ~280 warnings; it now builds with 0 warnings, 0 errors.
The dominant cluster was CS8629 at ~100 MCP tool call sites across all three apps.
ServerResolver.ResolveOrErrorreturned a nullable tuple, so the "error == nullimpliesresolved != null" invariant was invisible to the compiler and every call site had to dereference.Value. All three resolvers (Dashboard, Lite, Darling) now return a non-nullableresolvedthat isdefaultwhenerroris set, and the call sites drop.Value— the guarantee is in the signature instead of in a comment.The rest, fixed honestly rather than suppressed: CA1834 (single-char
Appendviacharalias consts), CA1859 (concrete return types on private helpers, including the four parallelBrushFromHex/MakeFrozencopies), CA1863 (cachedCompositeFormatin the two template-formatting collectors), CA1416 (platform-guard restructuring so the DPAPI and Event Log calls are provably Windows-only), CA1310/CA2249/CA1865 (ordinal comparisons), CA1716 (IViewerServerSecretStore.Get->Find), CS0108 (redundantHasReportXmlre-declaration), CS8602 (a real latent null-deref guard in the Dashboard actual-plan path), and xUnit assertion modernizations.Two suppressions, both documented in place: CA1720 on
CollectorColumnType/CollectorParameterType(naming SQL types is exactly their job) and xUnit1051 in the test projects (the ambient-CancellationTokenstyle nag, suppressed alongside the existing CS4014).CI was not running the deprecated Dashboard at all
#1612 defined a
dashboardpath filter when the Full Dashboard moved underdeprecated/, but no workflow step ever consumed it — so Dashboard.Tests stopped building and running in CI entirely. The gap was self-concealing, and it explains both findings above it:ThemeParityTestshad been hard-failing since the move:FindRepoRootstill looked forDashboard\Themes, which is nowdeprecated\Dashboard\Themes, so every run died on the repo-root assert without ever comparing a palette.Both fixed. The test resolves the deprecated path from a single constant and genuinely compares again (768/768, no drift found), and the workflow gains the missing restore/build/test steps gated on the existing filter. Dashboard.Tests gets a
packages.lock.jsonso it restores--locked-modelike every other CI root.Worth stating plainly: this guard is not Dashboard-only in effect. The palettes it checks are Lite's too, so what had silently stopped running was Lite's cross-app theme-drift protection.
Test plan
All four suites run locally against the bumped dependencies:
DARLING_TEST_PG)ThemeParityTestsfix)Plus: full-solution
--no-incrementalbuild at 0 warnings, and the DuckDB compaction repro above.Also in this pass
Repo hygiene: deleted 27 dead
worktree-agent-*branches (all pointing at one stalemainSHA with no unique work), a closed-PR checkout (pr-963), and two merged feature branches; pruned the fork remote..gitattributesalready present and correct.Verified current, no change needed: the
vpkCLI pin matching the VelopackPackageReference, zeroTODO/FIXME/HACKmarkers, and .NET 10 as a supported released TFM.GitHub Actions are behind and deliberately NOT bumped here. An earlier revision of this description claimed they were current; that was wrong, and the correction is committed. Checked against the releases API:
These are major bumps, and
upload-artifactfeedsartifact-idoutputs straight into the six SignPath signing steps — with v7 introducing non-zipped artifacts, that blast radius reaches the release-signing path. Per the maintenance rule that majors and release-critical changes get their own effort, they are tracked separately rather than riding along with a dependency PR.Findings from the audit half of this pass (security review of
v3.1.0..dev, retention/archival coverage, bundled community procs) are filed separately — they are behavior changes and do not belong in a dependency PR.🤖 Generated with Claude Code