Add node-swap: one reusable Windows fleet-node binary swap tool - #476
Merged
Merged
Conversation
Root-cause fix for the 2026-09-24 Aorus outage (02:56-05:30): the 0.140.8 swap script's restart-and-verify phase ran inside an interactive SSH session, and Windows OpenSSH kills the whole remote process tree on client disconnect. The script had already swapped the binary but never reached its restart/verify/rollback lines when the session dropped. `local-offload node-swap` (internal/nodeswap/) replaces the family of hand-adapted per-deploy scripts (aorus-swap-<sha>.ps1, deploy-node-exe.ps1, fleet-node-restart.ps1 stitched together each release) with one engine: verify hash -> wait idle -> backup+rename (diagnosing and clearing only an idle MCP-helper holder on a rename failure, never a live server) -> install -> optional render-tree swap -> restart -> verify PID+image-sha256+health within a timeout -> automatic rollback on any failure from the render swap onward. Every OS operation sits behind a Deps seam, so the full sequence including every rollback branch is unit-tested with fakes. setup/windows-node-swap-launch.ps1 launches it via Win32_Process.Create (never Start-Process, which dies with the SSH session) so a run survives the launching connection dropping and still reaches its own rollback path. Tested: go test ./... green on GOOS=linux (CI) and cross-compiled for GOOS=windows; a dry run and the real swap exercised live on the Qube against a scratch fleet node, including a genuinely killed ssh client mid-flight (the node came up on the swapped binary, proven by new PID + image sha256 + /fleet/health, entirely after the ssh client process was gone). The rollback path is covered by deterministic unit tests (restart-failure and post-restart-verify-failure both roll back and re-verify the old binary); a live rollback demo on the scratch node hit an environment-specific hang tied to the throwaway test binary/script, not the shipped restart mechanism (see the PR description). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Addresses the pr-review-toolkit:code-reviewer findings on PR #476. BLOCKING: a failure at the backup-old or install-new step happened AFTER stop-node had already stopped the node, but neither path routed through rollback() or ever restarted it -- the node was left DOWN, the exact outage class this tool exists to prevent (a Windows file lock is a realistic trigger for backup-old; a staged file vanishing/corrupting for install-new). Both steps now route through rollback(), which is extended to handle the case where nothing was ever moved (Target still holds the original binary untouched): it skips the file restore and goes straight to restart+verify. Two new tests (TestRun_BackupOldFailureRestartsOldBinary, TestRun_InstallNewFailureRestartsOldBinary) pin this, confirmed red against the pre-fix code before being made green. Docs (FLEET-NODE.md, docs/systems/node-swap.md) corrected to describe the real coverage (rollback from step 4 onward, not "the render swap onward"). SHOULD-FIX: windows-node-swap-launch.ps1's Quote-Arg doubled embedded `"` (PowerShell/cmd convention), but the receiving process is a Go binary parsing argv via the CommandLineToArgvW/MSVCRT convention, which needs `\"` plus the backslash-run-before-a-quote doubling rule. A --restart-command wrapping a quoted inner PowerShell command silently corrupted argv under the old scheme. Replaced with Quote-ArgForNativeArgv, a verbatim port of Go's syscall.EscapeArg, verified with a -SelfTest mode that round-trips its output through the REAL Win32 CommandLineToArgvW API (not just hand-computed expected strings) -- run live on the Qube and wired into CI's installer-windows job via the new setup/windows-node-swap-launch.tests.ps1. SHOULD-FIX: a flag-parsing or --log-open failure in node_swap_cmd.go used to exit with neither --log nor --result written -- indistinguishable to a detached poller from "still running". runNodeSwap now pre-scans args for --result/--log before the real parse and writes a minimal failure Outcome to whatever paths it found on either failure path. Two new tests (TestRunNodeSwap_BadFlagStillWritesAResult, TestRunNodeSwap_UnopenableLogStillWritesAResult), confirmed red without the fix. The launcher also gained a defense-in-depth liveness check after Win32_Process.Create. Also added TestRun_RollbackRestoreItselfFails, asserting RollbackOK surfaces false rather than a silent false recovery when the rollback's own file restore fails. Tested: go test ./... green on GOOS=linux and cross-compiled for GOOS=windows; go vet clean on both; the argv self-test run live. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
Root-cause fix for the 2026-09-24 Aorus outage (down 02:56-05:30): the 0.140.8 swap
script's restart-and-verify phase ran inside an interactive SSH session. Windows
OpenSSH kills the whole remote process tree on client disconnect, so when the
session dropped at the 120s mark the script had already swapped the binary but
never reached its
schtasks /run/ health-verify / rollback lines. A second hazardthe same day: an idle
offload-harness.exe mcpprocess on the OptiPlex held anOS-level handle on the live exe and silently blocked
Move-Item/Rename-Item.local-offload node-swap(internal/nodeswap/, wired innode_swap_cmd.go)is the one reusable engine every future Windows fleet-node deploy calls, replacing
the family of hand-adapted per-release scripts (
aorus-swap-<sha>.ps1,deploy-node-exe.ps1,fleet-node-restart.ps1stitched together each time).Sequence: verify staged sha256 → wait idle (
/fleet/healthqueue 0) → stop thenode → backup-rename the old exe (on failure, diagnose the holder via CIM and
clear ONLY an idle MCP-helper process sharing the exe, never a live server) →
install the new exe → optional render-tree swap (tarball + backup + extract +
hash-verify) → restart (scheduled task or an arbitrary restart command) → prove
it by PID + running-image sha256 + health within a timeout → automatic
rollback (restore backup, restart, re-verify) on any failure from the render
swap onward.
setup/windows-node-swap-launch.ps1launches it viaWin32_Process.Create(never
Start-Process, which dies with the SSH session) so a run survives thelaunching connection dropping and keeps writing
--log/--resultand stillreaches its own rollback path.
docs/systems/node-swap.md(full system doc) + pointers fromdocs/FLEET-NODE.md,docs/README.md,AGENTS.md.Every OS operation (hash, health read, CIM process enumeration, rename, tar.gz
extraction,
Start/Stop-ScheduledTask) sits behind aDepsfunction-value seam,so the whole state machine — every rollback branch included — is unit-tested with
fakes, no real Windows box required for CI (which runs on
ubuntu-latest).How tested
go build ./.../go vet ./.../go test ./...green onGOOS=linux(matchesCI) and cross-compiled clean for
GOOS=windows.internal/nodeswap: 20+ unit tests covering the happy path, hash mismatch,never-idle timeout, rename-retry (idle MCP holder stopped vs. a live fleet-serve
holder left alone), restart failure → rollback, post-restart-verify failure →
rollback, render-tree swap rolled back on a later failure, standalone-node
hash-only verification, plus the real cross-platform primitives (hashing,
/fleet/healthdecoding incl. the pre-0.100.0queue_depthfallback, tar.gzextraction incl. a path-escape refusal).
node_swap_cmd_test.go: CLI flag parsing.a scratch fleet-serve node (own port, own dir — the real production Qube/Aorus
fleet nodes were never touched).
--dry-runverified live (touched nothing).Then the real swap, launched over
ssh localhostthroughwindows-node-swap-launch.ps1, with the ssh client process killed by PID(SIGKILL) 3 seconds after launch — well before the swap's own multi-step
sequence (7.4s total) finished. The node came up on the swapped binary anyway:
new PID, image sha256 matching the staged exe,
/fleet/healthansweringcorrectly — all confirmed over a fresh connection after the kill.
docs/systems/node-swap.md's testing-notes section and the PR conversation havethe full transcript.
failure rolls back, restarts again, and re-verifies the old binary — including
when that first restart attempt fails and the SECOND, rollback-triggered restart
attempt succeeds). A live-Windows rollback demo against the same scratch node hit
an environment-specific hang traced to the throwaway test's own
Start-Process-based restart script (a Go
exec.Cmdstdout/stderr pipe-inheritance hang when achild spawns a long-lived grandchild that isn't fully detached) — not the shipped
Win32_Process.Create-based restart pattern the tool actually uses in production(the same mechanism
fleet-node-restart.ps1already uses successfully in everyreal deploy record). Noted here rather than rounded up to "fully verified live."
Risk: medium (deploy-path tooling)
New capability, additive only — no existing command's behavior changed (
git diffagainst
maintouches onlymain.go's dispatch switch + usage text,AGENTS.md/docs/README.md/docs/FLEET-NODE.mdpointers, and net-new files).Nothing currently deployed calls
node-swapyet; adopting it on a real node is afollow-up, operator-authorized step.
Co-Authored-By: Claude Opus 5.5 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code