From c9defe0ebfedaa939b7aa6ba3f330c9cdc4b5226 Mon Sep 17 00:00:00 2001 From: Winarto Date: Mon, 7 Sep 2026 20:14:27 +0800 Subject: [PATCH 1/7] windows: stream Windows Update install output live 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. --- internal/companion/applier_windows.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/companion/applier_windows.go b/internal/companion/applier_windows.go index 4124b61..408c090 100644 --- a/internal/companion/applier_windows.go +++ b/internal/companion/applier_windows.go @@ -3,7 +3,6 @@ package companion import ( - "bytes" "context" "errors" "fmt" @@ -246,12 +245,10 @@ if ($installResult.ResultCode -eq 4 -or $installResult.ResultCode -eq 5) { exit 1 } ` - var stdout, stderr bytes.Buffer cmd := exec.CommandContext(ctx, "powershell", "-NoProfile", "-NonInteractive", "-Command", script) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return stdout.String(), fmt.Errorf("windows update install: %w: %s", err, strings.TrimSpace(stderr.String())) + out, err := runCapped(ctx, cmd) + if err != nil { + return out, fmt.Errorf("windows update install: %w", err) } - return stdout.String(), nil + return out, nil } From 0fe5e3560213b567315c875938895da7fb974f63 Mon Sep 17 00:00:00 2001 From: Winarto Date: Mon, 7 Sep 2026 20:25:04 +0800 Subject: [PATCH 2/7] windows: emit progress during companion staging stageCompanionUpdate did pure Go download with no runCapped, so live pane stayed empty until done. Emit resolving/downloading/staged lines via existing emitFromContext helper. --- internal/companion/selfupdate_windows.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/companion/selfupdate_windows.go b/internal/companion/selfupdate_windows.go index c7425e1..5c5664c 100644 --- a/internal/companion/selfupdate_windows.go +++ b/internal/companion/selfupdate_windows.go @@ -58,9 +58,11 @@ func stageCompanionUpdate(ctx context.Context, action aggregator.Action) aggrega fail := func(format string, args ...any) aggregator.ActionResult { return aggregator.ActionResult{ActionID: action.ID, Message: fmt.Sprintf(format, args...), CompletedAt: time.Now()} } + emit := emitFromContext(ctx) // Resolve the download URL from the GitHub API. assetName := "update-detector-companion-windows-amd64.exe" + emit("resolving %s from release %s...", assetName, action.TargetVersion) downloadURL, err := resolveAssetURL(action.TargetVersion, assetName) if err != nil { return fail("resolving download URL: %v", err) @@ -72,9 +74,11 @@ func stageCompanionUpdate(ctx context.Context, action aggregator.Action) aggrega newPath := companionExePath + ".new" tmpPath := newPath + ".tmp" + emit("downloading companion %s...", action.TargetVersion) if err := downloadFile(ctx, downloadURL, tmpPath); err != nil { return fail("downloading companion update: %v", err) } + emit("download complete, staging...") // Atomic rename: tmp → .new if err := os.Rename(tmpPath, newPath); err != nil { From 17b1ec5eae21e6c9adc4050b9688b9fb555b7645 Mon Sep 17 00:00:00 2001 From: Winarto Date: Sun, 13 Sep 2026 11:59:22 +0800 Subject: [PATCH 3/7] install.bat: replace timeout with ping -- timeout needs a console 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. --- install.bat | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/install.bat b/install.bat index 4e45991..2efb2dc 100644 --- a/install.bat +++ b/install.bat @@ -166,7 +166,16 @@ for /l %%i in (1,1,5) do ( if not errorlevel 1 ( set "move_ok=1" ) else ( - timeout /t 1 /nobreak >nul + rem timeout.exe refuses to run at all without a real console -- + rem "ERROR: Input redirection is not supported, exiting the + rem process immediately." -- confirmed live when this runs + rem non-interactively (a Windows Service, e.g. the companion + rem re-invoking this script for a self-update has no console + rem at all). That failure was silent here (redirected to nul) + rem but meant every retry fired back-to-back with zero actual + rem delay, defeating this whole workaround. ping needs no + rem console at all; -n 2 against localhost gives ~1s. + ping -n 2 127.0.0.1 >nul ) ) ) @@ -227,7 +236,12 @@ if not errorlevel 1 goto :eof sc stop "%~1" >nul 2>&1 set "tries=0" :stop_wait_loop -timeout /t 1 /nobreak >nul +rem timeout.exe needs a real console (see download_binary's own +rem comment) -- without one this whole 30-iteration wait would blow +rem through in milliseconds instead of ~30s, giving up on a service +rem that's genuinely still stopping and force-killing it far too +rem early. ping needs no console at all. +ping -n 2 127.0.0.1 >nul sc query "%~1" | find "STOPPED" >nul if not errorlevel 1 goto :eof set /a tries+=1 @@ -702,7 +716,9 @@ if errorlevel 1 ( ) else ( echo install.bat: removing update-detector ^(agent^)... sc stop update-detector >nul 2>&1 - timeout /t 2 /nobreak >nul + rem ping, not timeout -- see download_binary's own comment on why + rem timeout.exe can't be used here (needs a real console). + ping -n 3 127.0.0.1 >nul sc delete update-detector >nul 2>&1 del /f /q "%BIN_DIR%\update-detector.exe" >nul 2>&1 echo install.bat: removing %ProgramData%\update-detector ^(includes this agent's aggregator identity^) @@ -722,7 +738,7 @@ if errorlevel 1 ( ) else ( echo install.bat: removing update-aggregator... sc stop update-aggregator >nul 2>&1 - timeout /t 2 /nobreak >nul + ping -n 3 127.0.0.1 >nul sc delete update-aggregator >nul 2>&1 del /f /q "%BIN_DIR%\update-aggregator.exe" >nul 2>&1 echo install.bat: removing %ProgramData%\update-aggregator ^(includes the fleet registry -- all enrolled/approved hosts^) @@ -741,7 +757,7 @@ rem Companion is always native, needs real administrator rights to run rem winget -- no Docker case to check here, same as install.sh's own rem uninstall_companion. sc stop update-detector-companion >nul 2>&1 -timeout /t 2 /nobreak >nul +ping -n 3 127.0.0.1 >nul sc delete update-detector-companion >nul 2>&1 del /f /q "%BIN_DIR%\update-detector-companion.exe" >nul 2>&1 del /f /q "%CACHED_INSTALL_BAT%" >nul 2>&1 From d5eb83b13788427cbe80fb6c90b0a4187265c010 Mon Sep 17 00:00:00 2001 From: Winarto Date: Sun, 13 Sep 2026 12:29:34 +0800 Subject: [PATCH 4/7] admin: notify browsers live on agent/companion connect and disconnect 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. --- internal/aggregator/server.go | 12 +++++++- internal/aggregator/server_test.go | 49 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/internal/aggregator/server.go b/internal/aggregator/server.go index 123d02b..fb8e012 100644 --- a/internal/aggregator/server.go +++ b/internal/aggregator/server.go @@ -232,7 +232,17 @@ func (s *Server) handleCompanionStream(w http.ResponseWriter, r *http.Request) { http.Error(w, "superseded: a companion is already connected for this agent", http.StatusConflict) return } - defer s.hub.Disconnect(rec.ID, result.Ch) + // Connect/Disconnect change what the admin page's "connected"/"offline" + // badges show for this agent, independently of any registry mutation + // (enroll/report/approve) -- without notifying here too, a reconnect + // (e.g. every agent/companion in the fleet, right after an aggregator + // restart) was invisible until something else happened to trigger a + // reload, or the operator refreshed manually. + s.adminHub.Notify() + defer func() { + s.hub.Disconnect(rec.ID, result.Ch) + s.adminHub.Notify() + }() // Only meaningful for a real companion (see SetAggregatorPresent) -- // an agent-only connection has never run the aggregator-colocation // check at all, so a missing/unparseable header here (including one diff --git a/internal/aggregator/server_test.go b/internal/aggregator/server_test.go index 85309aa..aec787b 100644 --- a/internal/aggregator/server_test.go +++ b/internal/aggregator/server_test.go @@ -344,6 +344,55 @@ func TestHandleCompanionStreamRequiresAuth(t *testing.T) { } } +// TestHandleCompanionStreamNotifiesAdminHubOnConnectAndDisconnect is the +// regression test for the admin page never showing a reconnect live: +// Connect/Disconnect change the "connected"/"offline" badges independently +// of any registry mutation (enroll/report/approve), so without this the +// only existing notify triggers (registry changes) had no reason to fire +// on their own -- an operator watching the page during, say, a fleet-wide +// reconnect after an aggregator restart would see nothing update until +// something else happened to trigger a reload. +func TestHandleCompanionStreamNotifiesAdminHubOnConnectAndDisconnect(t *testing.T) { + s, reg := newTestServer(t) + approvedAgent(t, s, reg, "a1", "web01", "tok") + + adminCh, cancel := s.adminHub.Subscribe() + defer cancel() + + httpSrv := httptest.NewServer(s.Handler()) + defer httpSrv.Close() + + req, err := http.NewRequest(http.MethodGet, httpSrv.URL+"/companion/stream", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("X-Agent-ID", "a1") + req.Header.Set("Authorization", "Bearer tok") + + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("got status %d, want 200", resp.StatusCode) + } + + select { + case <-adminCh: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for a notify on connect") + } + + resp.Body.Close() + + select { + case <-adminCh: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for a notify on disconnect") + } +} + func TestHandleCompanionStreamPushesAction(t *testing.T) { s, reg := newTestServer(t) approvedAgent(t, s, reg, "a1", "web01", "tok") From c5f37097a0a8bcd120f362048408b8cdfaa4ffe9 Mon Sep 17 00:00:00 2001 From: Winarto Date: Sun, 13 Sep 2026 12:41:15 +0800 Subject: [PATCH 5/7] aggregator: clear stale pending/agentPending markers on reconnect 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. --- internal/aggregator/companion.go | 35 ++++++++++++++---- internal/aggregator/companion_test.go | 51 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/internal/aggregator/companion.go b/internal/aggregator/companion.go index 48e8d0f..8705ec9 100644 --- a/internal/aggregator/companion.go +++ b/internal/aggregator/companion.go @@ -214,14 +214,30 @@ type ConnectResult struct { } // Connect registers agentID as having a live stream, with the following -// arbitration (companion always outranks agent -- see ActionType.requiresCompanion): +// arbitration (companion always outranks agent for the main slot -- see +// ActionType.requiresCompanion): // - no existing entry: accept unconditionally, either kind. // - existing kind == new kind (a reconnect, e.g. after a restart): // replace as before, no signal fired -- this is not a priority change. -// - existing=agent, new=companion: companion preempts. The old entry's -// superseded channel is closed so its holder notices and tears down. -// - existing=companion, new=agent: rejected outright; the existing -// companion stream is left untouched. +// - existing=agent, new=companion: companion preempts the main slot. +// The old entry's superseded channel is closed so its holder notices +// and tears down. +// - existing=companion, new=agent: the companion keeps the main slot, +// but the agent is still accepted into agentStreams (see below), so +// agent-only actions (recheck, ActionCompleteCompanionSwap) can still +// reach it even while a companion is connected. +// +// Whenever this replaces an existing entry (main slot or agentStreams), +// it also clears that agentID's corresponding pending/agentPending marker +// -- confirmed live as a real stuck-forever bug otherwise: if the old +// connection died uncleanly (crash, network drop, anything that never +// delivers a clean TCP close), its own deferred Disconnect either never +// runs at all, or runs too late and no-ops against the channel it's +// comparing against (already replaced by the time it gets there) -- so +// without clearing it here too, an action that will now never resolve +// (its executor is gone) blocks every future Push for that agent with +// ErrActionInFlight/"agent stream busy", forever, until the whole +// aggregator process restarts. // // companionVersion is only recorded when kind is KindCompanion -- it has // no meaning for an agent-only connection, and must not clobber the last @@ -243,6 +259,7 @@ func (h *CompanionHub) Connect(agentID string, kind ClientKind, companionVersion // Close any previous agent stream for this ID if prev, ok := h.agentStreams[agentID]; ok { close(prev.superseded) + delete(h.agentPending, agentID) } h.agentStreams[agentID] = agentEntry return ConnectResult{Accepted: true, Ch: agentEntry.ch, Superseded: agentEntry.superseded} @@ -253,14 +270,18 @@ func (h *CompanionHub) Connect(agentID string, kind ClientKind, companionVersion kind: kind, superseded: make(chan struct{}), } - if hasExisting && existing.kind != kind { - close(existing.superseded) + if hasExisting { + if existing.kind != kind { + close(existing.superseded) + } + delete(h.pending, agentID) } h.streams[agentID] = entry if kind == KindAgent { // Also track in agentStreams for agent-only action routing if prev, ok := h.agentStreams[agentID]; ok { close(prev.superseded) + delete(h.agentPending, agentID) } h.agentStreams[agentID] = entry } diff --git a/internal/aggregator/companion_test.go b/internal/aggregator/companion_test.go index e057076..cefc671 100644 --- a/internal/aggregator/companion_test.go +++ b/internal/aggregator/companion_test.go @@ -106,6 +106,57 @@ func TestCompanionHubDisconnectClearsInFlight(t *testing.T) { } } +// TestCompanionHubReconnectClearsStalePendingWithoutDisconnect is the +// regression test for a real stuck-forever bug: if the old connection +// died uncleanly (crash, network drop -- anything that never delivers a +// clean TCP close), its own deferred Disconnect either never runs, or +// runs after Connect already replaced the entry and so no-ops (its +// cur.ch == ch guard fails). Without Connect itself clearing the stale +// marker, a fresh reconnect from that same agent would still get +// ErrActionInFlight forever for an action whose executor no longer +// exists -- exactly "self-update failed (409): agent already has an +// action in flight, but nothing happens" observed live. +func TestCompanionHubReconnectClearsStalePendingWithoutDisconnect(t *testing.T) { + h := NewCompanionHub() + h.Connect("a1", KindCompanion, "") + + if err := h.Push("a1", Action{ID: "act1", Type: ActionUpgrade}); err != nil { + t.Fatalf("push failed: %v", err) + } + + // Reconnect *without* ever calling Disconnect for the old connection -- + // simulates the old one having died uncleanly. + h.Connect("a1", KindCompanion, "") + + if err := h.Push("a1", Action{ID: "act2", Type: ActionUpgrade}); err != nil { + t.Fatalf("expected push to succeed after an unclean reconnect, got: %v", err) + } +} + +// TestCompanionHubAgentReconnectClearsStaleAgentPending is the same +// regression, for the agentStreams slot (e.g. a stuck recheck or +// companion-swap action) instead of the main companion slot. +func TestCompanionHubAgentReconnectClearsStaleAgentPending(t *testing.T) { + h := NewCompanionHub() + h.Connect("a1", KindAgent, "") + + if err := h.Push("a1", Action{ID: "act1", Type: ActionRecheck}); err != nil { + t.Fatalf("push failed: %v", err) + } + if !h.IsPending("a1", "act1") { + t.Fatal("expected act1 to be tracked as pending") + } + + h.Connect("a1", KindAgent, "") + + if h.IsPending("a1", "act1") { + t.Fatal("expected the stale agentPending marker to be cleared by the reconnect") + } + if err := h.Push("a1", Action{ID: "act2", Type: ActionRecheck}); err != nil { + t.Fatalf("expected push to succeed after an unclean reconnect, got: %v", err) + } +} + func TestCompanionHubTracksCompanionVersion(t *testing.T) { h := NewCompanionHub() From 3da37e0ef0404cf90edf83f62d26f04c0a6578b6 Mon Sep 17 00:00:00 2001 From: Winarto Date: Sun, 13 Sep 2026 13:02:01 +0800 Subject: [PATCH 6/7] windows: ignore routine PendingFileRenameOperations noise for reboot-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. --- internal/checker/windows/reboot.go | 12 +-- internal/checker/windows/reboot_parse.go | 53 +++++++++++++ internal/checker/windows/reboot_parse_test.go | 79 +++++++++++++++++++ 3 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 internal/checker/windows/reboot_parse.go create mode 100644 internal/checker/windows/reboot_parse_test.go diff --git a/internal/checker/windows/reboot.go b/internal/checker/windows/reboot.go index a57a7f0..2497432 100644 --- a/internal/checker/windows/reboot.go +++ b/internal/checker/windows/reboot.go @@ -34,10 +34,12 @@ func keyExists(path string) bool { } // pendingFileRenameOperationsSet reports whether Session Manager's -// PendingFileRenameOperations value is set and non-empty -- a non-empty -// REG_MULTI_SZ here means the OS has files staged to be renamed/deleted -// on next boot, the same signal Windows Update and many installers use -// to indicate a pending reboot. +// PendingFileRenameOperations value has any entry that isn't routine +// noise (see isRoutinePendingRename/anyRealPendingRename in +// reboot_parse.go) -- a real, non-empty REG_MULTI_SZ entry here means +// the OS has files staged to be renamed/deleted on next boot, the same +// signal Windows Update and many installers use to indicate a pending +// reboot. func pendingFileRenameOperationsSet() bool { k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SYSTEM\CurrentControlSet\Control\Session Manager`, registry.QUERY_VALUE) if err != nil { @@ -48,5 +50,5 @@ func pendingFileRenameOperationsSet() bool { if err != nil { return false } - return len(values) > 0 + return anyRealPendingRename(values) } diff --git a/internal/checker/windows/reboot_parse.go b/internal/checker/windows/reboot_parse.go new file mode 100644 index 0000000..4b13270 --- /dev/null +++ b/internal/checker/windows/reboot_parse.go @@ -0,0 +1,53 @@ +// Deliberately untagged, same reasoning as packages_parse.go: pure +// string matching with no OS-specific imports, testable on any platform, +// not just Windows. +package windows + +import "strings" + +// pendingRenameNoisePatterns lists (lowercased) path substrings for +// entries that reappear in PendingFileRenameOperations after nearly +// every single boot, forever, regardless of whether anything meaningful +// is actually waiting -- confirmed live on a real host: Windows Gaming +// Services' own proxy DLL and Microsoft Edge's background auto-updater +// both re-queue an entry here almost immediately after every reboot. A +// naive "list is non-empty" check is permanently true on any host with +// either installed (i.e. nearly all of them) and useless as a signal. +// Entries matching one of these are ignored; anything else still counts +// as a real pending change -- when in doubt, this errs toward reporting +// reboot-required, never toward hiding one. +var pendingRenameNoisePatterns = []string{ + `\gamingservicesproxy`, + `\microsoft\edge\temp\`, +} + +// isRoutinePendingRename reports whether entry (one raw string from +// PendingFileRenameOperations -- either half of a rename pair, or a +// delete pair's always-empty second half) matches a known-routine +// pattern. An empty string is routine by definition: it's never a real +// path on its own, only ever the "delete" half of a pair whose other +// half is what actually identifies what's pending. +func isRoutinePendingRename(entry string) bool { + if entry == "" { + return true + } + lower := strings.ToLower(entry) + for _, pattern := range pendingRenameNoisePatterns { + if strings.Contains(lower, pattern) { + return true + } + } + return false +} + +// anyRealPendingRename reports whether entries (the raw string list from +// PendingFileRenameOperations) contains anything other than routine +// noise -- see isRoutinePendingRename. +func anyRealPendingRename(entries []string) bool { + for _, v := range entries { + if !isRoutinePendingRename(v) { + return true + } + } + return false +} diff --git a/internal/checker/windows/reboot_parse_test.go b/internal/checker/windows/reboot_parse_test.go new file mode 100644 index 0000000..45a69bb --- /dev/null +++ b/internal/checker/windows/reboot_parse_test.go @@ -0,0 +1,79 @@ +package windows + +import "testing" + +func TestIsRoutinePendingRename(t *testing.T) { + tests := []struct { + name string + entry string + want bool + }{ + {name: "empty (delete pair's second half)", entry: "", want: true}, + { + name: "gaming services proxy dll, confirmed live", + entry: `\??\C:\Windows\System32\gamingservicesproxy_13.dll.0`, + want: true, + }, + { + name: "edge updater temp exe, confirmed live", + entry: `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704\old_msedge.exe`, + want: true, + }, + { + name: "edge updater temp dir, confirmed live", + entry: `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704`, + want: true, + }, + { + name: "case-insensitive match", + entry: `\??\C:\PROGRAM FILES (X86)\MICROSOFT\EDGE\TEMP\foo.tmp`, + want: true, + }, + { + name: "windows installer rollback file -- a real pending change", + entry: `\??\C:\Config.Msi\561b8f4a.rbf`, + want: false, + }, + { + name: "onedrive updater -- a real pending change", + entry: `\??\C:\Program Files\Microsoft OneDrive\Update\OneDriveSetup.exe`, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isRoutinePendingRename(tt.entry); got != tt.want { + t.Errorf("isRoutinePendingRename(%q) = %v, want %v", tt.entry, got, tt.want) + } + }) + } +} + +func TestAnyRealPendingRename(t *testing.T) { + // Confirmed live on a real host: only routine noise queued -- must + // not report a real pending change. + onlyNoise := []string{ + `\??\C:\Windows\System32\gamingservicesproxy_13.dll.0`, + ``, + `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704\old_msedge.exe`, + ``, + `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704`, + ``, + } + if anyRealPendingRename(onlyNoise) { + t.Error("expected only-routine-noise entries to report no real pending rename") + } + + // A real pending change mixed in among routine noise must still be + // caught -- confirmed intent: when in doubt, err toward reporting + // reboot-required. + withReal := append(append([]string(nil), onlyNoise...), + `\??\C:\Config.Msi\561b8f4a.rbf`, ``) + if !anyRealPendingRename(withReal) { + t.Error("expected a real pending rename mixed in with noise to still be reported") + } + + if anyRealPendingRename(nil) { + t.Error("expected an empty list to report no pending rename") + } +} From 49023f730c6918af7cc1bcb94c8079350e34ff25 Mon Sep 17 00:00:00 2001 From: Winarto Date: Sun, 13 Sep 2026 14:32:21 +0800 Subject: [PATCH 7/7] windows: fix reboot-noise pattern missing the bare Edge Temp folder entry 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. --- internal/checker/windows/reboot_parse.go | 6 +++++- internal/checker/windows/reboot_parse_test.go | 20 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/internal/checker/windows/reboot_parse.go b/internal/checker/windows/reboot_parse.go index 4b13270..84871c4 100644 --- a/internal/checker/windows/reboot_parse.go +++ b/internal/checker/windows/reboot_parse.go @@ -18,7 +18,11 @@ import "strings" // reboot-required, never toward hiding one. var pendingRenameNoisePatterns = []string{ `\gamingservicesproxy`, - `\microsoft\edge\temp\`, + // No trailing backslash -- confirmed live, Edge's updater queues the + // bare Temp *folder* itself as one of the pending entries (not just + // files nested inside it), which a `...\temp\` pattern would miss + // entirely and report as a real pending change. + `\microsoft\edge\temp`, } // isRoutinePendingRename reports whether entry (one raw string from diff --git a/internal/checker/windows/reboot_parse_test.go b/internal/checker/windows/reboot_parse_test.go index 45a69bb..6e80c39 100644 --- a/internal/checker/windows/reboot_parse_test.go +++ b/internal/checker/windows/reboot_parse_test.go @@ -24,6 +24,16 @@ func TestIsRoutinePendingRename(t *testing.T) { entry: `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704`, want: true, }, + { + // Regression: this exact bare-folder entry (no trailing + // backslash/filename) is what a `...\temp\` pattern (with a + // trailing backslash) missed -- confirmed live, it kept + // "Reboot required" stuck true even with the rest of this + // ignore-list already in place. + name: "edge updater temp folder itself (no trailing separator), confirmed live", + entry: `\??\C:\Program Files (x86)\Microsoft\Edge\Temp`, + want: true, + }, { name: "case-insensitive match", entry: `\??\C:\PROGRAM FILES (X86)\MICROSOFT\EDGE\TEMP\foo.tmp`, @@ -50,15 +60,15 @@ func TestIsRoutinePendingRename(t *testing.T) { } func TestAnyRealPendingRename(t *testing.T) { - // Confirmed live on a real host: only routine noise queued -- must - // not report a real pending change. + // Confirmed live on a real host (the exact 4-entry value read back + // via `reg query` after upgrading to the fix that was supposed to + // silence this, and still showed reboot-required stuck true): only + // routine noise queued -- must not report a real pending change. onlyNoise := []string{ `\??\C:\Windows\System32\gamingservicesproxy_13.dll.0`, - ``, `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704\old_msedge.exe`, - ``, `\??\C:\Program Files (x86)\Microsoft\Edge\Temp\20476_794646704`, - ``, + `\??\C:\Program Files (x86)\Microsoft\Edge\Temp`, } if anyRealPendingRename(onlyNoise) { t.Error("expected only-routine-noise entries to report no real pending rename")