Severity

Info / Warning (Maintainability + Testability)

Location

src/Servy.Core/Services/ServiceManager.cs lines 547 and 583

Code

```csharp
// line 547
Logger.Warn($"Failed to standardize start type before uninstall for '{serviceName}': Win32 Error {Marshal.GetLastWin32Error()}");

// line 583
string errorMsg = $"Failed to uninstall service '{serviceName}'. Win32 Error {Marshal.GetLastWin32Error()}";
```

Explanation

ServiceManager is constructed with an injected IWin32ErrorProvider _win32ErrorProvider specifically to allow Win32 error retrieval to be mocked in unit tests. Every other call site in this file uses it consistently (lines 112, 136, 165, 257, 298, 439, 866).

These two call sites in UninstallServiceAsync deviate from that pattern and call the static Marshal.GetLastWin32Error() directly. Consequences:

1. Test seam bypassed — Unit tests of the uninstall failure path cannot inject a deterministic Win32 error code; they have to rely on whatever the OS happens to set on the real Marshal static.
2. Inconsistent style — Two of nine call sites do not match the abstraction. A reader of this file will reasonably wonder whether the abstraction is meaningful or decorative.
3. Reliability — Marshal.GetLastWin32Error() retrieves the LastError from the calling thread. Any allocation, P/Invoke, or framework call between the failing API and the read may overwrite the value. The provider abstraction was presumably introduced (or could be) to capture LastError immediately after the offending call.

Suggested fix

Replace the two Marshal.GetLastWin32Error() calls with _win32ErrorProvider.GetLastWin32Error(), matching the rest of the file:

```csharp
// line 547
Logger.Warn($"Failed to standardize start type before uninstall for '{serviceName}': Win32 Error {_win32ErrorProvider.GetLastWin32Error()}");

// line 583
string errorMsg = $"Failed to uninstall service '{serviceName}'. Win32 Error {_win32ErrorProvider.GetLastWin32Error()}";
```