diff --git a/docs/superpowers/plans/2026-08-13-windows-agent-parity.md b/docs/superpowers/plans/2026-08-13-windows-agent-parity.md new file mode 100644 index 0000000..7e282e4 --- /dev/null +++ b/docs/superpowers/plans/2026-08-13-windows-agent-parity.md @@ -0,0 +1,1913 @@ +# Windows Agent Parity Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give the Windows agent working OS update check/apply and a working workload registry (services and containers, with control and logs), matching what the Linux agent already does. + +**Architecture:** The platform split moves into the agent as Go build tags, following the existing `inventory/collect_linux.go` / `collect_windows.go` / `collect_other.go` pattern. Windows work is done by PowerShell scripts invoked through a small `winexec` helper; every script emits JSON, and the JSON parsers live in build-tag-free files so they are testable on a Linux development machine. The control plane stays OS-blind — a Windows service is reported as the same `unit` kind a systemd service is — so the only wire change in the whole project is one new `reboot_required` field on `InventoryReport`. + +**Tech Stack:** Go 1.26 (agent is its own module, `agent/go.mod`), PowerShell 5.1 (`powershell.exe`, present on every supported Windows), Windows Update COM (`Microsoft.Update.Session`), CIM (`Win32_Service`), `Get-WinEvent`, Next.js 16 + Tailwind for `web/`. + +## Global Constraints + +- **`proto/vantage/v1/vantage.proto` is documentation, not a generator input.** Both `pb` packages are hand-written. A message field added anywhere must be added to `proto/vantage/v1/vantage.proto`, `agent/internal/grpc/pb/`, and `server/internal/grpc/pb/` **in the same commit**. +- **`agent/` is a separate Go module** with an `internal/` tree. It cannot import `server/` or `shared/`, and constants shared with the server (like `MaxWorkloadLogLines`) are mirrored by hand. +- **No component in `web/` may carry a hex colour.** Use the existing Tailwind token classes (`text-warning`, `text-text-secondary`, `variant="warning"`, and so on). +- **PowerShell is always invoked `-NoProfile -NonInteractive`**, and always as `powershell.exe` (not `pwsh`) for anything touching Windows Update COM. +- **The agent never reboots a host.** It reports `reboot_required` and stops there. +- Build-tag files: `_linux.go` / `_windows.go` suffixes carry implicit constraints; any "everything else" file needs an explicit `//go:build !linux && !windows` line, because `_other` is not a GOOS suffix. +- Parsers must live in files **without** a platform suffix or build tag, so `go test ./...` exercises them on Linux. +- Run `go build ./...` **and** `GOOS=windows go build ./...` from `agent/` at the end of every agent task. A build-tag split is exactly the change that compiles on one platform and nowhere else. + +--- + +### Task 1: `winexec` — running PowerShell from the agent + +**Files:** +- Create: `agent/internal/winexec/encode.go` +- Create: `agent/internal/winexec/encode_test.go` +- Create: `agent/internal/winexec/run_windows.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `winexec.EncodeCommand(script string) string` (base64 of UTF-16LE, used by the runner and directly testable); `winexec.Run(ctx context.Context, script string) (string, error)` — Windows-only, returns the script's stdout. + +Scripts are passed with `-EncodedCommand` rather than `-Command` or a temp `.ps1` file. `-Command` requires quoting a multi-line script through Go, `cmd.exe` and PowerShell's own parser, and every one of the scripts in this plan contains both quote characters. A temp file needs a writable path and cleanup on a host where the agent may be killed mid-run. + +- [ ] **Step 1: Write the failing test** + +Create `agent/internal/winexec/encode_test.go`: + +```go +package winexec + +import "testing" + +func TestEncodeCommand(t *testing.T) { + // "hi" as UTF-16LE is 68 00 69 00, which base64-encodes to aABpAA==. + if got := EncodeCommand("hi"); got != "aABpAA==" { + t.Fatalf("EncodeCommand(hi) = %q, want aABpAA==", got) + } +} + +func TestEncodeCommandMultiline(t *testing.T) { + // Only that it round-trips through the same encoding PowerShell expects: + // every ASCII byte followed by a zero byte, no BOM. + got := EncodeCommand("a\nb") + want := "YQAKAGIA" + if got != want { + t.Fatalf("EncodeCommand = %q, want %q", got, want) + } +} +``` + +- [ ] **Step 2: Run the test and confirm it fails** + +```bash +cd agent && go test ./internal/winexec/ -run TestEncodeCommand -v +``` + +Expected: FAIL — `undefined: EncodeCommand`. + +- [ ] **Step 3: Write the implementation** + +Create `agent/internal/winexec/encode.go`: + +```go +// Package winexec runs PowerShell on Windows hosts. +// +// It exists because three subsystems — updates, workload collection and +// workload logs — all need the same invocation, and because getting a +// multi-line script past Go quoting, cmd.exe quoting and PowerShell's own +// parser is a problem worth solving once. +package winexec + +import ( + "encoding/base64" + "unicode/utf16" +) + +// EncodeCommand renders a script for powershell.exe -EncodedCommand: UTF-16LE, +// no byte-order mark, base64. This is deliberately free of build tags so it is +// tested on a Linux development machine like every other pure function here. +func EncodeCommand(script string) string { + units := utf16.Encode([]rune(script)) + b := make([]byte, 0, len(units)*2) + for _, u := range units { + b = append(b, byte(u), byte(u>>8)) + } + return base64.StdEncoding.EncodeToString(b) +} +``` + +- [ ] **Step 4: Run the test and confirm it passes** + +```bash +cd agent && go test ./internal/winexec/ -v +``` + +Expected: PASS, both tests. + +- [ ] **Step 5: Write the Windows runner** + +Create `agent/internal/winexec/run_windows.go`: + +```go +package winexec + +import ( + "context" + "fmt" + "os/exec" + "strings" +) + +// Run executes a PowerShell script and returns its stdout. +// +// powershell.exe rather than pwsh: everything this agent runs through here +// touches Windows Update COM or CIM, both of which are most reliable under +// Windows PowerShell 5.1, and 5.1 is present on every supported Windows while +// pwsh is an optional install. +func Run(ctx context.Context, script string) (string, error) { + cmd := exec.CommandContext(ctx, "powershell.exe", + "-NoProfile", "-NonInteractive", "-EncodedCommand", EncodeCommand(script)) + + out, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + return "", fmt.Errorf("powershell: %s", strings.TrimSpace(string(ee.Stderr))) + } + if ctx.Err() == context.DeadlineExceeded { + return "", fmt.Errorf("powershell: timed out") + } + return "", fmt.Errorf("powershell: %w", err) + } + return string(out), nil +} +``` + +- [ ] **Step 6: Verify both platforms build** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... +``` + +Expected: both succeed with no output. + +- [ ] **Step 7: Commit** + +```bash +git add agent/internal/winexec/ +git commit -m "feat: Add winexec helper for running PowerShell from the agent" +``` + +--- + +### Task 2: Split the updates package by build tag + +No behaviour changes on Linux. This task only moves the existing code behind a platform boundary so Task 3 has somewhere to land. + +**Files:** +- Modify: `agent/internal/updates/updates.go` (reduced to the shared surface) +- Create: `agent/internal/updates/updates_linux.go` (everything that is there now) +- Create: `agent/internal/updates/updates_other.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: unchanged public surface `updates.PackageUpdate{Name, CurrentVersion, NewVersion string}`, `updates.CheckAvailable() ([]PackageUpdate, error)`, `updates.ApplyAll() error`; new `updates.RebootRequired() bool`. Each dispatches to unexported `checkAvailable()`, `applyAll()`, `rebootRequired()` supplied per platform. + +- [ ] **Step 1: Reduce `updates.go` to the shared surface** + +Replace the whole contents of `agent/internal/updates/updates.go` with: + +```go +package updates + +// PackageUpdate is one pending update. On Linux it is a package with a version +// on each side. On Windows CurrentVersion is empty and NewVersion carries the +// KB article ID: a Windows update is not a version bump of a named package, +// and inventing a current version would put a wrong string in front of an +// operator. +type PackageUpdate struct { + Name string + CurrentVersion string + NewVersion string +} + +// CheckAvailable lists pending OS updates. +func CheckAvailable() ([]PackageUpdate, error) { return checkAvailable() } + +// ApplyAll installs every pending update. It never reboots: a control plane +// silently restarting a production server is unrecoverable from the UI, so the +// reboot stays a decision a person or a workflow makes. RebootRequired reports +// when one is owed. +func ApplyAll() error { return applyAll() } + +// RebootRequired reports whether this host is waiting on a restart. +func RebootRequired() bool { return rebootRequired() } +``` + +- [ ] **Step 2: Move the Linux implementation into its own file** + +Create `agent/internal/updates/updates_linux.go` containing every function the old `updates.go` had — `detectPM`, `checkApt`, `checkDnfYum`, `checkPacman`, `checkZypper`, `checkApk`, `apkName`, `apkVersion` — verbatim, with its imports (`bufio`, `bytes`, `context`, `os/exec`, `strings`, `time`), plus these three entry points. `CheckAvailable`'s old body becomes `checkAvailable`; `ApplyAll`'s old body becomes `applyAll`: + +```go +package updates + +// (…existing detectPM, checkApt, checkDnfYum, checkPacman, checkZypper, +// checkApk, apkName, apkVersion moved here unchanged…) + +func checkAvailable() ([]PackageUpdate, error) { + switch detectPM() { + case "apt": + return checkApt() + case "dnf": + return checkDnfYum("dnf") + case "yum": + return checkDnfYum("yum") + case "pacman": + return checkPacman() + case "zypper": + return checkZypper() + case "apk": + return checkApk() + default: + return nil, nil + } +} + +func applyAll() error { + // (…the existing ApplyAll switch, unchanged…) +} + +// rebootRequired reads what the distributions themselves record. Debian and +// Ubuntu drop a file; the RPM family answers through needs-restarting, whose +// exit code is 1 when a reboot is owed and 0 when it is not. +func rebootRequired() bool { + if _, err := os.Stat("/var/run/reboot-required"); err == nil { + return true + } + if _, err := exec.LookPath("dnf"); err == nil { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "dnf", "needs-restarting", "-r").Run(); err != nil { + if ee, ok := err.(*exec.ExitError); ok && ee.ExitCode() == 1 { + return true + } + } + } + return false +} +``` + +Add `"os"` to the import block for `os.Stat`. + +- [ ] **Step 3: Add the fallback platform file** + +Create `agent/internal/updates/updates_other.go`: + +```go +//go:build !linux && !windows + +// The build constraint above is load-bearing: "_other" is not a GOOS suffix, so +// without it this file compiles on Linux too and collides with updates_linux.go. +package updates + +func checkAvailable() ([]PackageUpdate, error) { return nil, nil } +func applyAll() error { return nil } +func rebootRequired() bool { return false } +``` + +- [ ] **Step 4: Verify both platforms build** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... +``` + +Expected: the Linux build succeeds. The Windows build **fails** with `undefined: checkAvailable` — Task 3 supplies it. Confirm the failure names exactly those three functions and nothing else; anything else means something was moved wrong. + +- [ ] **Step 5: Commit** + +```bash +git add agent/internal/updates/ +git commit -m "refactor: Split the agent updates package by build tag" +``` + +--- + +### Task 3: Windows Update check, apply and reboot detection + +**Files:** +- Create: `agent/internal/updates/winparse.go` +- Create: `agent/internal/updates/winparse_test.go` +- Create: `agent/internal/updates/updates_windows.go` + +**Interfaces:** +- Consumes: `winexec.Run(ctx, script) (string, error)` from Task 1; `PackageUpdate` from Task 2. +- Produces: the `checkAvailable`/`applyAll`/`rebootRequired` trio for `GOOS=windows`, plus `parseUpdateSearch(jsonText string) ([]PackageUpdate, error)`. + +- [ ] **Step 1: Write the failing parser test** + +Create `agent/internal/updates/winparse_test.go`: + +```go +package updates + +import "testing" + +func TestParseUpdateSearchArray(t *testing.T) { + in := `[{"title":"2026-08 Cumulative Update for Windows Server 2022","kb":"5034123"}, + {"title":"Windows Malicious Software Removal Tool","kb":"890830"}]` + + got, err := parseUpdateSearch(in) + if err != nil { + t.Fatalf("parseUpdateSearch: %v", err) + } + if len(got) != 2 { + t.Fatalf("got %d updates, want 2", len(got)) + } + if got[0].Name != "2026-08 Cumulative Update for Windows Server 2022" { + t.Errorf("Name = %q", got[0].Name) + } + if got[0].NewVersion != "KB5034123" { + t.Errorf("NewVersion = %q, want KB5034123", got[0].NewVersion) + } + if got[0].CurrentVersion != "" { + t.Errorf("CurrentVersion = %q, want empty", got[0].CurrentVersion) + } +} + +// PowerShell 5.1's ConvertTo-Json collapses a one-element array into a bare +// object. A host with exactly one pending update is common, and a parser that +// only accepts arrays reports it as zero. +func TestParseUpdateSearchSingleObject(t *testing.T) { + got, err := parseUpdateSearch(`{"title":"Security Intelligence Update","kb":"2267602"}`) + if err != nil { + t.Fatalf("parseUpdateSearch: %v", err) + } + if len(got) != 1 || got[0].NewVersion != "KB2267602" { + t.Fatalf("got %+v", got) + } +} + +func TestParseUpdateSearchNoKB(t *testing.T) { + got, err := parseUpdateSearch(`[{"title":"Driver update for Contoso NIC","kb":""}]`) + if err != nil { + t.Fatalf("parseUpdateSearch: %v", err) + } + if len(got) != 1 || got[0].NewVersion != "" { + t.Fatalf("got %+v, want one update with an empty NewVersion", got) + } +} + +// An empty result set is "nothing pending", not a parse failure. +func TestParseUpdateSearchEmpty(t *testing.T) { + for _, in := range []string{"", " \r\n", "[]", "null"} { + got, err := parseUpdateSearch(in) + if err != nil { + t.Fatalf("parseUpdateSearch(%q): %v", in, err) + } + if len(got) != 0 { + t.Fatalf("parseUpdateSearch(%q) = %+v, want none", in, got) + } + } +} + +// A KB already carrying its prefix must not become KBKB5034123. +func TestParseUpdateSearchPrefixedKB(t *testing.T) { + got, _ := parseUpdateSearch(`[{"title":"x","kb":"KB5034123"}]`) + if got[0].NewVersion != "KB5034123" { + t.Fatalf("NewVersion = %q", got[0].NewVersion) + } +} +``` + +- [ ] **Step 2: Run the test and confirm it fails** + +```bash +cd agent && go test ./internal/updates/ -v +``` + +Expected: FAIL — `undefined: parseUpdateSearch`. + +- [ ] **Step 3: Write the parser** + +Create `agent/internal/updates/winparse.go`: + +```go +package updates + +import ( + "encoding/json" + "strings" +) + +// winUpdate is one row of the Windows Update searcher's output, in the shape +// searchScript emits it. +type winUpdate struct { + Title string `json:"title"` + KB string `json:"kb"` +} + +// parseUpdateSearch reads the searcher's JSON. +// +// It carries no build tag on purpose: this is the half of the Windows update +// path that can be tested on a development machine, and the agent module has no +// Windows CI. +func parseUpdateSearch(jsonText string) ([]PackageUpdate, error) { + s := strings.TrimSpace(jsonText) + if s == "" || s == "null" { + return nil, nil + } + + var rows []winUpdate + if err := json.Unmarshal([]byte(s), &rows); err != nil { + // ConvertTo-Json renders a one-element array as a bare object. + var one winUpdate + if err2 := json.Unmarshal([]byte(s), &one); err2 != nil { + return nil, err + } + rows = []winUpdate{one} + } + + out := make([]PackageUpdate, 0, len(rows)) + for _, r := range rows { + u := PackageUpdate{Name: r.Title} + if kb := strings.TrimSpace(r.KB); kb != "" { + if !strings.HasPrefix(strings.ToUpper(kb), "KB") { + kb = "KB" + kb + } + u.NewVersion = kb + } + out = append(out, u) + } + return out, nil +} +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +```bash +cd agent && go test ./internal/updates/ -v +``` + +Expected: PASS, all five tests. + +- [ ] **Step 5: Write the Windows implementation** + +Create `agent/internal/updates/updates_windows.go`: + +```go +package updates + +import ( + "context" + "fmt" + "strings" + "time" + + "gitea.hostxtra.co.uk/mrhid6/vantage/agent/internal/winexec" +) + +const ( + // The first search after a boot contacts Microsoft Update (or WSUS) and is + // routinely slow. Ten minutes is not generous, it is realistic. + searchTimeout = 10 * time.Minute + + // A patch-Tuesday cumulative genuinely takes this long to download and + // install on a modest server. + applyTimeout = 60 * time.Minute + + rebootTimeout = 2 * time.Minute +) + +// The Windows Update COM API is used rather than the PSWindowsUpdate module: it +// is present on every supported Windows, needs no PowerShell Gallery install, +// and works unchanged against a WSUS server on an air-gapped fleet. The agent +// runs as LocalSystem, which holds the rights it requires. +const searchScript = ` +$ErrorActionPreference = 'Stop' +$searcher = (New-Object -ComObject Microsoft.Update.Session).CreateUpdateSearcher() +$result = $searcher.Search("IsInstalled=0 and Type='Software' and IsHidden=0") +$rows = @() +foreach ($u in $result.Updates) { + $ids = @($u.KBArticleIDs) + $kb = '' + if ($ids.Count -gt 0) { $kb = [string]$ids[0] } + $rows += [pscustomobject]@{ title = [string]$u.Title; kb = $kb } +} +ConvertTo-Json -InputObject @($rows) -Depth 3 -Compress +` + +const applyScript = ` +$ErrorActionPreference = 'Stop' +$session = New-Object -ComObject Microsoft.Update.Session +$result = $session.CreateUpdateSearcher().Search("IsInstalled=0 and Type='Software' and IsHidden=0") + +$batch = New-Object -ComObject Microsoft.Update.UpdateColl +foreach ($u in $result.Updates) { + if ($u.InstallationBehavior.CanRequestUserInput) { continue } + if (-not $u.EulaAccepted) { + try { $u.AcceptEula() } catch { continue } + } + $null = $batch.Add($u) +} + +if ($batch.Count -eq 0) { Write-Output 'nothing-to-install'; exit 0 } + +$downloader = $session.CreateUpdateDownloader() +$downloader.Updates = $batch +$null = $downloader.Download() + +$installer = $session.CreateUpdateInstaller() +$installer.Updates = $batch +$r = $installer.Install() + +Write-Output ('resultcode=' + $r.ResultCode) +# 2 = succeeded, 3 = succeeded with errors. Anything else failed, and this +# process must exit non-zero so the agent logs a failure rather than an ack. +if ($r.ResultCode -ne 2 -and $r.ResultCode -ne 3) { exit 1 } +exit 0 +` + +const rebootScript = ` +$ErrorActionPreference = 'SilentlyContinue' +$si = New-Object -ComObject Microsoft.Update.SystemInfo +if ($si.RebootRequired) { Write-Output 'true'; exit 0 } +$keys = @( + 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootPending', + 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired' +) +foreach ($k in $keys) { if (Test-Path $k) { Write-Output 'true'; exit 0 } } +$sm = Get-ItemProperty 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager' -Name PendingFileRenameOperations +if ($sm -and $sm.PendingFileRenameOperations) { Write-Output 'true'; exit 0 } +Write-Output 'false' +` + +func checkAvailable() ([]PackageUpdate, error) { + ctx, cancel := context.WithTimeout(context.Background(), searchTimeout) + defer cancel() + + out, err := winexec.Run(ctx, searchScript) + if err != nil { + return nil, fmt.Errorf("windows update search: %w", err) + } + return parseUpdateSearch(out) +} + +func applyAll() error { + ctx, cancel := context.WithTimeout(context.Background(), applyTimeout) + defer cancel() + + if _, err := winexec.Run(ctx, applyScript); err != nil { + return fmt.Errorf("windows update install: %w", err) + } + return nil +} + +func rebootRequired() bool { + ctx, cancel := context.WithTimeout(context.Background(), rebootTimeout) + defer cancel() + + out, err := winexec.Run(ctx, rebootScript) + if err != nil { + return false + } + return strings.TrimSpace(out) == "true" +} +``` + +- [ ] **Step 6: Verify both platforms build and tests pass** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... && go test ./... +``` + +Expected: all three succeed. The Windows build no longer reports undefined symbols. + +- [ ] **Step 7: Commit** + +```bash +git add agent/internal/updates/ +git commit -m "feat: Check and apply Windows updates through the Windows Update COM API" +``` + +--- + +### Task 4: `reboot_required` end to end (wire, agent, server) + +**Files:** +- Modify: `proto/vantage/v1/vantage.proto:161-171` +- Modify: `agent/internal/grpc/pb/vantage.pb.go:121-131` +- Modify: `server/internal/grpc/pb/vantage.pb.go` (the `InventoryReport` struct) +- Modify: `agent/internal/sync/sync.go:441-471` (`runInventory`) +- Modify: `server/internal/services/inventory.go:27-46` +- Modify: `server/internal/models/server.go:35-44` + +**Interfaces:** +- Consumes: `updates.RebootRequired() bool` from Tasks 2 and 3. +- Produces: `InventoryReport.RebootRequired bool` on the wire (JSON key `reboot_required`); `models.Inventory.RebootRequired bool` (bson and JSON key `reboot_required`) for Task 8's UI. + +- [ ] **Step 1: Add the field to the proto** + +In `proto/vantage/v1/vantage.proto`, extend `InventoryReport`: + +```protobuf +message InventoryReport { + string server_id = 1; + string agent_token = 2; + bool include_static = 3; + CPUReport cpu = 4; + MemReport memory = 5; + uint64 swap_total = 6; + uint64 swap_used = 7; + repeated PartitionReport partitions = 8; + string kernel = 9; + // Set on static snapshots only. The agent never reboots; it reports that one + // is owed and leaves the decision to a person or a workflow. + bool reboot_required = 10; +} +``` + +- [ ] **Step 2: Add the field to both hand-written pb copies** + +In **both** `agent/internal/grpc/pb/vantage.pb.go` and `server/internal/grpc/pb/vantage.pb.go`, add the last line to `InventoryReport`: + +```go +type InventoryReport struct { + ServerId string `json:"server_id"` + AgentToken string `json:"agent_token"` + IncludeStatic bool `json:"include_static"` + CPU *CPUReport `json:"cpu,omitempty"` + Memory *MemReport `json:"memory,omitempty"` + SwapTotal uint64 `json:"swap_total"` + SwapUsed uint64 `json:"swap_used"` + Partitions []PartitionReport `json:"partitions,omitempty"` + Kernel string `json:"kernel,omitempty"` + RebootRequired bool `json:"reboot_required,omitempty"` +} +``` + +- [ ] **Step 3: Set it in the agent's inventory loop** + +In `agent/internal/sync/sync.go`, change `runInventory`'s `report` closure so the flag is computed only for static snapshots: + +```go + report := func(static bool) { + r := inventory.Collect(static) + r.ServerId = cfg.ServerID + r.AgentToken = cfg.AgentToken + // Static snapshots only — every 15 minutes, not every 30 seconds. On + // Windows this spawns a PowerShell process, which is not something to + // do twice a minute forever, and a host rebooted by hand clearing the + // flag within a quarter of an hour is soon enough. + // + // Computed here rather than inside inventory.Collect so the inventory + // package gains no dependency on updates. + if static { + r.RebootRequired = updates.RebootRequired() + } + if err := client.ReportInventory(r); err != nil { + log.Printf("report inventory: %v", err) + } + } +``` + +`updates` is already imported by `sync.go`, so the import block needs no change. + +- [ ] **Step 4: Store it on the server** + +In `server/internal/services/inventory.go`, inside the `if r.IncludeStatic {` branch, beside the existing `set["inventory.kernel"] = r.Kernel`: + +```go + set["inventory.reboot_required"] = r.RebootRequired +``` + +In `server/internal/models/server.go`, add the field to `Inventory` after `Kernel`: + +```go + RebootRequired bool `bson:"reboot_required,omitempty" json:"reboot_required,omitempty"` +``` + +- [ ] **Step 5: Build everything** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... +cd ../server && go build ./... +``` + +Expected: all succeed. + +- [ ] **Step 6: Verify against a running Linux agent** + +On any Linux host with the agent installed, create the marker Debian and Ubuntu use, wait for the next static snapshot (up to 15 minutes), and read the stored document: + +```bash +sudo touch /var/run/reboot-required +# then, against the control plane's MongoDB: +# db.servers.findOne({server_id:""}, {"inventory.reboot_required":1}) +``` + +Expected: `reboot_required: true`. Remove the file afterwards. + +- [ ] **Step 7: Commit** + +```bash +git add proto/vantage/v1/vantage.proto agent/internal/grpc/pb/vantage.pb.go \ + server/internal/grpc/pb/vantage.pb.go agent/internal/sync/sync.go \ + server/internal/services/inventory.go server/internal/models/server.go +git commit -m "feat: Report whether a managed host is waiting on a reboot" +``` + +--- + +### Task 5: Split the workloads package by build tag + +No behaviour change on Linux; this opens the Windows path, which Tasks 6 and 7 fill. + +**Files:** +- Modify: `agent/internal/workloads/workloads.go:22-38` +- Rename: `agent/internal/workloads/systemd.go` → `agent/internal/workloads/systemd_linux.go` +- Modify: `agent/internal/workloads/control.go` (shared half only) +- Create: `agent/internal/workloads/control_linux.go` +- Modify: `agent/internal/workloads/logs.go` (shared half only) +- Create: `agent/internal/workloads/logs_linux.go` +- Create: `agent/internal/workloads/units_other.go` +- Modify: `agent/internal/sync/workloads.go:20-24,44-47` + +**Interfaces:** +- Consumes: `Workload`, `errText` from the unchanged `docker.go`. +- Produces, for every platform file to supply: `collectUnits(ctx context.Context) ([]Workload, bool, string)`, `controlPlatform(ctx context.Context, kind, id, action string) error`, `logsPlatform(ctx context.Context, kind, id string, tail int) (string, error)`, `isProtectedUnit(id, name string) bool`, and `var ownContainerID string`. + +- [ ] **Step 1: Make `Collect` platform-blind** + +In `agent/internal/workloads/workloads.go`, drop the `runtime` import and replace the guard: + +```go +// Collect enumerates every workload on this host: containers from Docker, and +// units from systemd on Linux or the service control manager on Windows. +func Collect(ctx context.Context) Result { + var r Result + containers, dockerOK, dockerErr := collectDocker(ctx) + units, systemdOK, systemdErr := collectUnits(ctx) + + r.DockerOK, r.DockerError = dockerOK, dockerErr + r.SystemdOK, r.SystemdError = systemdOK, systemdErr + r.Workloads = append(append([]Workload{}, containers...), units...) + + markProtected(r.Workloads) + return r +} +``` + +The `SystemdOK` / `SystemdError` names stay as they are. A Windows service is reported as the same `unit` kind, and renaming these would cost a proto change, both pb copies, the server model, the service layer and the web client — to describe the same thing. The naming is corrected where it is read, in the UI, which knows the server's OS. + +- [ ] **Step 2: Rename the systemd collector and its entry point** + +```bash +git mv agent/internal/workloads/systemd.go agent/internal/workloads/systemd_linux.go +``` + +In the renamed file, rename `collectSystemd` to `collectUnits`. Its body, `excludedPrefixes` and `excluded` are unchanged. + +- [ ] **Step 3: Split `control.go`** + +`agent/internal/workloads/control.go` keeps only what is platform-independent: + +```go +package workloads + +import ( + "context" + "errors" + "fmt" + "strings" + "time" +) + +// ErrProtected is returned for a workload the agent will not act on. +var ErrProtected = errors.New("workload is protected") + +// controlTimeout bounds a stop that may never finish on its own. `docker stop` +// waits on a container that may ignore SIGTERM, and both systemctl and +// Stop-Service block for as long as the unit's own stop timeout says. A timeout +// must return a real error rather than an ack implying success. +const controlTimeout = 90 * time.Second + +// isProtected reports whether the agent refuses to act on this workload. +// +// The refusal lives here, in the agent, and not in the control plane. As with +// the console relay hardcoding 127.0.0.1 agent-side: the control plane may name +// a target, but the agent decides what it will do to itself. A server-side +// denylist alone would be bypassed by the next dispatch path someone adds. +func isProtected(kind, id, name string) bool { + if kind == "unit" { + return isProtectedUnit(id, name) + } + if ownContainerID == "" { + return false + } + // Container IDs are commonly abbreviated to 12 characters; compare on the + // shorter of the two so a short id still matches a full one. + return strings.HasPrefix(ownContainerID, id) || strings.HasPrefix(id, ownContainerID) +} + +// markProtected stamps the flag onto a collected list so the UI can render the +// action disabled with a reason. +func markProtected(wls []Workload) { + for i := range wls { + wls[i].Protected = isProtected(wls[i].Kind, wls[i].ID, wls[i].Name) + } +} + +// Control starts, stops or restarts a workload. +func Control(ctx context.Context, kind, id, action string) error { + switch action { + case "start", "stop", "restart": + default: + return fmt.Errorf("unknown action %q", action) + } + + // Checked before anything else happens, and checked here rather than only + // on the server. See isProtected. + if isProtected(kind, id, strings.TrimSuffix(id, ".service")) { + return fmt.Errorf("%w: %s", ErrProtected, id) + } + + ctx, cancel := context.WithTimeout(ctx, controlTimeout) + defer cancel() + + return controlPlatform(ctx, kind, id, action) +} +``` + +Create `agent/internal/workloads/control_linux.go` with the platform half: + +```go +package workloads + +import ( + "context" + "fmt" + "os" + "os/exec" + "regexp" + "strings" +) + +// AgentUnit is the systemd unit this agent runs as. +const AgentUnit = "vantage-agent.service" + +// ownContainerID is read once: the container this agent runs in, if any. +var ownContainerID = detectOwnContainer() + +var cgroupContainerRe = regexp.MustCompile(`[0-9a-f]{64}`) + +// detectOwnContainer returns this process's container ID, or "" on a host +// install. The agent is normally a systemd service, so "" is the common case; +// this exists so containerising it later cannot silently remove the guard. +func detectOwnContainer() string { + b, err := os.ReadFile("/proc/self/cgroup") + if err != nil { + return "" + } + if m := cgroupContainerRe.FindString(string(b)); m != "" { + return m + } + return "" +} + +func isProtectedUnit(id, name string) bool { + return id == AgentUnit || name == strings.TrimSuffix(AgentUnit, ".service") +} + +func controlPlatform(ctx context.Context, kind, id, action string) error { + var cmd *exec.Cmd + switch kind { + case "container": + cmd = exec.CommandContext(ctx, "docker", action, id) + case "unit": + cmd = exec.CommandContext(ctx, "systemctl", action, id) + default: + return fmt.Errorf("unknown workload kind %q", kind) + } + + if out, err := cmd.CombinedOutput(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("%s %s timed out after %s", action, id, controlTimeout) + } + return fmt.Errorf("%s %s: %s", action, id, strings.TrimSpace(string(out))) + } + return nil +} +``` + +- [ ] **Step 4: Split `logs.go`** + +`agent/internal/workloads/logs.go` keeps the caps, the doc comment, `Logs` and `capLog`; the command construction moves out: + +```go +package workloads + +import ( + "context" + "strings" + "time" +) + +const ( + // MaxLogLines and MaxLogBytes are BOTH enforced, whichever binds first. + // + // A line count alone does not bound size: 500 lines of a container printing + // 4KB JSON blobs is 2MB travelling over the bus. This is the same reasoning + // that gave workflow logs a per-line cap as well as a per-run one. + MaxLogLines = 500 + MaxLogBytes = 256 * 1024 + + logTimeout = 60 * time.Second +) + +// Logs returns a bounded snapshot of a workload's recent output. +// +// There is no follow mode. The browser console already offers a real terminal +// on the same server where `docker logs -f` works properly, with its own +// scrollback and cancellation. A snapshot answers "why did this restart", +// which is the question that sends people to the console in the first place. +func Logs(ctx context.Context, kind, id string, tail int) (string, bool, error) { + if tail <= 0 || tail > MaxLogLines { + tail = MaxLogLines + } + + ctx, cancel := context.WithTimeout(ctx, logTimeout) + defer cancel() + + out, err := logsPlatform(ctx, kind, id, tail) + if err != nil { + return "", false, err + } + + text, truncated := capLog(out) + return text, truncated, nil +} + +// (capLog unchanged) +``` + +Create `agent/internal/workloads/logs_linux.go`: + +```go +package workloads + +import ( + "context" + "fmt" + "os/exec" + "strconv" +) + +func logsPlatform(ctx context.Context, kind, id string, tail int) (string, error) { + var cmd *exec.Cmd + switch kind { + case "container": + cmd = exec.CommandContext(ctx, "docker", "logs", + "--tail", strconv.Itoa(tail), "--timestamps", id) + case "unit": + cmd = exec.CommandContext(ctx, "journalctl", "-u", id, + "-n", strconv.Itoa(tail), "--no-pager", "--output=short-iso") + default: + return "", fmt.Errorf("unknown workload kind %q", kind) + } + + // docker logs writes container stderr to our stderr, so both streams must + // be captured or half the output silently disappears. + out, err := cmd.CombinedOutput() + if err != nil && len(out) == 0 { + return "", fmt.Errorf("read logs for %s: %s", id, errText(err)) + } + return string(out), nil +} +``` + +- [ ] **Step 5: Add the fallback platform file** + +Create `agent/internal/workloads/units_other.go`: + +```go +//go:build !linux && !windows + +// The build constraint is load-bearing — see updates_other.go. +package workloads + +import ( + "context" + "fmt" +) + +var ownContainerID = "" + +func collectUnits(context.Context) ([]Workload, bool, string) { return nil, false, "" } +func isProtectedUnit(string, string) bool { return false } + +func controlPlatform(context.Context, string, string, string) error { + return fmt.Errorf("workload control is not supported on this platform") +} + +func logsPlatform(context.Context, string, string, int) (string, error) { + return "", fmt.Errorf("workload logs are not supported on this platform") +} +``` + +- [ ] **Step 6: Remove the Linux gates from the reporting loop** + +In `agent/internal/sync/workloads.go`, delete all three `runtime.GOOS != "linux"` early returns — the two at the top of `runWorkloads` and `reportWorkloads` — and drop the now-unused `runtime` import. + +- [ ] **Step 7: Verify both platforms build** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... +``` + +Expected: the Linux build succeeds. The Windows build fails with `undefined: collectUnits`, `undefined: controlPlatform`, `undefined: logsPlatform`, `undefined: isProtectedUnit`, `undefined: ownContainerID` — and nothing else. Tasks 6 and 7 supply them. + +- [ ] **Step 8: Commit** + +```bash +git add agent/internal/workloads/ agent/internal/sync/workloads.go +git commit -m "refactor: Split the agent workloads package by build tag" +``` + +--- + +### Task 6: Collect Windows services as workloads + +**Files:** +- Create: `agent/internal/workloads/winparse.go` +- Create: `agent/internal/workloads/winparse_test.go` +- Create: `agent/internal/workloads/services_windows.go` + +**Interfaces:** +- Consumes: `Workload` from `docker.go`; `winexec.Run` from Task 1; the `collectUnits` signature from Task 5. +- Produces: `parseServices(jsonText, systemRoot string) ([]Workload, error)`, `servicePath(pathName string) string`, `psQuote(s string) string` — all build-tag-free — and `collectUnits` for `GOOS=windows`. + +- [ ] **Step 1: Write the failing tests** + +Create `agent/internal/workloads/winparse_test.go`: + +```go +package workloads + +import "testing" + +func TestServicePath(t *testing.T) { + cases := []struct{ in, want string }{ + {`"C:\Program Files\Contoso\svc.exe" -service`, `C:\Program Files\Contoso\svc.exe`}, + {`C:\WINDOWS\system32\svchost.exe -k netsvcs`, `C:\WINDOWS\system32\svchost.exe`}, + {`C:\Vantage\vantage-agent.exe`, `C:\Vantage\vantage-agent.exe`}, + {`"C:\no\args.exe"`, `C:\no\args.exe`}, + {``, ``}, + } + for _, c := range cases { + if got := servicePath(c.in); got != c.want { + t.Errorf("servicePath(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseServicesFilters(t *testing.T) { + in := `[ + {"Name":"Contoso","DisplayName":"Contoso Broker","State":"Running","StartMode":"Auto","PathName":"\"C:\\Program Files\\Contoso\\svc.exe\" -service","ExitCode":0}, + {"Name":"Themes","DisplayName":"Themes","State":"Running","StartMode":"Auto","PathName":"C:\\WINDOWS\\system32\\svchost.exe -k netsvcs","ExitCode":0}, + {"Name":"Fabrikam","DisplayName":"Fabrikam Sync","State":"Stopped","StartMode":"Auto","PathName":"C:\\Fabrikam\\sync.exe","ExitCode":0}, + {"Name":"Northwind","DisplayName":"Northwind Poller","State":"Stopped","StartMode":"Manual","PathName":"C:\\Northwind\\poll.exe","ExitCode":0}, + {"Name":"Crashed","DisplayName":"Crashed Thing","State":"Stopped","StartMode":"Auto","PathName":"C:\\Crashed\\c.exe","ExitCode":1067} + ]` + + got, err := parseServices(in, `C:\WINDOWS`) + if err != nil { + t.Fatalf("parseServices: %v", err) + } + + byID := map[string]Workload{} + for _, w := range got { + byID[w.ID] = w + } + + // The OS's own svchost service is dropped; a manual, stopped, never-failed + // service is nobody's business either. + if _, ok := byID["Themes"]; ok { + t.Error("Themes (under %SystemRoot%) should be filtered out") + } + if _, ok := byID["Northwind"]; ok { + t.Error("stopped Manual service should be filtered out") + } + if len(got) != 3 { + t.Fatalf("got %d workloads, want 3: %+v", len(got), got) + } + + if w := byID["Contoso"]; w.Kind != "unit" || w.Name != "Contoso Broker" || w.State != "running" { + t.Errorf("Contoso = %+v", w) + } + // Enabled but not running is exactly the row worth seeing. + if byID["Fabrikam"].State != "stopped" { + t.Errorf("Fabrikam state = %q, want stopped", byID["Fabrikam"].State) + } + // A non-zero exit code on a stopped service is a crash, not a clean stop. + if byID["Crashed"].State != "failed" { + t.Errorf("Crashed state = %q, want failed", byID["Crashed"].State) + } +} + +// 1077 means "no attempt to start since boot" — a clean stopped service, not a +// failure, and reporting it red would cry wolf on every host. +func TestParseServicesExitCode1077(t *testing.T) { + in := `[{"Name":"Idle","DisplayName":"Idle","State":"Stopped","StartMode":"Auto","PathName":"C:\\Idle\\i.exe","ExitCode":1077}]` + got, err := parseServices(in, `C:\WINDOWS`) + if err != nil { + t.Fatalf("parseServices: %v", err) + } + if len(got) != 1 || got[0].State != "stopped" { + t.Fatalf("got %+v, want one stopped workload", got) + } +} + +func TestParseServicesSingleObjectAndEmpty(t *testing.T) { + one := `{"Name":"Solo","DisplayName":"Solo","State":"Running","StartMode":"Auto","PathName":"C:\\Solo\\s.exe","ExitCode":0}` + got, err := parseServices(one, `C:\WINDOWS`) + if err != nil || len(got) != 1 { + t.Fatalf("single object: got %+v, err %v", got, err) + } + + for _, in := range []string{"", "[]", "null"} { + got, err := parseServices(in, `C:\WINDOWS`) + if err != nil || len(got) != 0 { + t.Fatalf("parseServices(%q) = %+v, err %v", in, got, err) + } + } +} + +func TestPSQuote(t *testing.T) { + if got := psQuote(`it's`); got != `'it''s'` { + t.Fatalf("psQuote = %s", got) + } + if got := psQuote(`plain`); got != `'plain'` { + t.Fatalf("psQuote = %s", got) + } +} +``` + +- [ ] **Step 2: Run the tests and confirm they fail** + +```bash +cd agent && go test ./internal/workloads/ -v +``` + +Expected: FAIL — `undefined: servicePath`, `undefined: parseServices`, `undefined: psQuote`. + +- [ ] **Step 3: Write the parser** + +Create `agent/internal/workloads/winparse.go`: + +```go +package workloads + +import ( + "encoding/json" + "strings" +) + +// winService is one row of Get-CimInstance Win32_Service. +// +// Win32_Service rather than Get-Service: Get-Service exposes neither PathName +// nor StartMode, and the filter below needs both. +type winService struct { + Name string `json:"Name"` + DisplayName string `json:"DisplayName"` + State string `json:"State"` + StartMode string `json:"StartMode"` + PathName string `json:"PathName"` + ExitCode int `json:"ExitCode"` +} + +// exitCodeNeverStarted is ERROR_SERVICE_NEVER_STARTED. A stopped service +// carrying it has not failed — it has not run since boot — and painting that +// red would cry wolf on every host. +const exitCodeNeverStarted = 1077 + +// servicePath extracts the executable from a Win32_Service PathName. +// +// A naive split on whitespace misfiles a substantial share of a real fleet: +// `"C:\Program Files\X\x.exe" -service` is one path and one argument. +func servicePath(pathName string) string { + s := strings.TrimSpace(pathName) + if s == "" { + return "" + } + if s[0] == '"' { + if end := strings.IndexByte(s[1:], '"'); end >= 0 { + return s[1 : 1+end] + } + return strings.TrimPrefix(s, `"`) + } + if i := strings.Index(strings.ToLower(s), ".exe"); i >= 0 { + return s[:i+len(".exe")] + } + if i := strings.IndexAny(s, " \t"); i >= 0 { + return s[:i] + } + return s +} + +// parseServices turns the collector's JSON into workloads. +// +// systemRoot is a parameter rather than an environment read so this is testable +// off Windows. The caller passes %SystemRoot%. +// +// The filter mirrors the systemd collector's intent: show what an operator +// installed, and show what is meant to be up but is not. Services under +// %SystemRoot%\System32 are the platform's own, and a typical host has well +// over a hundred of them. +func parseServices(jsonText, systemRoot string) ([]Workload, error) { + s := strings.TrimSpace(jsonText) + if s == "" || s == "null" { + return nil, nil + } + + var rows []winService + if err := json.Unmarshal([]byte(s), &rows); err != nil { + var one winService + if err2 := json.Unmarshal([]byte(s), &one); err2 != nil { + return nil, err + } + rows = []winService{one} + } + + sys32 := strings.ToLower(strings.TrimRight(systemRoot, `\`) + `\system32\`) + + var wls []Workload + for _, r := range rows { + if p := strings.ToLower(servicePath(r.PathName)); p != "" && strings.HasPrefix(p, sys32) { + continue + } + + running := strings.EqualFold(r.State, "Running") + failed := !running && r.ExitCode != 0 && r.ExitCode != exitCodeNeverStarted + auto := strings.HasPrefix(strings.ToLower(r.StartMode), "auto") + if !running && !failed && !auto { + continue + } + + state := "stopped" + switch { + case running: + state = "running" + case failed: + state = "failed" + } + + name := r.DisplayName + if name == "" { + name = r.Name + } + + wls = append(wls, Workload{ + Kind: "unit", + ID: r.Name, + Name: name, + State: state, + }) + } + return wls, nil +} + +// psQuote renders a Go string as a PowerShell single-quoted literal. Single +// quotes suppress every form of expansion, so the only character needing an +// escape is the quote itself, which is doubled. +func psQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "''") + "'" +} +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +```bash +cd agent && go test ./internal/workloads/ -v +``` + +Expected: PASS, all five tests. + +- [ ] **Step 5: Write the Windows collector** + +Create `agent/internal/workloads/services_windows.go`: + +```go +package workloads + +import ( + "context" + "os" + "time" + + "gitea.hostxtra.co.uk/mrhid6/vantage/agent/internal/winexec" +) + +const servicesTimeout = 60 * time.Second + +const servicesScript = ` +$ErrorActionPreference = 'Stop' +$svcs = Get-CimInstance Win32_Service | + Select-Object Name,DisplayName,State,StartMode,PathName,ExitCode +ConvertTo-Json -InputObject @($svcs) -Depth 3 -Compress +` + +// collectUnits enumerates Windows services. The bool and string it returns are +// the same SystemdOK / SystemdError pair the Linux collector fills: the wire +// shape is shared, and the UI words it per platform. +func collectUnits(ctx context.Context) ([]Workload, bool, string) { + ctx, cancel := context.WithTimeout(ctx, servicesTimeout) + defer cancel() + + out, err := winexec.Run(ctx, servicesScript) + if err != nil { + return nil, false, "Win32_Service query failed: " + err.Error() + } + + systemRoot := os.Getenv("SystemRoot") + if systemRoot == "" { + systemRoot = `C:\Windows` + } + + wls, err := parseServices(out, systemRoot) + if err != nil { + return nil, false, "Win32_Service output could not be read: " + err.Error() + } + return wls, true, "" +} +``` + +- [ ] **Step 6: Verify the Linux build and tests still pass** + +```bash +cd agent && go build ./... && go test ./... +``` + +Expected: both succeed. `GOOS=windows go build ./...` still fails on `controlPlatform`, `logsPlatform`, `isProtectedUnit` and `ownContainerID`, which Task 7 supplies. + +- [ ] **Step 7: Commit** + +```bash +git add agent/internal/workloads/ +git commit -m "feat: Collect Windows services as workloads" +``` + +--- + +### Task 7: Windows workload control and logs + +**Files:** +- Create: `agent/internal/workloads/control_windows.go` +- Create: `agent/internal/workloads/logs_windows.go` +- Modify: `agent/internal/workloads/winparse.go` (add the event parser) +- Modify: `agent/internal/workloads/winparse_test.go` (add its tests) + +**Interfaces:** +- Consumes: `psQuote` from Task 6; `winexec.Run` from Task 1; `controlTimeout`, `MaxLogLines` from the shared files in Task 5. +- Produces: `controlPlatform`, `logsPlatform`, `isProtectedUnit`, `ownContainerID` for `GOOS=windows`; `parseEvents(jsonText, serviceName, displayName string) (string, error)`, build-tag-free. + +- [ ] **Step 1: Write the failing event-parser tests** + +Append to `agent/internal/workloads/winparse_test.go`: + +```go +func TestParseEventsFormatsAndOrders(t *testing.T) { + // Get-WinEvent returns newest first; journalctl --output=short-iso returns + // oldest first, and the log dialog and capLog's front-trim both assume the + // most recent line is at the bottom. + in := `[ + {"t":"2026-08-13T10:22:31.0000000Z","l":"Error","p":"Contoso","m":"broker died"}, + {"t":"2026-08-13T10:22:03.0000000Z","l":"Information","p":"Contoso","m":"broker starting"} + ]` + + got, err := parseEvents(in, "Contoso", "Contoso Broker") + if err != nil { + t.Fatalf("parseEvents: %v", err) + } + + want := "2026-08-13T10:22:03.0000000Z Information broker starting\n" + + "2026-08-13T10:22:31.0000000Z Error broker died" + if got != want { + t.Fatalf("parseEvents =\n%q\nwant\n%q", got, want) + } +} + +// Service Control Manager logs every service on the host under one provider, so +// its rows must be filtered down to the target or the log is somebody else's. +func TestParseEventsFiltersOtherServicesSCM(t *testing.T) { + in := `[ + {"t":"2026-08-13T10:00:00Z","l":"Information","p":"Service Control Manager","m":"The Print Spooler service entered the running state."}, + {"t":"2026-08-13T10:00:01Z","l":"Information","p":"Service Control Manager","m":"The Contoso Broker service entered the running state."} + ]` + + got, err := parseEvents(in, "Contoso", "Contoso Broker") + if err != nil { + t.Fatalf("parseEvents: %v", err) + } + if strings.Contains(got, "Print Spooler") { + t.Errorf("another service's SCM event leaked in:\n%s", got) + } + if !strings.Contains(got, "Contoso Broker") { + t.Errorf("the target's SCM event was dropped:\n%s", got) + } +} + +// A service that has logged nothing is normal. An error there would read as a +// broken feature. +func TestParseEventsEmpty(t *testing.T) { + for _, in := range []string{"", "[]", "null"} { + got, err := parseEvents(in, "Contoso", "Contoso Broker") + if err != nil || got != "" { + t.Fatalf("parseEvents(%q) = %q, err %v", in, got, err) + } + } +} +``` + +Add `"strings"` to that file's imports. + +- [ ] **Step 2: Run the tests and confirm they fail** + +```bash +cd agent && go test ./internal/workloads/ -run TestParseEvents -v +``` + +Expected: FAIL — `undefined: parseEvents`. + +- [ ] **Step 3: Write the event parser** + +Append to `agent/internal/workloads/winparse.go`: + +```go +// scmProvider is the provider every service's start and stop is logged under, +// host-wide. +const scmProvider = "Service Control Manager" + +type winEvent struct { + T string `json:"t"` + L string `json:"l"` + P string `json:"p"` + M string `json:"m"` +} + +// parseEvents renders Get-WinEvent output as text in the shape journalctl +// --output=short-iso produces, so the log dialog needs no per-platform +// rendering: " ", oldest first. +func parseEvents(jsonText, serviceName, displayName string) (string, error) { + s := strings.TrimSpace(jsonText) + if s == "" || s == "null" { + return "", nil + } + + var rows []winEvent + if err := json.Unmarshal([]byte(s), &rows); err != nil { + var one winEvent + if err2 := json.Unmarshal([]byte(s), &one); err2 != nil { + return "", err + } + rows = []winEvent{one} + } + + var lines []string + for _, e := range rows { + if strings.EqualFold(e.P, scmProvider) { + if !strings.Contains(e.M, serviceName) && + (displayName == "" || !strings.Contains(e.M, displayName)) { + continue + } + } + msg := strings.TrimSpace(strings.ReplaceAll(e.M, "\r\n", " ")) + lines = append(lines, e.T+" "+e.L+" "+msg) + } + + // Get-WinEvent is newest-first. Reverse it. + for i, j := 0, len(lines)-1; i < j; i, j = i+1, j-1 { + lines[i], lines[j] = lines[j], lines[i] + } + return strings.Join(lines, "\n"), nil +} +``` + +- [ ] **Step 4: Run the tests and confirm they pass** + +```bash +cd agent && go test ./internal/workloads/ -v +``` + +Expected: PASS, all eight tests in the package. + +- [ ] **Step 5: Write the Windows control half** + +Create `agent/internal/workloads/control_windows.go`: + +```go +package workloads + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "gitea.hostxtra.co.uk/mrhid6/vantage/agent/internal/winexec" +) + +// AgentUnit is the service this agent runs as — the NSSM service name written +// by installer/setup.ps1. Change one, change the other. +const AgentUnit = "VantageAgent" + +// A Windows agent is never itself in a container; the Linux build reads +// /proc/self/cgroup, and there is no equivalent question to ask here. +var ownContainerID = "" + +// Windows service names are case-insensitive, so the comparison must be too. +func isProtectedUnit(id, name string) bool { + return strings.EqualFold(id, AgentUnit) || strings.EqualFold(name, AgentUnit) +} + +func controlPlatform(ctx context.Context, kind, id, action string) error { + switch kind { + case "container": + // Docker behaves identically on Windows, so this path is shared in + // spirit with the Linux one rather than routed through PowerShell. + cmd := exec.CommandContext(ctx, "docker", action, id) + if out, err := cmd.CombinedOutput(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("%s %s timed out after %s", action, id, controlTimeout) + } + return fmt.Errorf("%s %s: %s", action, id, strings.TrimSpace(string(out))) + } + return nil + + case "unit": + // -Force is required: Stop-Service without it refuses outright when + // another service depends on the target, and that refusal reads to an + // operator as a silent no-op. + // + // sc.exe is avoided because it returns before the operation completes, + // which turns a timeout into a false success. + var verb string + switch action { + case "start": + verb = "Start-Service" + case "stop": + verb = "Stop-Service" + case "restart": + verb = "Restart-Service" + default: + return fmt.Errorf("unknown action %q", action) + } + + script := "$ErrorActionPreference='Stop'\n" + verb + " -Name " + psQuote(id) + if action != "start" { + script += " -Force" + } + + if _, err := winexec.Run(ctx, script); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("%s %s timed out after %s", action, id, controlTimeout) + } + return fmt.Errorf("%s %s: %w", action, id, err) + } + return nil + + default: + return fmt.Errorf("unknown workload kind %q", kind) + } +} +``` + +- [ ] **Step 6: Write the Windows logs half** + +Create `agent/internal/workloads/logs_windows.go`: + +```go +package workloads + +import ( + "context" + "fmt" + "os/exec" + "strconv" + + "gitea.hostxtra.co.uk/mrhid6/vantage/agent/internal/winexec" +) + +func logsPlatform(ctx context.Context, kind, id string, tail int) (string, error) { + switch kind { + case "container": + cmd := exec.CommandContext(ctx, "docker", "logs", + "--tail", strconv.Itoa(tail), "--timestamps", id) + out, err := cmd.CombinedOutput() + if err != nil && len(out) == 0 { + return "", fmt.Errorf("read logs for %s: %s", id, errText(err)) + } + return string(out), nil + + case "unit": + display := serviceDisplayName(ctx, id) + + // Timestamps are formatted PowerShell-side rather than left to + // ConvertTo-Json, whose DateTime rendering differs between PowerShell + // versions — one of them emits /Date(1699...)/. + // + // -ErrorAction SilentlyContinue because Get-WinEvent treats "no events + // matched" as a terminating error, and a quiet service is normal. + names := psQuote(id) + if display != "" && display != id { + names += "," + psQuote(display) + } + names += "," + psQuote(scmProvider) + + script := ` +$ErrorActionPreference = 'SilentlyContinue' +$rows = Get-WinEvent -FilterHashtable @{LogName='System','Application'; ProviderName=@(` + names + `)} ` + + `-MaxEvents ` + strconv.Itoa(tail) + ` | + ForEach-Object { + [pscustomobject]@{ + t = $_.TimeCreated.ToUniversalTime().ToString('o') + l = [string]$_.LevelDisplayName + p = [string]$_.ProviderName + m = [string]$_.Message + } + } +ConvertTo-Json -InputObject @($rows) -Depth 3 -Compress +` + + out, err := winexec.Run(ctx, script) + if err != nil { + return "", fmt.Errorf("read events for %s: %w", id, err) + } + return parseEvents(out, id, display) + + default: + return "", fmt.Errorf("unknown workload kind %q", kind) + } +} + +// serviceDisplayName resolves a service's display name, which is what Service +// Control Manager events name it by. An empty answer is fine — the filter then +// matches on the service name alone. +func serviceDisplayName(ctx context.Context, id string) string { + out, err := winexec.Run(ctx, + "$ErrorActionPreference='SilentlyContinue'\n"+ + "(Get-Service -Name "+psQuote(id)+").DisplayName") + if err != nil { + return "" + } + return trimLine(out) +} +``` + +Add `trimLine` to `winparse.go` (build-tag-free, so it is available to both platforms and testable): + +```go +// trimLine reduces single-value PowerShell output to its first non-empty line. +func trimLine(s string) string { + for _, l := range strings.Split(s, "\n") { + if t := strings.TrimSpace(l); t != "" { + return t + } + } + return "" +} +``` + +- [ ] **Step 7: Verify both platforms build and all tests pass** + +```bash +cd agent && go build ./... && GOOS=windows go build ./... && go test ./... +``` + +Expected: all three succeed. This is the first point in the plan where the Windows build is clean. + +- [ ] **Step 8: Commit** + +```bash +git add agent/internal/workloads/ +git commit -m "feat: Control Windows services and read their event log as workloads" +``` + +--- + +### Task 8: Web — Windows wording and the reboot badge + +**Files:** +- Modify: `web/lib/api.ts:10-19` (`Inventory`) +- Modify: `web/components/workloads/WorkloadList.tsx:48,102-138` +- Modify: `web/app/(app)/servers/[id]/page.tsx:304` (the `WorkloadList` call site) +- Modify: `web/components/servers/tabs/MaintenanceTab.tsx:45-94` + +**Interfaces:** +- Consumes: `models.Inventory.RebootRequired` from Task 4, serialised as `inventory.reboot_required`. +- Produces: `WorkloadList({ serverId, canControl, isWindows })`. + +Windows is detected with the same `os_info` test `MaintenanceTab.tsx` already uses. `os_type` is stored and serialised but unread by `web/` today, and introducing a second Windows test in the same component is how the two come to disagree. + +- [ ] **Step 1: Add the field to the client type** + +In `web/lib/api.ts`, extend `Inventory`: + +```ts +export interface Inventory { + cpu: { model?: string; cores?: number; usage_pct: number; load1?: number }; + memory: { total_bytes: number; used_bytes: number }; + swap_total_bytes: number; + swap_used_bytes: number; + partitions?: { device: string; mountpoint: string; fstype?: string; total_bytes: number; used_bytes: number }[]; + kernel?: string; + reboot_required?: boolean; + metrics_at?: string; + static_at?: string; +} +``` + +- [ ] **Step 2: Teach `WorkloadList` which platform it is describing** + +In `web/components/workloads/WorkloadList.tsx`, change the signature: + +```tsx +export function WorkloadList({ serverId, canControl, isWindows }: { serverId: string; canControl: boolean; isWindows: boolean }) { +``` + +and replace the empty state and the systemd status lines (currently lines 117–136): + +```tsx + ) : !data ? ( +

