Fix Update.AutoApply crash-looping forever on Linux - #30
Merged
Conversation
systemd's default KillMode=control-group kills every process in a service's cgroup on stop/restart -- including the update.ps1 child process UpdateCheckService spawns right before calling StopApplication() to let it swap the binary. That killed update.ps1 before it ever ran, and Restart=always brought the old binary straight back up, which immediately re-detected the same newer release and repeated forever, ~10-13s apart, never once reaching the point of fetching alerts. Confirmed live on a production instance stuck in this loop for 20+ minutes. setup-service.ps1's generated unit (and the hand-maintained deploy.yml unit documented in CONTRIBUTING.md) now set KillMode=process, so only the app's own PID is killed on stop -- update.ps1 survives to finish the swap and its own systemctl restart. Existing installations need KillMode=process added to their unit file by hand (or re-run setup-service.ps1) since this only changes the generated template. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CfWmHii9XYmPNBhz5dGDxZ
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+221
to
+223
| repeated forever — a confirmed-live crash loop that never actually updated. Existing installs | ||
| need `KillMode=process` added to their unit file by hand (or re-run `setup-service.ps1`) since | ||
| this only changes the generated template. Linux passwordless-sudo is now opt-in via `setup-service.ps1 -ConfigurePasswordlessSudo` (a scoped `/etc/sudoers.d/` rule, `visudo -c`-validated before install); `AlertPollingService` now logs the running version at startup (closes "no confirmation an update succeeded"); `setup-service.ps1` pre-checks `appsettings.json` exists; `UpdateCheckService` skips the check entirely on an unversioned (`0.0.0`) dev build (`UnversionedDevBuild` constant); `update.ps1` now health-checks the new version after restarting (`Start-BotService`/`Test-BotIsRunning`, `-RollbackCheckDelaySeconds` default 15) and automatically restores `.bak` + restarts if it didn't stay running — closes "no automatic rollback if a bad release is applied"; `release.yml` now publishes a `checksums.txt` (SHA256 per asset) and `update.ps1` verifies the downloaded archive against it before extracting, aborting on a missing/mismatched entry — closes "no integrity check on downloaded releases" (this protects against corrupted uploads/transport tampering only, not a compromised `release.yml`/repo/token — see docs/TECHNICAL.md for the honest scope). The health check is process/service-alive only, not a real app health check (no endpoint exists to call) — it catches "didn't start at all," not "started but is subtly broken." None of the remaining gaps are regressions — they're accepted-for-now, called out explicitly so they don't get rediscovered as surprises. |
There was a problem hiding this comment.
"Existing installs need KillMode=process added to their unit file by hand (or re-run setup-service.ps1)" is inaccurate: setup-service.ps1 refuses to re-run against an already-installed unit — it errors out with "Unit '$unitPath' already exists. Run with -Uninstall first if you want to recreate it..." (see scripts/setup-service.ps1#L237, unchanged by this PR). A bare re-run will error, not regenerate the file.
This PR's own description correctly says "...or the service reinstalled via setup-service.ps1 -Uninstall + re-run", but that -Uninstall step was dropped when the same sentence was written into the docs. The identical inaccurate phrasing was also added to:
Suggest updating all three to say setup-service.ps1 -Uninstall + re-run, matching the PR description and the script's actual guard.
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
setup-service.ps1's generated systemd unit now setsKillMode=process(was relying on the defaultKillMode=control-group).CONTRIBUTING.mdfor thedeploy.yml-managed instance, for consistency/defense-in-depth.docs/TECHNICAL.md"Known Limitations" andCLAUDE.md's Auto-Update Common Pitfalls entry.Why
UpdateCheckServicespawnsupdate.ps1as a child process, then callsStopApplication()to exit so it can swap the binary. Under the defaultKillMode=control-group, stopping the service kills the entire cgroup — including that just-spawnedupdate.ps1— before it ever runs.Restart=alwaysthen brings the old binary straight back up, which immediately re-detects the same newer release and repeats, forever, ~10-13s apart, never once reaching the point of actually fetching alerts.Confirmed live: a production instance was stuck in exactly this loop for 20+ minutes before we caught it via the logs.
KillMode=processlimits the kill signal to the unit's main PID, lettingupdate.ps1(the only child process this app ever spawns) survive to finish the swap and its ownsystemctl restart.Note for existing installs: this only changes the generated template — an already-installed unit file needs
KillMode=processadded by hand (thensystemctl daemon-reload) or the service reinstalled viasetup-service.ps1 -Uninstall+ re-run.Test plan
dotnet test NwsAlertBot.Tests/NwsAlertBot.Tests.csproj— 97/97 passed (no C# changes, ran to confirm no incidental breakage)setup-service.ps1— no errorssetup-service.ps1install withKillMode=processactually survives a realUpdate.AutoApplycycle end-to-end (not done here — needs a real systemd host with a newer release available to update to)https://claude.ai/code/session_01CfWmHii9XYmPNBhz5dGDxZ