Reorganize EventLogExpert.Eventing folders, tighten API surface, and split unit/integration tests

[jschick04]

Stacked on #515 — base is jschick/move-modals-to-components because PR-Eventing is the first slice of the 5-PR library reorganization series and #515's branch is the natural starting tree.

Summary

PR1 of a 5-PR series (Eventing → UI → EventDbTool → Components → MAUI head). EventLogExpert.Eventing folder reorg, API tightening, and test project split. 138 files changed (+2,626 / −2,019). Same baseline test counts preserved end-to-end (1,872 passed + 1 env-skip).

Why now

EventLogExpert.Eventing had grown a Helpers/ catch-all folder mixing P/Invoke surface, enums, model types, and dead extension methods, with public-by-default visibility on types only consumed inside the assembly. The Tests project's Helpers/ similarly conflated unit-style tests with integration tests that touched real Windows event-log infrastructure (so unit runs were slower than they needed to be and CI signal was muddier when an environment-dependent test failed). This PR resolves both — folder structure now mirrors logical concerns (Interop / Models / Providers / Readers), public surface is reduced to genuine consumers, and integration tests live in a separate project so unit suites stay fast and environment failures stay localized.

Changes

19 commits, organized bottom-up so each is independently reviewable and revertable. Producers move first, consumers track.

Folder reorganization (compile-preserving)

1. Move P/Invoke types to Interop/ and rename Interop class to Win32ErrorCodes for clarity.
2. Move and rename internal interop classes (NativeMethods partials) to Interop/.
3. Split Helpers/EventMethods.cs into Interop partials and extract LoadLibraryFlags enum to its own file.
4. Move PathType enum to Readers/ namespace alongside its consumers.
5. Delete dead Helpers.ExtensionMethods extension (zero call sites in any assembly).
6. Move SeverityLevel and Severity to Models/ namespace.
7. Move remaining Helpers types to Logging/ and Readers/, draining the Helpers/ folder completely.

API tightening

8. Internalize EventLogExpert.Eventing surface and seal EventXmlResolver. Per-symbol consumer-set verification; types and members with no cross-asm prod consumers demoted to internal. RegistryProvider and CompressedJsonValueConverter<T> become internal sealed. JsonValueConverter<T> deleted (was unused). EventXmlResolver stays public sealed (registered via DI in MauiProgram.cs). EventMessageProvider.GetMessages static factory demoted to internal static (only same-asm LoadProviderDetailsCore plus friend-test consumers).
9. Migrate InternalsVisibleTo to csproj entries (replaces Properties/AssemblyInfo.cs per repo convention). Two friends granted: EventLogExpert.Eventing.Tests and EventLogExpert.Eventing.IntegrationTests.

Test project split + targeted improvements

10. Move Eventing.Tests Helpers/ tests into Interop/ and Readers/ mirroring the prod reorg.
11. Remove tautological and vacuous tests flagged by Step 9 audit catalog.
12. Replace EventLogWatcher test sleeps with signals and cancellable delays — eliminates Thread.Sleep flakiness, makes intent explicit, drops total test runtime.
13. Fix EventLogInformation invalid-handle check (SUT bug discovered during test rework — IsInvalid was being checked AFTER first use) and replace ThrowsAny<Exception> with specific assertions.
14. Replace conditional logic in test bodies with deterministic fixtures and explicit skips — removes if (count > 0) partial-skip patterns that masked failures.
15. Split Eventing tests into unit and integration projects — new EventLogExpert.Eventing.IntegrationTests project hosts the 232 tests that touch real Windows event-log infrastructure. Existing Eventing.Tests keeps the 389 fast unit tests. CI workflow updated to run both as separate steps for clearer failure signal.
16. Tighten RegistryProvider integration test probes — discovered SUT contract: helper treats inputs as PROVIDER names, not LOG names; Application and System are log names that don't have an EventMessageFile of their own. Tests now probe known-legacy provider names instead, with Assert.Skip for environment-optional cases (semicolon-split message files) and helpers (FindAnyLegacyProviderName, FindLegacyProviderWithSemicolonSplitFiles) for first-match registry scans.
17. Tighten thread-safety and vacuity in EventLogWatcherTests — Step 9 audit-catalog Categories C+D follow-through.
18. Strengthen resolver fallback tests and convert provider loop to Theory — [Theory] with InlineData per provider, plus new mixed-DB-and-unknown-providers test that asserts seeded DB text appears for the DB-known provider and "No matching" appears for the unknown one.
19. Isolate handler exceptions and reject reentrant stop in EventLogWatcher (SUT change + tests in one commit). Multicast EventRecordWritten now invokes each handler in its own try/catch so a throwing handler can't starve the rest. Dispose(disposing=true) and Enabled = false now throw InvalidOperationException if called from inside a callback (would otherwise deadlock); idempotent Dispose from another thread post-disposal still succeeds. Public XML doc publishes the new contract. Cross-asm LiveLogWatcherService already wraps its Dispose in Task.Run so the new throw path is unreachable from the only external consumer.

