recheck: stream live output (verbose real command output or narration) - #3
Merged
Merged
Conversation
Force recheck previously did nothing visible: the agent's ActionRecheck handling was fire-and-forget (queue a signal, report success unconditionally) and never opened the same output stream Apply/Upgrade already use. It now runs synchronously and streams to the same live console, whether or not a companion is installed on that host -- the agent always holds its own dedicated stream for non-companion actions like recheck, so one fix covers both cases. A per-row "verbose" checkbox controls what streams: checked taps the real apt-get/apt-check/winget/powershell output (via a new optional checker.WithLineSink context hook, wired through aptutil and each platform backend with zero behavior change when absent); unchecked (default) emits short synthetic progress narration instead. OutputHub now buffers each in-flight action's lines so a browser reconnecting mid-recheck (e.g. a page refresh) replays what it missed before continuing live -- scoped to in-flight only, nothing persisted once an action ends. Also: promoted internal/companion's private lineTee into a shared internal/linetee package (checker's backends need the same line-splitting tee, and companion already imports checker, so checker can't import companion back); exported OutputSink.Push so the agent binary can push into a sink it owns directly. Deleted internal/aggregator/adminbus.go, an unused, superseded-by-OutputHub pub/sub left over from an earlier attempt at this same problem.
They fake apt-get as a #!/bin/sh script on PATH, which windows-latest's exec.LookPath can't run (no shebang support, PATHEXT-based lookup expects a known executable extension) -- confirmed by CI failing on exactly this. apt-get itself is Linux-only in practice anyway, matching the same //go:build !windows already on the equivalent ubuntu/debian checker tests added alongside these.
…ver set) Confirmed on real Ubuntu, Windows, and WSL hosts: Force recheck opened the live-output pane but never showed anything, verbose or not, on any platform. Root cause: CompanionHub.Push's agentStreams branch (the one that routes recheck, since it doesn't require a companion) pushed the action but never recorded it in any in-flight map. handleCompanionOutput authorizes every output-stream POST against that same tracking (previously just `pending`), so the agent's own StreamOutput call for a recheck was rejected with 409 on its very first request -- silently, since the failure only ever logged on the agent side, never surfaced to the browser. Added a separate agentPending map (recheck must never be blocked by, or clobber the tracking of, an already in-flight companion action on the same host -- Push deliberately skips the ErrActionInFlight check for it) and a new IsPending(agentID, actionID) that checks both maps, used by handleCompanionOutput instead of comparing against the single-value Pending() getter (which stays as-is for its own separate "what to resume watching after a page reload" purpose).
Confirmed on a real Windows host: applying a winget-sourced package via the companion (always running as a Windows Service, so stdin is never a real console) failed with "ERROR: Input redirection is not supported, exiting the process immediately." -- a winget error that --accept-package-agreements/--accept-source-agreements alone don't prevent. The detection side's own winget invocation (internal/checker/windows/packages.go) already passes --disable-interactivity and has no such problem; the apply side's three winget commands (Packages, Upgrade, and FullUpgrade via Upgrade) were missing it.
The companion binary reported the self-update-of-itself action's result *before* running Apply at all (Linux), specifically to guarantee an outcome got recorded even though install.sh's own systemctl restart kills this very process partway through. That report ends the action's output stream (OutputHub.End fires on the first /companion/result, and Publish becomes a no-op afterward for that action ID) -- so the live pane always closed itself off before install.sh had produced a single line, on every self-update-of-companion, regardless of how long the process actually survived first. Apply now always runs first, synchronously, exactly like every other action -- its real output streams normally for however long the process survives. The existing (and separately confirmed-live) rule that a failure coinciding with ctx already being canceled is spurious (the process is mid-restart, not actually broken) now substitutes the optimistic message only in that specific case, after the fact, instead of assuming it unconditionally upfront. This also let the Windows-only branch collapse away entirely: Apply already returns a real result there (no restart of this process happens on that platform at all), so both platforms now share one code path.
README's intro said the agent "never applies" updates -- true of the agent process itself, but no longer true of the project as a whole now that the companion can apply on trigger. Rewrote the intro to say that plainly, and to state clearly that the agent (update-detector) and the aggregator (update-aggregator) are two separate Docker images, not one -- the old "ships as a single Docker image" line was ambiguous now that the paragraph covers all three components. Also: winget is not supported by this project (confirmed) -- it never runs under install.bat's default LocalSystem service account in the first place. Reframed every winget mention in docs/reference.md's Platform limitations from "optional/supplementary signal" to "not supported, here's why," and updated both docs to reflect that detection, install.bat, Force recheck streaming, and companion self-update have now been confirmed against a real Windows host this session.
Windows companion self-update is two actions, not one: the companion stages a new binary (ActionSelfUpdate), then the agent stops the service/swaps the binary/restarts it (ActionCompleteCompanionSwap, auto-pushed on a staged success). CompleteCompanionSwap already called emitFromContext throughout (stopping/swapping/starting narration) -- ready to stream -- but its caller in cmd/update-detector/main.go never attached a sink to its context at all, so there was nothing to stream to regardless. Wired it up with the same sink/StreamOutput pattern already used for recheck and apply. That alone wasn't enough, though: the browser's live-output pane closes itself the moment it sees "done" for the first (staged) action, before the swap action -- now streaming correctly -- even starts, so the second phase would still never be seen without a manual page reload. OutputHub's "done" event now carries whether the underlying result was staged, and the admin page's JS re-subscribes to keep watching instead of switching to version-polling when it sees that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ActionRecheckhandling was fire-and-forget and never streamed anything. It now runs synchronously and streams to the same live console Apply/Upgrade already use, regardless of whether a companion is installed on that host.OutputHubnow buffers each in-flight action's lines) -- scoped to in-flight only, nothing persisted once an action ends.internal/aggregator/adminbus.go, an unused, superseded-by-OutputHubpub/sub left over from an earlier attempt at this same problem.internal/companion's private line-tee writer into a sharedinternal/lineteepackage sointernal/checker's backends can reuse it without an import cycle, and exportedOutputSink.Pushso the agent binary can push into a sink it owns directly.Test plan
go build ./.../go vet ./...on Linux andGOOS=windowslocally -- clean.go test ./...locally -- all green except two pre-existing, unrelated Unix-socket-path failures specific to this sandbox (internal/companion'sTestFetchIdentityWithRetry*,internal/companiontoken'sTestListenRemovesStaleSocket) -- not touched by this change.writeFakeAptGetpattern) verify the real-output tee actually receives a fake command's lines, with and without a sink attached, ininternal/checker/ubuntu,internal/checker/debian, andinternal/aptutil.OutputHubtests cover backlog replay on subscribe, backlog cleared onEnd, and backlog reset on a newBegin.ubuntu-latest+windows-latestmatrix) -- pending on this PR.