Windows reports boot time as now minus uptime, which drifts by a second or so
between reports. A static report sent in the grace period before the reboot
could read as a changed boot and mark a server still owing a reboot as failed.
- no dispatch in the last 15 minutes of a window; no-result timeout from dispatch time
- per-server output moves to patch_run_outputs (16MB document limit)
- reboot proven by a changed boot time; RebootTimeout 45m, ResultGrace 20m
- window update and delete are server-scoped against the policies using them
- scheduler puts the claim back on an error after it, so the next tick retries
- cancelled runs with failures alert; MCP apply_updates audits per server
- apply-updates 503 body documented; openapi regenerated
- web: cleared numeric fields no longer save as 0; Run now asks for confirmation
Fix round 1 review findings on the patch run service:
- advanceRun no longer runs every server's dispatch claim and failed-send
reset on the caller's shared short context; each gets its own fresh
patchCtx(), and a failed reset write is logged instead of discarded.
- The dispatch claim (queued/waiting_offline -> patching) now also requires
the run to still be status running with no cancelled_at, closing a race
where a tick that loaded the run just before CancelPatchRun wrote
cancelled_at could still dispatch.
- RecordPatchResult's write is now guarded on command_id too, so a late
result for a superseded command cannot land on a re-dispatched attempt.
Named patch_window.go (not patch_windows.go) since the _windows.go
suffix is Go's implicit GOOS build constraint and would silently
exclude the file on non-Windows builds.
Implements the patchrun package with a pure functional state machine for managing
patch runs. Contains no database dependencies - the services layer loads a run,
asks this package what should change, and writes changes guarded by expected status.
All 14 test cases pass, covering:
- Agent version parsing and support detection
- Concurrency limits and queueing
- Window deadlines and offline handling
- Result timeouts (ResultGrace, ManualTimeout, RebootTimeout)
- Reboot verification with boot time proof
- Run finalization logic
- Summary generation for alerts
The previous tests reimplemented the target-scope rule instead of calling
validateWorkflowTargetScope, so they proved nothing about CreateWorkflow
or UpdateWorkflow's actual enforcement. Split the check into a pure
decideWorkflowTargetScope (tested directly, no database) and a thin
wrapper behind an overridable listServersForScope seam, so tests can
invoke the real CreateWorkflow/UpdateWorkflow without a live database and
fail if the call sites are removed.
Close the time-of-write/time-of-fire gap: a restricted caller could
previously save target_tags matching no server today (a selector aimed
at hosts not yet provisioned or not yet tagged), pass validation on an
empty set, and have the scheduler fire on those hosts the moment they
appeared. Now a restricted caller specifying targets that resolve to
nothing is refused with the same message as an out-of-scope match; a
workflow with no targets at all, and an unrestricted caller, are
unaffected.
A database error while resolving the fleet now surfaces as an error
instead of folding into a pass.
Correct three comments that overstated what the code does: the
create/update route comment now mentions the tag-scope check, not only
validateTargetServers; the schedule route comment explains its safety
holds only for workflows written after this check existed, not for rows
already in the database under the old rule.
CreateWorkflow and UpdateWorkflow validated TargetServerIDs against the
caller's scope but never checked TargetTags, letting a restricted token
save a workflow with an empty ID list and an out-of-scope tag selector,
then reach those hosts once the scheduler fires it as the system with no
restriction of its own. Add validateWorkflowTargetScope, applying the
same all-or-nothing rule the MCP create_workflow tool already enforces:
resolve the full target set unscoped and scoped, refuse unless they
match. Update the PUT /api/workflows/:id/schedule fleetWide comment to
say why it is safe now (targets are constrained at write time) rather
than repeating the falsified claim that scheduling reaches nothing new.
TestServerScopeMapCoversEveryScopedRoute wrongly required serverScopedRoutes
to be a subset of routeScopes, which only covers the authenticated /api
group. The ESO route is registered on the root router and is exempt from
routeScopes by design, so it failed the test despite being correctly
declared. Lift the hardcoded exception in AssertScopeMapComplete into a
named routesOutsideAPIGroup set that both scopes.go and the test read.
GET /api/monitors and GET /api/monitors/:id returned Monitor.Runner
unfiltered; for an agent-pushed monitor that field is literally a server
ID, so a restricted token learned which out-of-scope server a monitor
runs on directly, not merely that one exists. services.RedactMonitorRunner
replaces Runner with models.RunnerRestricted when it names a server
outside the caller's scope, resolved once via the new
services.VisibleServerIDs rather than per monitor. The monitor itself is
still returned — a restricted operator may legitimately need to see that
it exists and is up or down — only the runner field goes neutral; omitting
the monitor entirely was considered and rejected as more surprising than
one field changing. Runner "server" (control-plane-run) is never
touched. The MCP list_monitors/get_monitor_status projections never had a
Runner field to begin with, so REST and the tool surface already agreed;
a comment now records why.
GET /api/workflows and GET /api/workflows/:id returned
Workflow.TargetServerIDs unfiltered — directly naming out-of-scope
servers, worse than a count. services.FilterVisibleServerIDs narrows the
list to what VisibleServerIDs admits and reports hidden (no count) when
at least one target was dropped; WorkflowResponse wraps *models.Workflow
with a scoped TargetServerIDs and a TargetsRestricted flag. TargetTags is
left untouched — the tag vocabulary is already ruled acceptable to
expose. The MCP list_workflows/get_workflow tools get the identical
treatment: list_workflows' target count is now based on the filtered ID
list, and get_workflow's workflowDetail carries the same
TargetsRestricted flag, so a model that sees a filtered target list and
then has run_workflow refuse the same workflow for out-of-scope targets
is not left concluding the refusal invented a problem the list never
mentioned.
All four routes recorded in serverScopedRoutes as true; none is
boot-enforced, for the same substring-filter reason as the key routes
added in the previous round.
mrhid6