Verification

• dotnet build src/EventLogExpert.slnx — 0 warnings, 0 errors
• dotnet test src/EventLogExpert.slnx — 1,872 / 1,873 pass, 1 env-skip (Eventing.Tests 389 / Eventing.IntegrationTests 232 + 1 env-skip / EventDbTool.Tests 19 / Components.Tests 144 / UI.Tests 1,088). Same total + same env-skip as the parent branch — pure refactor, no test additions or deletions net of the integration split.
• dotnet format whitespace --verify-no-changes src/EventLogExpert.slnx — clean
• dotnet format style --verify-no-changes src/EventLogExpert.slnx --diagnostics IDE0005 IDE0065 — clean
• All file moves used git mv — history preserved (rename similarity ≥ 80% on every move).
• Per-commit micro-hygiene + branch-wide rename-first sweep + branch-wide 6-axis least-privilege re-audit ran per pre-pr-push.md. Audit produced 1 finding (EventMessageProvider.GetMessages public static → internal static); applied + amended into commit 7b29a46.
• Multi-model panel (Claude Opus xhigh + GPT-5.5 + GPT-5.3-codex + rubber-duck) ran on every commit; SUT changes (commits 13, 19) had a panel on the SUT design itself before the test rewrite.

Companion change (Azure DevOps release pipeline)

The release pipeline at https://github-private.visualstudio.com/microsoft/_git/EventLogExpert needs its build/test targets updated to mirror the new GH workflow split. Already pushed (commit a3344b7 on branch users/jschick/eventing-integration-test-split); ADO PR creation must happen through the ADO web UI. Both PRs need to land before this branch can ship through the release pipeline.

Stack note

This is PR1 of 5. Subsequent PRs (UI, EventDbTool, Components, MAUI head) are queued in files/survey-0.md but will be opened one at a time as each parent merges, to avoid compounding rebase pain.

[Copilot]

Pull request overview

Refactors EventLogExpert.Eventing into clearer functional folders (Interop/Models/Providers/Readers), reduces the public API surface to better match real consumers, and splits Eventing tests into separate unit vs. integration projects (with CI running them as distinct steps).

Changes:

• Moved/renamed interop + helper types (e.g., Interop → Win32ErrorCodes, EventMethods → NativeMethods) and relocated shared enums/models into targeted namespaces.
• Tightened API visibility (internalizing/sealing types like RegistryProvider, CompressedJsonValueConverter<T>, and parts of EventMessageProvider), and moved InternalsVisibleTo to the Eventing csproj.
• Split integration tests into EventLogExpert.Eventing.IntegrationTests and updated the GitHub Actions workflow to run unit vs. integration tests separately.

Reviewed changes

Copilot reviewed 135 out of 138 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/EventLogExpert/Services/MauiMenuActionService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/Services/ClipboardService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/MauiProgram.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/MainPage.xaml.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/Components/Sections/SplitLogTabPane.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/Components/Sections/EventTable.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/Components/Sections/DetailsPane.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/Components/Layout/UnhandledExceptionHandler.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/App.xaml.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert/_Imports.razor	Update razor imports to new Eventing namespaces
src/EventLogExpert.UI/Store/LoggingMiddleware.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/FilterCache/FilterCacheReducers.cs	Formatting-only adjustment in reducer bodies
src/EventLogExpert.UI/Store/EventTable/EventTableReducers.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/EventLog/LiveLogWatcherService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/EventLog/ILogReloadCoordinator.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/EventLog/EventLogReducers.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/EventLog/EventLogEffects.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Store/EventLog/EventLogAction.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Services/UpdateService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Services/GitHubService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Services/FilterCategoryItemsCache.cs	Update Eventing model import after folder/namespace reorg
src/EventLogExpert.UI/Services/DeploymentService.cs	Update Eventing namespace imports + minor formatting normalization
src/EventLogExpert.UI/Services/DebugLogService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Services/DatabaseService.cs	Update Eventing namespace imports + minor formatting normalization
src/EventLogExpert.UI/Services/BannerService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Services/ApplicationRestartService.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Models/EventTableModel.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/Models/EventLogData.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI/LogNameMethods.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/TestUtils/LoggerUtils.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/TestUtils/EventUtils.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/TestUtils/DatabaseSeedUtils.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Store/FilterPane/FilterPaneEffectsTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Store/EventTable/EventTableStoreTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Store/EventLog/EventLogStoreTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Store/EventLog/EventLogEffectsTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/UpdateServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/GitHubServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/FilterServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/FilterCategoryItemsCacheTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/DeploymentServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/DatabaseServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/Services/BannerServiceTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.UI.Tests/DateRangeDefaultsTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.slnx	Add new Eventing integration test project to the solution
src/EventLogExpert.Eventing/Readers/PathType.cs	New PathType enum in Readers namespace
src/EventLogExpert.Eventing/Readers/LogNames.cs	Move LogNames into Readers namespace
src/EventLogExpert.Eventing/Readers/EventLogWatcher.cs	Add per-subscriber exception isolation + callback-thread stop protection + interop renames
src/EventLogExpert.Eventing/Readers/EventLogSession.cs	Switch to NativeMethods and Win32ErrorCodes in interop layer
src/EventLogExpert.Eventing/Readers/EventLogReader.cs	Switch to NativeMethods and Win32ErrorCodes in interop layer
src/EventLogExpert.Eventing/Readers/EventLogInformation.cs	Fix invalid-handle error masking + add argument validation + interop renames
src/EventLogExpert.Eventing/Providers/RegistryProvider.cs	Internalize/seal provider + namespace updates
src/EventLogExpert.Eventing/Providers/ProviderMetadata.cs	Switch to NativeMethods/NativeErrorResolver + interop renames
src/EventLogExpert.Eventing/Providers/EventMessageProvider.cs	Internalize GetMessages + interop renames
src/EventLogExpert.Eventing/Properties/AssemblyInfo.cs	Remove AssemblyInfo-based InternalsVisibleTo (migrated to csproj)
src/EventLogExpert.Eventing/Models/SeverityLevel.cs	New SeverityLevel enum in Models namespace
src/EventLogExpert.Eventing/Models/Severity.cs	Move Severity into Models namespace (and split enum into separate file)
src/EventLogExpert.Eventing/Models/EventRecord.cs	Update imports after namespace moves
src/EventLogExpert.Eventing/Models/DisplayEventModel.cs	Update imports after namespace moves
src/EventLogExpert.Eventing/Logging/WarnLogHandler.cs	New warning-level interpolated string handler
src/EventLogExpert.Eventing/Logging/TraceLogHandler.cs	New trace-level interpolated string handler
src/EventLogExpert.Eventing/Logging/TraceLogger.cs	Move TraceLogger to Logging namespace
src/EventLogExpert.Eventing/Logging/LogHandlerCore.cs	New shared core for interpolated string handlers
src/EventLogExpert.Eventing/Logging/ITraceLogger.cs	Move ITraceLogger to Logging namespace
src/EventLogExpert.Eventing/Logging/InfoLogHandler.cs	New info-level interpolated string handler
src/EventLogExpert.Eventing/Logging/ErrorLogHandler.cs	New error-level interpolated string handler
src/EventLogExpert.Eventing/Logging/DebugLogHandler.cs	New debug-level interpolated string handler
src/EventLogExpert.Eventing/Logging/CriticalLogHandler.cs	New critical-level interpolated string handler
src/EventLogExpert.Eventing/Interop/Win32ErrorCodes.cs	Rename/move Win32 error constants container
src/EventLogExpert.Eventing/Interop/SystemTime.cs	Move SystemTime struct into Interop namespace
src/EventLogExpert.Eventing/Interop/NativeMethods.Evt.cs	Consolidate EVT P/Invoke and enums under Interop + rename EventMethods → NativeMethods
src/EventLogExpert.Eventing/Interop/NativeMethods.cs	Move remaining interop helpers under Interop + extract LoadLibraryFlags
src/EventLogExpert.Eventing/Interop/NativeErrorResolver.cs	Rename/move resolver to NativeErrorResolver under Interop
src/EventLogExpert.Eventing/Interop/MessageResourceBlock.cs	Move struct into Interop namespace
src/EventLogExpert.Eventing/Interop/LoadLibraryFlags.cs	New extracted LoadLibraryFlags enum file
src/EventLogExpert.Eventing/Interop/LibraryHandle.cs	Move LibraryHandle into Interop namespace
src/EventLogExpert.Eventing/Interop/HResultConverter.cs	Rename/move Converter → HResultConverter under Interop
src/EventLogExpert.Eventing/Interop/EvtVariant.cs	Move EvtVariant into Interop namespace
src/EventLogExpert.Eventing/Interop/EvtHandle.cs	Move EvtHandle into Interop namespace and use NativeMethods.EvtClose
src/EventLogExpert.Eventing/Interop/EvtEnums.cs	New file extracting EVT-related enums into Interop
src/EventLogExpert.Eventing/Helpers/TraceInterpolatedStringHandler.cs	Remove old combined interpolated string handler file (replaced by Logging/*)
src/EventLogExpert.Eventing/Helpers/ExtensionMethods.cs	Remove unused extension methods
src/EventLogExpert.Eventing/EventResolvers/EventXmlResolver.cs	Seal resolver + switch to injectable resolve strategy + interop renames
src/EventLogExpert.Eventing/EventResolvers/EventResolverBase.cs	Update interop/error resolver usage + formatting normalization
src/EventLogExpert.Eventing/EventResolvers/EventResolver.cs	Update logging namespace imports
src/EventLogExpert.Eventing/EventProviderDatabase/JsonValueConverter.cs	Remove unused converter
src/EventLogExpert.Eventing/EventProviderDatabase/EventProviderDbContext.cs	Update logging namespace imports + minor formatting normalization
src/EventLogExpert.Eventing/EventProviderDatabase/CompressedJsonValueConverter.cs	Internalize/seal converter type
src/EventLogExpert.Eventing/EventLogExpert.Eventing.csproj	Add InternalsVisibleTo entries for unit + integration test assemblies
src/EventLogExpert.Eventing.Tests/Readers/SmallEvtxFixture.cs	Remove fixture from unit tests (moved to integration tests)
src/EventLogExpert.Eventing.Tests/Readers/LogNamesTests.cs	Update namespaces to match Readers folder structure
src/EventLogExpert.Eventing.Tests/Providers/EventMessageProviderTests.cs	Trim unit tests to non-environmental cases + move integration behaviors out
src/EventLogExpert.Eventing.Tests/Interop/NativeMethodsTests.cs	Update namespaces to Interop structure
src/EventLogExpert.Eventing.Tests/Interop/NativeMethodsEvtTests.cs	Rename test class + update interop references
src/EventLogExpert.Eventing.Tests/Interop/NativeErrorResolverTests.cs	Rename/update tests for NativeErrorResolver
src/EventLogExpert.Eventing.Tests/EventResolvers/VersatileEventResolverTests.cs	Strengthen mixed-provider resolution coverage + convert provider loop to theory
src/EventLogExpert.Eventing.Tests/EventResolvers/LocalProviderEventResolverTests.cs	Update logging namespace imports
src/EventLogExpert.Eventing.Tests/EventResolvers/EventXmlResolverTests.cs	Switch from inheritance-based test doubles to strategy injection + adjust assertions
src/EventLogExpert.Eventing.Tests/EventResolvers/EventResolverCacheTests.cs	Minor formatting normalization
src/EventLogExpert.Eventing.Tests/EventResolvers/EventResolverBaseTests.cs	Update interop/logging imports + strengthen assertion determinism
src/EventLogExpert.Eventing.Tests/EventResolvers/EventProviderDatabaseEventResolverTests.cs	Update logging namespace imports
src/EventLogExpert.Eventing.Tests/EventProviderDatabase/EventProviderDbContextTests.cs	Update logging namespace imports + minor formatting normalization
src/EventLogExpert.Eventing.Tests/EventProviderDatabase/CompressedJsonValueConverterTests.cs	Tighten exception assertion to InvalidDataException
src/EventLogExpert.Eventing.IntegrationTests/Readers/SmallEvtxFixture.cs	New deterministic EVTX fixture for integration tests (exports bounded live-log window)
src/EventLogExpert.Eventing.IntegrationTests/Readers/EventLogSessionTests.cs	Move tests to integration project + tighten exception expectations
src/EventLogExpert.Eventing.IntegrationTests/Readers/EventLogReaderTests.cs	Replace vacuous live-log checks with deterministic fixture-backed assertions
src/EventLogExpert.Eventing.IntegrationTests/Readers/EventLogInformationTests.cs	Move tests to integration project + strengthen argument/exception coverage
src/EventLogExpert.Eventing.IntegrationTests/Providers/RegistryProviderTests.cs	Move tests to integration project + make probes deterministic/non-vacuous
src/EventLogExpert.Eventing.IntegrationTests/Providers/ProviderMetadataTests.cs	Move tests to integration project and trim vacuous assertions
src/EventLogExpert.Eventing.IntegrationTests/Providers/EventMessageProviderIntegrationTests.cs	New integration tests for environment-dependent EventMessageProvider behaviors
src/EventLogExpert.Eventing.IntegrationTests/GlobalUsings.cs	Add xUnit global using for integration project
src/EventLogExpert.Eventing.IntegrationTests/EventLogExpert.Eventing.IntegrationTests.csproj	New integration test project definition
src/EventLogExpert.EventDbTool/UpgradeDatabaseCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/ShowCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/RegexHelper.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/ProviderSource.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/Program.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/MtaProviderSource.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/MergeDatabaseCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/DiffDatabaseCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/DbToolCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool/CreateDatabaseCommand.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool.Tests/UpgradeDatabaseCommandTests.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool.Tests/ProviderSourceTests.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool.Tests/MergeDatabaseCommandTests.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.EventDbTool.Tests/DiffDatabaseCommandTests.cs	Update logging namespace imports after folder/namespace reorg
src/EventLogExpert.Components/Menu/MenuBar.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components/Filters/SubFilterRow.razor.cs	Minor formatting-only adjustment
src/EventLogExpert.Components/Database/SettingsUpgradeProgressBanner.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components/Database/DatabaseRecoveryHost.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components/Database/DatabaseRecoveryDialog.razor.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components/BannerHost.razor.cs	Update Eventing namespace imports after folder/namespace reorg + minor formatting normalization
src/EventLogExpert.Components.Tests/Database/SettingsUpgradeProgressBannerTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components.Tests/Database/DatabaseRecoveryHostTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components.Tests/Database/DatabaseRecoveryDialogTests.cs	Update Eventing namespace imports after folder/namespace reorg
src/EventLogExpert.Components.Tests/BannerHostTests.cs	Update Eventing namespace imports after folder/namespace reorg
.github/workflows/PullRequest.yml	Split CI test execution into unit vs. integration steps

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jschick04]

Fixed by tightening the docs (option A) rather than the impl. Verified the early-return is the IDisposable.Dispose idempotency contract that callers rely on, so demoting it to "always throw from a handler" would force unnecessary coordination at every call site.

Updated three XML docs in EventLogWatcher.cs to publish the conditional contract:

• EventRecordWritten: throws "unless another thread has already begun disposing or unsubscribing the watcher (in which case the call is a silent no-op for IDisposable idempotency)."
• Enabled setter (false case): throws "while the watcher is still subscribed... If another thread has already unsubscribed or disposed the watcher, the call is a silent no-op."
• Dispose(): throws "while the watcher is still live... If another thread has already begun disposing the watcher, the call is a silent no-op for IDisposable idempotency."

Existing tests already cover both invariants independently (Dispose_WhenCalledMultipleTimes_ShouldNotThrow for idempotency, Dispose_WhenCalledFromHandler_ShouldThrowInvalidOperationException + Enabled_WhenSetToFalseFromHandler_ShouldThrowInvalidOperationException for the reentrancy guard). Skipped adding a race-coordinated test for the conjunction since the documented behavior is the natural composition of those two.

Amended into commit 68f999f (Isolate handler exceptions and reject reentrant stop in EventLogWatcher) → new SHA db702a6.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated 2 comments.

[jschick04]

Both findings addressed in commit be201ac (fixup-autosquashed into the existing "Isolate handler exceptions and reject reentrant stop in EventLogWatcher" commit).

Finding 1 — Unsubscribe() race / hang on second Unregister

Rather than checking the RegisteredWaitHandle.Unregister(...) return value (which only papers over the symptom), the underlying race is the _waitHandle is not null check not being atomic with the assignment. Two concurrent callers (Dispose racing Enabled = false, or two Enabled = false setters) can both pass the null check and both call Unregister; the second call returns false and never signals its ManualResetEvent, so WaitOne() hangs.

Fix: serialize the entire lifecycle (Subscribe + Unsubscribe + the disposed-state check that gates Subscribe) under a single Lock _lifecycleLock. Lock-based serialization (rather than Interlocked.Exchange) is required because callers rely on the contract "no callback fires after Unsubscribe returns" — the loser must observe the winner's drain, which Interlocked.Exchange cannot guarantee. Reentry from inside a callback is impossible because both Unsubscribe call sites first invoke ThrowIfCurrentCallbackWouldDeadlock.

While here, also caught a related pre-existing race the panel review surfaced: Subscribe() mutates _subscriptionHandle / _waitHandle outside any lock, so a concurrent Enabled = true racing with Dispose() (which the consumer in LiveLogWatcherService does — line 199 starts subscription on Task.Run, line 236 disposes on a separate Task.Run) could leave _subscriptionHandle disposed mid-Subscribe. Putting Subscribe under the same lock closes the window. Post-dispose Enabled = true now throws a clean ObjectDisposedException instead of an obscure native-handle failure (documented on the setter via a new <exception> tag).

Finding 2 — _newEvents AutoResetEvent leak

Disposed in the if (disposing) block of Dispose(bool), immediately after Unsubscribe(). Ordering matters: in-flight ProcessNewEvents callbacks reference _newEvents.SafeWaitHandle via the RegisteredWaitHandle, so disposing earlier could crash a callback mid-flight. After Unsubscribe's drain completes, no more callbacks are in flight, so disposal is safe.

Bonus correctness fix (panel-surfaced)

In the new lock-serialized Unsubscribe, both lock entrants would otherwise execute the _subscriptionHandle.Dispose() block — SafeHandle.IsInvalid is based on the handle value and does NOT flip after Dispose(). Changed the guard to !_subscriptionHandle.IsClosed so the loser correctly skips the second dispose. (Today this is benign because SafeHandle.Dispose is idempotent; the explicit check removes the dependency on that idempotency.)

Tests added (3)

1. Dispose_ShouldReleaseUnderlyingWaitHandle — reflects on _newEvents, asserts SafeWaitHandle.IsClosed after Dispose(), and re-asserts after a second Dispose() to lock in idempotency. (Reflection here is justified: the leak was found precisely because there was no externally-observable signal of disposal; exposing internal state via a test-only API would be a SUT smell.)
2. Enabled_WhenRacingFromMultipleThreads_ShouldNotHangOrCorruptState — races 50 Enabled = true against 50 Enabled = false calls in parallel Task.Runs with a 30s timeout. Replaces an earlier draft test that raced 4 Dispose() calls — that test was a placebo because the existing _disposed CAS at the top of Dispose(bool) already serializes Dispose-vs-Dispose; the actual race window is between two Enabled = false callers (or Enabled = true racing Enabled = false).
3. Enabled_WhenSetToTrueAfterDispose_ShouldThrowObjectDisposedException — locks in the new contract for the post-dispose Enabled = true case.

All gates green: 0 warnings/0 errors; whitespace + IDE0005/IDE0065 format clean; 1875/1876 tests passing + 1 env-skip baseline.

[jschick04]

Round 3 — addressed both XML-doc/impl mismatch findings by deleting the overstated wording rather than tightening the impl.

The EventRecordWritten/Enabled/Dispose() docs added in round 1 claimed silent-no-op semantics for "another thread already disposing/unsubscribing" that the impl never actually implemented (and that have no operational justification). Per csharp.instructions.md <exception> mirror rule, the docs now mirror the actual conditional branching:

• Enabled = false: Thrown when set to false from inside an EventRecordWritten handler while the watcher is still subscribed (matches case false when _isSubscribed gate).
• Dispose(): Thrown when invoked from inside an EventRecordWritten handler unless the watcher has already been disposed (matches the _disposed != 0 fast-path).

Also did a global comment-discipline pass across the PR diff (10 files, -279 net lines) per AGENTS.md §3.1 hard cap of ≤ 12 words for inline comments and one-sentence cap for XML <summary>. Block comments that restated the test name (// Act — drive the SUT through whichever well-known…), narrated obvious code (// Volatile.Write provides cross-thread visibility…), or speculated about future surfaces have been deleted. Load-bearing facts kept: IsClosed vs IsInvalid, Unregister wait-object, _newEvents.Dispose ordering after Unsubscribe.

• tip: 5e8481e
• build 0/0, format clean, EventLogWatcher 58/58, resolver tests 90/90.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated 2 comments.

[jschick04]

Round 4 — addresses both new bot findings on top of 5e8481e ➜ 64dea41.

1. LogHandlerCore.AppendFormatted(ReadOnlySpan<char>, int alignment, string? format) ignored alignment (id 3209162188).
The span overload now mirrors the padding logic in AppendAligned<T>: positive alignment right-pads, negative left-pads, int.MinValue clamps to int.MaxValue width, and alignment == 0 short-circuits to a plain append. format remains intentionally unused — ReadOnlySpan<char> is not IFormattable — but the parameter is required for C# interpolation-handler binding (so $"{span,10:F2}" and $"{span:F2}" still compile).

2. Subscribe() invoked ProcessNewEvents (and therefore subscriber handlers) while holding _lifecycleLock (id 3209162099).
Moved the explicit initial-backlog drain outside the lock. The lock now only covers state mutation and wait-handle registration; handlers (both initial drain and ongoing threadpool callbacks) run lock-free, eliminating the lock-order-deadlock vector.

The Subscribe→Dispose race during the unlocked drain is bounded by:

• ProcessNewEvents checks _isSubscribed at the top of every EvtNext iteration and breaks out if Unsubscribe flipped it.
• The EvtNext P/Invoke takes EvtHandle (a SafeHandle); the marshaller DangerousAddRef-s for the duration of each call, so a concurrent _subscriptionHandle.Dispose() cannot close the native handle out from under an in-flight EvtNext — it'll surface as a clean ERROR_INVALID_HANDLE return.

Verification:

• Build 0/0 (Eventing + IntegrationTests + Tests).
• Whitespace + IDE0005/IDE0065 format clean.
• 235/236 + 1 env-skip integration tests pass (incl. all 58 EventLogWatcherTests).
• 389/389 EventLogExpert.Eventing.Tests pass.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/EventLogExpert.Eventing/Interop/NativeMethods.Evt.cs:6

• NativeMethods (Interop layer) now takes a dependency on EventLogExpert.Eventing.Readers solely to use PathType. Since PathType is a Win32 interop flag used by P/Invoke signatures (EvtQuery, EvtOpenLog), keeping it in the Readers namespace couples Interop ↔ Readers and muddles the layering. Consider moving PathType into EventLogExpert.Eventing.Interop (alongside the other EVT enums) and updating readers/consumers to reference it there.

[jschick04]

Round 5 — addresses both new bot findings on top of 64dea41 ➜ 7573962.

1. _isSubscribed lacked memory-barrier semantics across threads (id 3209333619).
All reads (Enabled getter, Enabled setter switch cases, ProcessNewEvents stop-condition) now go through Volatile.Read(ref _isSubscribed); both writes (Subscribe success-path, Unsubscribe) go through Volatile.Write(ref _isSubscribed, …). Pattern matches the existing Volatile.Read(ref _disposed) discipline. The Enabled setter switch cases remain a check-then-act fast-path (Subscribe()/Unsubscribe() re-validate under _lifecycleLock), so Volatile here just guarantees current observation rather than serialization.

2. RegisterWaitForSingleObject could fire ProcessNewEvents concurrently with the explicit Subscribe-thread drain (id 3209333562).
Reordered Subscribe() so the wait registration happens after the initial drain, in a second locked phase:

1. Lock A: validate, EvtSubscribe, Volatile.Write(ref _isSubscribed, true).
2. Unlocked: ProcessNewEvents(null, false) — only the calling thread is invoking EvtNext (no TP wait registered yet).
3. Lock B: re-check _disposed (throws ObjectDisposedException matching the documented Enabled = true contract); re-check _isSubscribed (silently returns if a concurrent Unsubscribe already tore it down); register the TP wait.

Result: only one thread ever runs EvtNext against _subscriptionHandle at a time during Subscribe; subsequent TP-fired callbacks are serialized by the OS (RegisterWaitForSingleObject does not queue overlapping callbacks for the same registration when the prior callback is still running with the auto-reset event in non-signaled state).

The unlocked-drain race with concurrent Dispose is bounded by:

• The EvtNext P/Invoke marshaller DangerousAddRef-s _subscriptionHandle for the call duration; a concurrent dispose surfaces as ERROR_INVALID_HANDLE rather than a native crash.
• Volatile.Read(ref _isSubscribed) at the top of every drain iteration breaks the loop as soon as Unsubscribe flips it.
• Lock B re-checks _disposed so we never register a wait against a disposed AutoResetEvent.

Verification:

• Build 0/0 (Eventing + IntegrationTests + Tests).
• Whitespace + IDE0005/IDE0065 format clean.
• 235/236 + 1 env-skip integration tests pass (incl. all 58 EventLogWatcherTests).
• 389/389 EventLogExpert.Eventing.Tests pass.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/EventLogExpert.Eventing/Interop/NativeMethods.Evt.cs:682

• ThrowEventLogException computes a resolved Win32 message but discards it for UnauthorizedAccessException, which makes diagnostics much harder (and leaves message unused in these cases). Pass the resolved message into UnauthorizedAccessException (and optionally include the error code) so callers get actionable context.

[jschick04]

Round 6 — addresses the low-confidence reviewer note (16:31 PT) on top of 7573962 ➜ e9f240c.

ThrowEventLogException discarded the resolved Win32 message for UnauthorizedAccessException (NativeMethods.Evt.cs:682). The four sibling cases (FileNotFoundException, InvalidDataException, OperationCanceledException, Exception) all forward message; the ERROR_ACCESS_DENIED / ERROR_INVALID_HANDLE case alone constructed an empty UAE, leaving callers without diagnostic context. One-line fix: throw new UnauthorizedAccessException(message);.

Verified that no test asserts on the empty message — both ThrowEventLogException_WhenAccessDenied_ShouldThrowUnauthorizedAccessException and …WhenInvalidHandle… only check the type via Assert.Throws<UnauthorizedAccessException>. All 13 ThrowEventLogException tests still pass.

Build 0/0; format clean.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated 1 comment.

[jschick04]

Round-7 finalizer-path fix applied in 81c9daa.

Problem: ~EventLogWatcher calls Dispose(false), but the original Dispose(bool disposing) only released the ThreadPool wait registration and the native EvtSubscribe handle when disposing == true. If a consumer skipped Dispose(), the wait registration kept the instance rooted via the TP delegate (so the finalizer effectively couldn't run), and even on the partial-init / abandoned-after-Unsubscribe edge cases the native subscription handle was left to SafeHandle's own finalizer rather than being released deterministically.

Fix: Added an explicit else branch to Dispose(bool disposing) for the finalizer path:

• _waitHandle?.Unregister(null) — non-blocking unregister (passing null for the WaitObject skips the signaling step, so it returns immediately rather than waiting for in-flight callbacks).
• if (!_subscriptionHandle.IsClosed) { _subscriptionHandle.Dispose(); } — explicit close of the native EvtHandle. Safe to call from a finalizer because EvtHandle is a SafeHandle with critical-finalizer semantics — its own finalizer is guaranteed to run after ours in the same finalization batch, so no double-close hazard.

Why no managed wait-handle disposal in the finalizer branch: _newEvents (AutoResetEvent) has its own WaitHandle finalizer; finalization order across managed wait primitives isn't deterministic, so disposing it from our finalizer would risk a use-after-dispose if the TP callback hasn't drained yet.

Build 0/0; format clean; 58/58 EventLogWatcher tests pass.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/EventLogExpert.Eventing.IntegrationTests/Providers/RegistryProviderTests.cs:19

• RegistryProvider.GetMessageFilesForLegacyProvider treats its argument as a provider/source subkey name under each EventLog channel (see RegistryProvider.cs where it does logSubKey?.OpenSubKey(providerName)). Including Constants.ApplicationLogName/Constants.SystemLogName in s_commonLegacyProviderNames is therefore mixing log names with provider names and can make FindAnyLegacyProviderFiles/Name fail or be host-dependent. Consider replacing these entries with stable legacy provider/source names (or skipping the test when none are found), and/or rename the array to reflect what it actually contains.

[Copilot]

Pull request overview

Copilot reviewed 135 out of 138 changed files in this pull request and generated no new comments.