Nothing reported yet. Agents report every 60 seconds.

+ ) : ( +
+ {/* Docker absent is the common case on a fleet built + around SSH keys, and is not a fault. Installed but + not responding is a different problem, so it reads + differently. */} + {data.docker_error ? ( +

Docker is installed but not responding: {data.docker_error}

+ ) : !data.docker_ok ? ( +

Docker is not in use on this server.

+ ) : null} + + {/* One wire field, two honest words for it: the agent + reports Windows services under the same `unit` kind + systemd units use, and only the UI knows which host + this is. On Windows there is no "not in use" case — + every Windows host has a service controller — so a + failure is the only thing worth saying. */} + {data.systemd_error ? ( +

+ {isWindows ? "Windows services could not be read: " : "systemd could not be read: "} + {data.systemd_error} +

+ ) : !data.systemd_ok && !isWindows ? ( +

systemd is not in use on this server.

+ ) : null} +
+ )} +``` + +- [ ] **Step 3: Pass the flag from the server detail page** + +In `web/app/(app)/servers/[id]/page.tsx`, at the `WorkloadList` call site: + +```tsx + {activeTab === "workloads" && ( + + )} +``` + +- [ ] **Step 4: Reword the updates panel and add the reboot badge** + +In `web/components/servers/tabs/MaintenanceTab.tsx`, the `isWindows` constant already exists at line 42. Change the panel header and the empty state: + +```tsx +
+

