Skip to content

fix: remove stuck node upgrade-scheduled status#145

Merged
jacderida merged 1 commit into
WithAutonomi:mainfrom
jacderida:remove-upgrade-scheduled-status
Jul 8, 2026
Merged

fix: remove stuck node upgrade-scheduled status#145
jacderida merged 1 commit into
WithAutonomi:mainfrom
jacderida:remove-upgrade-scheduled-status

Conversation

@jacderida

Copy link
Copy Markdown
Contributor

The daemon flipped a running node to UpgradeScheduled from a 60s poll the moment its on-disk
binary version drifted, but only returned it to Running on process exit + respawn. When the exit
never followed (false-positive drift, an adopted node still alive, or a fast/Windows upgrade the
poll missed), the node stayed UpgradeScheduled indefinitely.

Remove the status entirely rather than fix it: the window it represented was tiny and not useful.
Upgrades are now detected purely at process exit by on-disk binary version drift for exit codes 0
(Unix) and 100 (Windows RESTART_EXIT_CODE) — which also fixes the pre-existing misclassification
of Windows and fast upgrades that the exit-code-0-only path missed.

Summary

  • Remove the NodeStatus::UpgradeScheduled status variant, the spawn_upgrade_monitor 60s poll,
    mark_upgrade_scheduled, the pending_version field/plumbing (runtime state, NodeInfo,
    NodeStatusSummary, REST handlers, CLI), and the NodeEvent::UpgradeScheduled SSE event.
  • Detect an upgrade at process exit by binary version drift for exit codes {0, RESTART_EXIT_CODE}
    and respawn directly via respawn_upgraded_node.
  • Retain version refresh (respawn_upgraded_node) and the NodeUpgraded completion event.
  • Backwards compatible: the status was never persisted (the registry stores only NodeConfig), and
    a #[serde(alias = "upgrade_scheduled")] on Running maps any value emitted by an older daemon
    during a rolling upgrade back to Running.
  • ant-ui needs no changes (it is a thin proxy over the daemon API).

Test plan

  • Manually tested auto-upgrade node transitions on Linux, macOS and Windows — upgraded nodes
    transition cleanly with the UpgradeScheduled status removed.
  • cargo check --all-targets and cargo clippy --all-targets --all-features -- -D warnings pass.
  • cargo test --all (full suite via CI).

🤖 Generated with Claude Code

The daemon flipped a running node to `UpgradeScheduled` from a 60s poll the
moment its on-disk binary version drifted, but only returned it to `Running`
on process exit + respawn. When the exit never followed (false-positive drift,
an adopted node still alive, or a fast/Windows upgrade the poll missed), the
node stayed `UpgradeScheduled` indefinitely.

Remove the status entirely rather than fix it: the window it represented was
tiny and not useful. Upgrades are now detected purely at process exit by
on-disk binary version drift for exit codes 0 (Unix) and 100 (Windows
RESTART_EXIT_CODE) — which also fixes the pre-existing misclassification of
Windows and fast upgrades that the exit-code-0-only path missed.

Removes the UpgradeScheduled status variant, the spawn_upgrade_monitor poll,
mark_upgrade_scheduled, the pending_version field/plumbing, and the
UpgradeScheduled SSE event. Version refresh (respawn_upgraded_node) and the
NodeUpgraded completion event are retained.

Backwards compatible: the status is never persisted, and a serde alias on
Running maps any `upgrade_scheduled` value emitted by an older daemon during a
rolling upgrade back to Running.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@dirvine dirvine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved under the requested severe-issue-only threshold.

Reviewed PR #145 at head a40b61a and did not find a PR-introduced severe blocker in the upgrade-status removal / exit-driven upgrade respawn path.

Verified locally:

  • cargo check --all-targets passed
  • cargo clippy --all-targets --all-features -- -D warnings passed
  • cargo test -p ant-core upgrade --lib --no-fail-fast passed
  • cargo test -p ant-core node_status --lib --no-fail-fast passed

Notes, not blockers for this PR:

  • PR-head Security Audit is failing because the branch is based before the quinn-proto 0.11.15 lockfile fix on main; GitHub's synthetic merge ref has quinn-proto 0.11.15, so this does not reintroduce that advisory in the merge result.
  • Follow-up issues should cover hardening upgrade-respawn against concurrent stop/eviction and updating docs/UI notes for the removal of UpgradeScheduled / pending_version.

Given Chris's stated release urgency and severe-only review bar, this is approved.

@jacderida
jacderida merged commit f6d38e9 into WithAutonomi:main Jul 8, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants