Add .NET 4.7.2 support and reliable WPF injection packaging - #22
Add .NET 4.7.2 support and reliable WPF injection packaging#22miloszkukla wants to merge 36 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands framework compatibility and hardens distribution for auto-injection by multi-targeting the Inspector/Shared contracts down to .NET Framework 4.7.2, packaging the full injection payload (native bootstrappers + managed dependency closure), adding validation during publish/pack, and introducing integration coverage across frameworks/architectures/modes.
Changes:
- Multi-target
WpfVisualTreeMcp.InspectorandWpfVisualTreeMcp.Sharedfornet472/net48/net8.0(-windows)and add .NET Framework assembly-resolution support for co-located private dependencies. - Make the Server own and validate the complete auto-injection payload during
Publish/Pack, and adjust payload layout (including CoreCLR runtimeconfig naming). - Add Shared and end-to-end integration test projects + a PowerShell runner, and update docs/workflows to build/verify payloads and run the inspection matrix.
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| WpfVisualTreeMcp.sln | Adds new Shared and Integration test projects to the solution. |
| tests/WpfVisualTreeMcp.Tests/ProcessInjectorTests.cs | Updates injector unit test setup to avoid relying on Inspector path resolution for invalid PID case. |
| tests/WpfVisualTreeMcp.Shared.Tests/WpfVisualTreeMcp.Shared.Tests.csproj | New multi-targeted Shared contract test project. |
| tests/WpfVisualTreeMcp.Shared.Tests/SharedModelsTests.cs | Adds basic model construction/shape tests for Shared DTOs. |
| tests/WpfVisualTreeMcp.Shared.Tests/IpcSerializerTests.cs | Adds regression coverage ensuring derived IPC request properties serialize/round-trip correctly. |
| tests/WpfVisualTreeMcp.IntegrationTests/WpfVisualTreeMcp.IntegrationTests.csproj | New net8.0-windows integration test project for the live WPF matrix. |
| tests/WpfVisualTreeMcp.IntegrationTests/IntegrationTheoryAttribute.cs | Adds an opt-in theory attribute that skips unless integration env vars are set. |
| tests/WpfVisualTreeMcp.IntegrationTests/InspectionModeMatrixTests.cs | Adds 12-case framework/arch/mode CLI-driven inspection matrix tests. |
| tests/WpfVisualTreeMcp.IntegrationTests/AssemblyInfo.cs | Disables test parallelization to reduce flakiness for process-level integration tests. |
| tests/run-integration-tests.ps1 | Adds a local runner to build native payload + publish samples and execute the live matrix. |
| src/WpfVisualTreeMcp.Shared/WpfVisualTreeMcp.Shared.csproj | Adds net472 target and System.Text.Json package reference for .NET Framework TFMs. |
| src/WpfVisualTreeMcp.Server/WpfVisualTreeMcp.Server.csproj | Packages/copies the full injection payload and validates required artifacts during publish/pack. |
| src/WpfVisualTreeMcp.Server/Services/ProcessManager.cs | Adds fast named-pipe probe to detect an already-running Inspector and standardizes status messaging. |
| src/WpfVisualTreeMcp.Inspector/WpfVisualTreeMcp.Inspector.csproj | Adds net472 target and updates explicit WPF refs for .NET Framework builds. |
| src/WpfVisualTreeMcp.Inspector/InspectorService.cs | Adds .NET Framework AssemblyResolve hook for co-located private dependency resolution. |
| src/WpfVisualTreeMcp.InjectorHelper/WpfVisualTreeMcp.InjectorHelper.csproj | Removes an obsolete warning-suppression comment and keeps helper configuration. |
| src/WpfVisualTreeMcp.Injector/WpfVisualTreeMcp.Injector.csproj | Simplifies Injector to net8.0 only and removes Inspector project reference. |
| src/WpfVisualTreeMcp.Injector/README.md | Rewrites injector docs to reflect real injection flow, requirements, and diagnostics. |
| src/WpfVisualTreeMcp.Injector/ProcessInjector.cs | Updates Inspector DLL path probing to support packaged layouts. |
| samples/SampleWpfApp/SelfHostedInspector.cs | Adds a helper wrapper for starting/stopping the Inspector in self-hosted mode. |
| samples/SampleWpfApp/SampleWpfApp.csproj | Multi-targets the sample app for net472/net48/net8.0-windows and adds explicit platforms. |
| samples/SampleWpfApp/App.xaml.cs | Makes self-hosting conditional via env var and routes start/stop through helper. |
| README.md | Documents auto-injection mode, requirements, diagnostics, multi-target support, and updated build/test commands. |
| docs/TOOLS_REFERENCE.md | Clarifies scope of the reference and points to README/CLI for the full tool list. |
| docs/GETTING_STARTED.md | Updates installation and framework support guidance, including sample run command. |
| docs/ARCHITECTURE.md | Updates architecture notes for multi-runtime Inspector and injection behavior/security model. |
| CLAUDE.md | Updates local build/test guidance and adds the integration matrix runner command. |
| CHANGELOG.md | Adds unreleased notes covering payload packaging fixes and runtimeconfig naming. |
| .github/workflows/release.yml | Builds native bootstrappers and verifies publish + NuGet payload contents during release. |
| .github/workflows/publish-mcp-registry.yml | Updates checkout action major version. |
| .github/workflows/build.yml | Adds native bootstrapper build, payload verification, x86 runtime install, and integration matrix execution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/release.yml:44
- The release job still publishes without running the advertised 12-case live matrix.
dotnet testskipsIntegrationTheoryunless the two integration environment variables are prepared, and tag pushes do not match the branch-only triggers inbuild.yml, so a tagged commit can release untested injection payloads. Install the x86 runtimes and invoke the integration runner in this workflow before creating the release publish.
- name: Publish MCP Server from clean checkout
run: dotnet publish src/WpfVisualTreeMcp.Server/WpfVisualTreeMcp.Server.csproj --no-restore --configuration Release --output ./publish/server
7d78e21 to
49093c9
Compare
|
Follow-up on the remaining status-provenance issue from the independent review: fixed in When the Inspector module was already loaded but its named pipe never becomes ready, attachment now reports I also assessed Copilot's latest suppressed suggestion to rerun the full 12-case live matrix in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/WpfVisualTreeMcp.IntegrationTests/InspectionModeMatrixTests.cs:83
- The AutoInjection cases launch the same SampleWpfApp artifact used for self-hosting, but that project always references Inspector, so its output already contains Inspector, Shared, and the .NET Framework dependency closure. The target can therefore satisfy injected dependencies from its own application directory, allowing this matrix to pass even if the packaged payload or private resolver is broken. Run auto-injection against a sample built without that reference/dependency closure (or a separate clean target artifact) so the packaging guarantee is exercised end to end.
src/WpfVisualTreeMcp.Inspector/InspectorService.cs:269 - When the new cooperative deadline expires,
CaptureFullContentthrowsTimeoutException, but the existingHandleRequestAsynccatch converts everyTimeoutExceptionintoRequest timeout: UI thread is busy. A capture that actively runs until its 25-second budget is therefore reported as a dispatcher problem. Preserve a distinct full-content deadline error (for example, by detecting this token's cancellation separately) and reserve the busy message for the Dispatcher scheduling timeout.
using var fullContentCaptureCts = requestType == "CaptureScreenshot"
? new CancellationTokenSource(FullContentCaptureTimeout)
: null;
var fullContentCaptureToken = fullContentCaptureCts?.Token ?? CancellationToken.None;
|
Addressed both suppressed findings from the latest Copilot review in
Validation at exact head
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/WpfVisualTreeMcp.Inspector/ScreenshotCapture.cs:521
- The overlap scan is not cancellation-aware even though it can compare the full viewport up to five times. If the 25-second token expires during these tile loops, the UI thread keeps processing
CopyPixels/byte comparisons until the entire scan returns, so a full-content request can substantially exceed its advertised execution deadline. Pass the capture token throughFindVerticalOverlap/VerticalRegionsEqualand check it inside the tile loops.
y += previous.PixelHeight - FindVerticalOverlap(
previous, bitmap, expectedOverlap, 2);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/build.yml:60
- The PR describes run 31855379392 as an exact-head validation, but that run executed commit
61cf29f; the current PR head is08a0b91, which subsequently changed screenshot overlap cancellation and its tests, and the current head has no check runs. Re-run this matrix for the current head (and update the Validation section) before treating the listed CI results as coverage of these changes.
- name: Test WPF inspection matrix
shell: pwsh
run: ./tests/run-integration-tests.ps1 -SkipNativeBuild
|
Addressed the latest suppressed validation finding. The full fork workflow was rerun at the current PR head The PR Validation section now references that exact-head run. It includes the 12-case WPF integration matrix and the complete managed suite ( |
There was a problem hiding this comment.
🟡 Changes recommended
The dependency-resolution scope leaks into the persistent IPC task through AsyncLocal execution-context flow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 49/50 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Follow-up validation at exact head a9bb029: fork workflow 32844813301 passed all jobs, including clean publish, both native bootstrapper architectures, 138 managed tests with 1 expected ordinary-run matrix skip, the live 12/12 WPF matrix, both .NET Framework target builds, payload verification, NuGet pack, and artifact upload. The valid dependency-resolver context-flow finding was fixed and replied to inline; the thread is resolved. |
There was a problem hiding this comment.
🔵 Needs a closer look
Despite comprehensive passing validation, it changes native injection, in-process dependency resolution, packaging, and UI-thread capture behavior that warrant final human review.
Review details
- Files reviewed: 50/51 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
net8.0net472,net48, andnet8.0-windows, x86/x64, and both inspection modeswpf-visual-tree-cliagent skill and makeAGENTS.mddelegate repository guidance toCLAUDE.mdWhy
The v0.12.0 release artifacts omit
WpfInspectorBootstrapper.dlland the managed Inspector dependency closure, so auto-injection cannot work from the official ZIP or NuGet package. The Inspector also did not target .NET Framework 4.7.2 even though its WPF implementation is compatible with that runtime.On .NET Framework, managed-module enumeration does not reliably reveal the loaded Inspector. A second
attach --auto-injectcould therefore attempt injection again. The Inspector's named pipe is the authoritative readiness signal and now provides a short pre-injection check, while the longer retry remains reserved for post-injection startup.Review feedback incorporated
usingLoaded (existing)when the Inspector was already present, including the pipe-timeout pathInjector remains on
net8.0: it no longer references the Windows-targeted Inspector assembly, and changing it tonet8.0-windowswould also force a Server/tool TFM change. Missing Inspector payloads already produceFileNotFoundExceptioninInjectIntoProcess.Validation
a9bb029, including clean publish, native and managed builds, the full test suite, the 12-case WPF matrix, payload verification, NuGet pack, code quality, and artifact uploaddotnet build WpfVisualTreeMcp.sln -c Release --no-incremental --no-restore: passed with no errors (existing nullable warnings only)dotnet publishafter removing all managed outputs: 35 native payload files, including both .NET Framework closures, InjectorHelper payloads, and both CoreCLR Inspector payloadsdotnet pack: 94 archive entries with all required injection payload entries presentattach --auto-injectusesLoaded (existing)and the target remains inspectablequick_validate.pyand agrees with the documented 28 tools/commands and target frameworks