fix: [sc-112409] Report StopPending checkpoints while the Windows service stops

[mlataza]

Problem

windowsRunner.Execute published StartPending at entry and Running once the agent was up, but on receiving svc.Stop/svc.Shutdown it only signalled its internal stop channel and returned — Stopped was published after host.runner.Execute had fully returned. The service therefore went from Running straight to Stopped with a silent gap in between, and never sent svc.Status{State: StopPending} at all.

Windows reads that silence as a hang. The SCM expects a stopping service to acknowledge the control with StopPending and then keep showing progress — an incremented CheckPoint within the WaitHint the service published — and logs a service that goes quiet as not responding (events 7009/7043), with services.msc or net stop reporting failure while the process is still legitimately shutting down.

The gap is not short. The agent's shutdown drains in-flight command workers, each of which an operator can let run for as long as command_timeout_seconds (30 minutes by default), then waits on plugin shutdown. A stop, restart, auto-update or uninstall issued while a long command was in flight looked like a wedged service for its whole duration — enough to trigger alerting or an external forced kill mid-update — even though the agent was following its own bounded shutdown sequence correctly.

No customer report; found by code review while auditing the service-stop path alongside the caller-side bound from sc-108855 (README §"Bounded Windows Service Stop"). That ticket bounded how long the updater waits for Stopped; this is the service's side of the same stop.

Fix

1. The stop control is acknowledged immediately, before the runner starts draining: StopPending with CheckPoint 1 and a 30-second WaitHint. The report goes out before stop <- struct{}{}, so the SCM has the acknowledgment in hand before any shutdown work begins.

2. reportStopProgress republishes StopPending with an incrementing CheckPoint every stopPendingCheckpointInterval (10 seconds) until the runner returns — for as long as the shutdown actually takes, not once at the start. stopPendingWaitHint is three times the interval, so a merely late checkpoint on a busy endpoint does not read as a wedge either. Both constants carry the reasoning in their doc comments; the interval is injectable (windowsRunner.checkpointInterval) so the periodic behavior is unit-testable without waiting real seconds.

3. All of Execute's status updates route through one statusReporter that enforces the SCM's lifecycle order. This is load-bearing rather than tidying — Execute reports from three goroutines, and the dispatcher stops reading the response channel once Stopped arrives:

• Nothing is published after Stopped. A checkpoint goroutine waking up late would otherwise block forever on a channel nobody reads, and would contradict the terminal status. The mutex also makes the ordering deterministic: a checkpoint that wins the race is published before Stopped, one that loses is dropped.
• Running is dropped once a stop has been acknowledged. A stop control can land before startup finishes, and publishing Running after StopPending tells the SCM the service went healthy again and re-advertises the controls it accepts. Pre-existing hole, cheap to close once every send went through one place.

4. Stopped is still published exactly once, after host.runner.Execute returns, and is always the last status sent — unchanged from today apart from the ordering guarantee. cancelStop() ends the checkpoints first so the SCM does not see progress reported after the service has stopped.

No change to the caller-side bounded wait. Stop()'s 5-minute poll loop is untouched; this fixes what the service reports during that wait. A shutdown that genuinely wedges still fails on the same bound with the same error.

Documented in the README under "Reporting Stop Progress to the SCM (Windows only)", directly after the existing "Bounded Windows Service Stop" section it complements, and in CLAUDE.md's internal/service/ entry.

Tests

internal/service/service_windows_test.go. mockRunner gained stopping/shutdown channels so a test can hold the runner in the middle of its shutdown, the way a draining command worker or a slow plugin does in production.

• TestWindowsRunner_Execute_AcknowledgesStopBeforeStopped — the AC's core assertion: at least one StopPending precedes Stopped, carries a non-zero WaitHint and CheckPoint, nothing follows Stopped, exactly one Stopped, and it is last.
• TestWindowsRunner_Execute_CheckpointsWhileShuttingDown — holds the runner mid-shutdown with a 1 ms checkpoint interval, asserts checkpoints keep arriving with strictly incrementing CheckPoint, then releases the shutdown and confirms Stopped still terminates the sequence.
• TestStatusReporter_DropsReportsAfterStopped and TestStatusReporter_DropsRunningOnceStopping — the two lifecycle rules directly.
• TestStopPendingStatus_WaitHintExceedsCheckpointInterval — invariant guard, so tuning one constant without the other cannot silently produce a WaitHint the checkpoints miss.
• _SendsRunningThenStopped, _ShutdownAlsoStops and _ReturnsExitCode updated for the new StopPending in the sequence.

go build ./..., go vet ./..., gofmt and golangci-lint run ./... are clean for the host GOOS and for GOOS=windows; go test ./... passes. The new tests are //go:build windows and compile under GOOS=windows go test -c but cannot execute on the dev machine — CI's test.yml covers windows-latest.

Because of that, the concurrency in this change was also verified locally by running the reporter, the checkpoint loop and Execute's goroutine structure through a standalone -race harness (a stub svc.Status, same logic, repeated runs): fast shutdown produces exactly StopPending(cp 1, 30000 ms) → Stopped; a held shutdown produces checkpoints 1..5 strictly incrementing; nothing is sent after Stopped; no races reported.

QA

• Install the service on Windows, trigger a command with an artificially long runtime, issue a stop mid-command, and confirm via sc queryex <service> in a loop that the service reports STOP_PENDING throughout the shutdown — with CHECKPOINT advancing — instead of sitting at RUNNING and jumping straight to STOPPED.
• Confirm no new 7009/7043 Event Log entries for a shutdown that completes within its expected bound.
• Confirm a genuinely wedged shutdown still resolves exactly as it does today: Stop() gives up on the same 5-minute bound with the same did not stop within 5m0s error, and update/uninstall still abort without touching files.
• Spot-check the ordinary fast path (stop with nothing in flight) and an auto-update, to confirm the extra status traffic changes nothing observable beyond STOP_PENDING being briefly visible.