OS updates

+
+ {updates.length > 0 ? {updates.length} pending : up to date} + {/* Sits with the updates panel because that is what + caused it. The agent never reboots a host itself. */} + {server.inventory?.reboot_required && reboot required} +
+
+ + {updates.length === 0 ? ( +

+ {isWindows ? "No pending Windows updates. The agent checks hourly." : "No pending package updates. The agent checks hourly."} +

+ ) : ( +``` + +and the table headings, since a Windows update has no current version and its identifier is a KB article: + +```tsx + + {isWindows ? "Update" : "Package"} + Current + {isWindows ? "KB" : "Available"} + +``` + +- [ ] **Step 5: Check it compiles and lints** + +```bash +cd web && npx tsc --noEmit && npm run lint +``` + +Expected: no errors. A missing `isWindows` prop at any other `WorkloadList` call site shows up here. + +- [ ] **Step 6: Commit** + +```bash +git add web/lib/api.ts web/components/workloads/WorkloadList.tsx \ + "web/app/(app)/servers/[id]/page.tsx" web/components/servers/tabs/MaintenanceTab.tsx +git commit -m "feat: Word the workload and update panels for Windows servers" +``` + +--- + +### Task 9: Documentation and manual verification on a Windows host + +**Files:** +- Modify: `CLAUDE.md` (the Windows second-class note and the workload section) +- Modify: `docsite/docs/reference/agent-config.md` (or the nearest Windows agent page) + +**Interfaces:** +- Consumes: everything above. +- Produces: nothing code depends on. + +- [ ] **Step 1: Verify on a real Windows host** + +Install or update the agent on a Windows server registered to a development control plane, then work through each of these and record the result: + +| Check | Expected | +| --- | --- | +| Wait one hour, or restart the agent service | The server's OS updates panel lists Windows updates by title with a `KB…` identifier | +| Press Apply updates | The panel empties within the hour; the agent's log shows `resultcode=2` or `resultcode=3` | +| After applying a cumulative update | A `reboot required` badge appears within 15 minutes | +| Open the Workloads tab | Non-Microsoft services are listed; `svchost`-hosted platform services are not | +| Stop, then start, a test service | Both succeed and the row's state follows within a couple of seconds | +| Restart `VantageAgent` from the UI | Refused with the protected message, HTTP 409 | +| Open logs on a service that writes events | Events oldest-first, timestamps ISO 8601 | +| Open logs on a silent service | Empty panel, no error | +| With Docker Desktop installed | Containers appear alongside services and can be restarted | +| Without Docker installed | "Docker is not in use on this server", not an error | + +- [ ] **Step 2: Update `CLAUDE.md`** + +In the Design Decisions list, replace the Windows line: + +```markdown +- **Windows agents cover the fleet-management path** — register, heartbeat, run + steps, report inventory, OS updates through the Windows Update COM API, and + workloads (services plus containers, with control and logs). They still do no + `authorized_keys` management, and no package inventory or CVE matching: the + vulnerability feeds this project uses carry no Windows data, so a Windows host + correctly reports `unsupported` rather than a clean bill of health. +``` + +In the "Workload registry" section, after the sentence beginning "A **workload** is one Docker container or one systemd unit", add: + +```markdown +On Windows a workload is a Docker container or a Windows **service**, reported +under the same `unit` kind and the same `systemd_ok` / `systemd_error` fields — +one wire shape, worded per platform in the UI, which is the only layer that +knows the host's OS. The platform split lives entirely in the agent, as build +tags (`systemd_linux.go` / `services_windows.go` and the matching `control_` +and `logs_` pairs); the control plane is OS-blind and needed no changes. +Windows collection runs PowerShell through `agent/internal/winexec`, and every +script emits JSON that a build-tag-free parser reads, so the parsers are tested +on Linux — the agent module has no Windows CI. +``` + +In the "Inventory and OS updates" section, add: + +```markdown +Windows update checking and applying go through the Windows Update COM API +(`Microsoft.Update.Session`) rather than the PSWindowsUpdate module, which would +need a PowerShell Gallery install on every host and fails on an air-gapped +fleet. `CurrentVersion` is empty on Windows and `NewVersion` carries the KB +article ID: a Windows update is not a version bump of a named package. + +**The agent never reboots a host.** `ApplyUpdatesCmd` installs and stops there; +`inventory.reboot_required` reports that one is owed, set on the static snapshot +every 15 minutes. Linux fills it too, from `/var/run/reboot-required` or +`dnf needs-restarting -r`. +``` + +- [ ] **Step 3: Update the docs site** + +In `docsite/docs/reference/agent-config.md`, wherever the Windows agent's capabilities are described, replace any "Windows agents register and heartbeat only" wording with the current list: register, heartbeat, inventory, workflow steps, console relay, OS updates and workloads; and note that SSH key management and vulnerability scanning remain Linux-only. + +- [ ] **Step 4: Commit** + +```bash +git add CLAUDE.md docsite/docs/reference/agent-config.md +git commit -m "docs: Describe the Windows agent's update and workload support" +``` diff --git a/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md b/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md index e340cae..d9877d2 100644 --- a/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md +++ b/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md @@ -115,9 +115,14 @@ A new field `reboot_required` on `InventoryReport`, added to (`agent/internal/grpc/pb`, `server/internal/grpc/pb`) in the same commit. It travels on the inventory report rather than the update report because it is a -host property like the kernel version, and inventory refreshes every 30 seconds -with a full static snapshot every 15 minutes — so a host rebooted by hand clears -the flag promptly instead of showing it for up to an hour. +host property like the kernel version, and it is set on the **static** snapshot +only — every 15 minutes rather than every 30 seconds. A host rebooted by hand +clears the flag in a quarter of an hour instead of showing it for up to a full +one, and the detection costs a PowerShell process on Windows, which is not +something to spawn twice a minute forever. + +It is set in `agentsync.runInventory`, not inside the `inventory` package, so +`inventory` gains no dependency on `updates`. Both platforms set it, since parity is free here: @@ -245,10 +250,15 @@ nothing is normal, and an error there would read as a broken feature. The server changes in one place: `services.ReportInventory` persists `reboot_required`. -The web changes in three, all keyed on `server.os_type`, which is already stored -on the server document and already serialised, but currently unread by `web/`: +The web changes in three, all keyed on the same `os_info` test +`MaintenanceTab.tsx` already uses (`server.os_info?.toLowerCase().includes("windows")`) +rather than on `os_type`. `os_type` is stored and serialised but unread by +`web/` today, and introducing a second Windows test in the same component is how +the two come to disagree. `WorkloadList` takes the result as a prop, since it +receives only a `serverId`: -1. `web/components/workloads/WorkloadList.tsx` — the systemd status lines become +1. `web/components/workloads/WorkloadList.tsx` — takes an `isWindows` prop from + the server detail page, and the systemd status lines become platform-worded. On Windows the error line reads "Windows services could not be read" and the "systemd is not in use on this server" line is not rendered at all. The empty-state line drops "on Linux only". The Docker lines are @@ -261,8 +271,10 @@ on the server document and already serialised, but currently unread by `web/`: ## Testing The Windows collectors are, in substance, parsers of PowerShell output. Parsing -is separated from invocation and table-tested against captured real output, -following `agent/internal/packages/parse.go`: +is separated from invocation and table-tested against captured real output. The +`agent` module has no tests at all today, so these are the first — they live +beside the parsers as ordinary `_test.go` files, run with `go test ./...` from +`agent/`, and need no new dependency: - `Win32_Service` JSON, including a quoted path with arguments, a `%SystemRoot%\System32` service that must be filtered out, a stopped