windows: fix live streaming for Update-all and companion staging - #5
Merged
Merged
Conversation
runWindowsUpdateInstall used direct bytes.Buffer + cmd.Run, bypassing runCapped's OutputSink/linetee, so admin live output stayed empty until the whole PowerShell download+install finished. Switch to runCapped(ctx, cmd) so each Write-Output line is tee'd to the aggregator via StreamOutput as it happens, like apt on Linux.
stageCompanionUpdate did pure Go download with no runCapped, so live pane stayed empty until done. Emit resolving/downloading/staged lines via existing emitFromContext helper.
timeout.exe refuses to run at all without a real console attached: "ERROR: Input redirection is not supported, exiting the process immediately." Confirmed live: every retry-delay call in this script hit this when install.bat was re-invoked non-interactively by the companion (a Windows Service, no console) for a self-update. The errors were silently swallowed (redirected to nul) in most call sites, but the practical effect was real -- every one of these delays silently did nothing, back-to-back, instead of actually waiting: - download_binary's antivirus-lock retry loop fired all 5 attempts instantly with no gap, giving Windows Defender's scan zero time to release a freshly-downloaded exe before giving up. - stop_if_running's 30-iteration stop-wait loop blew through in milliseconds instead of ~30s, risking a premature force-kill of a service that was genuinely still stopping. - All three uninstall_* paths' post-stop delay before sc delete. ping needs no console at all (confirmed live, ~1s for `-n 2` against 127.0.0.1 regardless of console presence) -- pinging loopback never touches the network, so this isn't a connectivity check, purely a side-effect delay, the standard console-free substitute for exactly this timeout.exe limitation.
Connect/Disconnect change what the "connected"/"offline" badges show for a host, independently of any registry mutation (enroll/report/ approve) -- the only things that fired AdminHub.Notify before this. So a reconnect (e.g. every agent/companion in the fleet, right after an aggregator restart) was invisible on the admin page until something else happened to trigger a reload, or the operator refreshed manually. Wired handleCompanionStream to notify on both, reusing the existing SSE-push-then-reload mechanism already in place for registry changes.
Confirmed live: "self-update failed (409): agent already has an action in flight" with no way to recover. pending/agentPending only got cleared by the specific connection's own deferred Disconnect -- if that connection instead died uncleanly (crash, network drop, anything that never delivers a clean TCP close), that cleanup either never ran, or ran after Connect had already replaced the entry and so no-opped (its cur.ch == ch guard correctly refusing to touch the new entry). Either way, the stale marker blocked every future Push for that agent with ErrActionInFlight/"agent stream busy" forever, since nothing else ever cleared it short of restarting the whole aggregator process. Connect now clears the relevant marker itself whenever it replaces an existing entry (main slot or agentStreams) -- a fresh connection taking over is exactly the right, safe point to declare whatever the old one had in flight unreachable, regardless of whether that old connection's own goroutine ever notices it's gone. Also corrected a stale doc comment on Connect describing the existing-companion/new-agent case as "rejected outright" -- that predated the agentStreams feature; it's actually accepted into agentStreams now, same as the code already does.
…required Confirmed live: Windows Gaming Services' own proxy DLL and Microsoft Edge's background auto-updater both re-queue an entry in PendingFileRenameOperations after nearly every single boot, forever -- a naive "list is non-empty" check made "Reboot required" (and the "Needs attention" badge that follows from it) permanently true on this host regardless of whether a reboot actually just happened, on practically any real Windows machine with Edge or Gaming Services installed (i.e. nearly all of them). Added an ignore-list for known-routine path patterns, checked per-entry (including the empty second half of a delete pair, which is routine by definition -- it's never a real path on its own). Anything not matching is still treated as a real pending change; when in doubt this errs toward reporting reboot-required, never toward hiding one. Split the pure matching logic into an untagged reboot_parse.go, same convention this package already uses for packages_parse.go/ windowsupdate_parse.go -- testable with fixture data on any platform, not just Windows.
…ntry Confirmed live on beta5: "Reboot required" stayed stuck true even with the ignore-list already in place. The Edge pattern had a trailing backslash (\microsoft\edge\temp\), matching only files/folders nested *inside* Temp -- but Edge's updater also queues the bare Temp folder itself as one of the pending entries, with no trailing separator, which that pattern silently never matched. Dropped the trailing backslash so it matches the folder path itself too, not just its contents.
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
1 participant
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.
Summary
applier_windows.go'srunWindowsUpdateInstallused a directbytes.Buffer+cmd.Run, bypassingrunCapped'sOutputSink/lineteetap entirely -- the admin live-output pane stayed empty for the whole PowerShell download+install, then jumped straight to the final result. Switched torunCapped(ctx, cmd)so each line streams as it happens, same as apt does on Linux.selfupdate_windows.go'sstageCompanionUpdatedid a plain Go download with no output at all. Addedresolving.../downloading.../download complete, staging...narration via the existingemitFromContexthelper.This is the root cause of "Windows doesn't stream on Update all" -- confirmed streaming already works correctly on Ubuntu, WSL Ubuntu, and Raspberry Pi (all go through the same
runCappedpath for apt), so this was Windows-specific.Test plan
go build/go vetclean on Linux andGOOS=windows.go test ./...clean except the two pre-existing, sandbox-only Unix-socket-path failures unrelated to this change.