Skip to content

Commit 41df9cf

Browse files
committed
fix: restart-to-apply never re-execs — drop stop_polling race in restart_bot (v0.14.1)
1 parent ffe0ca5 commit 41df9cf

10 files changed

Lines changed: 72 additions & 35 deletions

File tree

.claude/structure/root.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ files:
3333
exports:
3434
- name: restart_bot
3535
kind: function
36-
description: Stops polling, optionally broadcasts a notice, then re-execs `python -m src.main`.
37-
params: [{name: bot, type: Bot}, {name: dp, type: Dispatcher}, {name: chat_id_store, type: ChatIdStore}, {name: notice, type: "str | None"}]
36+
description: Optionally broadcasts a notice, then re-execs `python -m src.main`. Does NOT stop polling/close the session (that would race main()'s teardown and cancel execv); os.execv replaces the whole process.
37+
params: [{name: bot, type: Bot}, {name: chat_id_store, type: ChatIdStore}, {name: notice, type: "str | None"}]
3838
returns: Coroutine
3939
imports:
4040
- from: src.alerts.manager

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
All notable changes to UnraidMonitor will be documented in this file.
44

5+
## [0.14.1] - 2026-06-05
6+
7+
### Fixed
8+
- **Restart-to-apply never came back** — enabling image updates (and the setup-wizard restart) called `dp.stop_polling()` before `os.execv`, which unwound `main()`'s polling loop and ran its shutdown `finally`, cancelling the restart coroutine before it re-exec'd. The bot shut down cleanly but never restarted (it only recovered if Docker's restart policy happened to be set). `restart_bot()` now re-execs directly without stopping polling or closing the session — `os.execv` replaces the whole process, and Python's close-on-exec defaults drop the in-flight long-poll so Telegram frees the getUpdates slot for the fresh process.
9+
510
## [0.14.0] - 2026-06-05
611

712
### Added

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ src/utils/version_store.py - read/write data/announced_version.json for startup
9292

9393
## Project Overview
9494

95-
Unraid Server Monitor Bot (v0.14.0) - A Docker-based Telegram bot for monitoring Unraid servers. Monitors Docker containers (events, logs, resources) and Unraid server health (CPU, memory, disks, array, UPS). Uses multi-provider LLM support (Anthropic, OpenAI, Ollama) for AI-powered diagnostics and natural language interaction. Sends alerts via Telegram with quick-action buttons.
95+
Unraid Server Monitor Bot (v0.14.1) - A Docker-based Telegram bot for monitoring Unraid servers. Monitors Docker containers (events, logs, resources) and Unraid server health (CPU, memory, disks, array, UPS). Uses multi-provider LLM support (Anthropic, OpenAI, Ollama) for AI-powered diagnostics and natural language interaction. Sends alerts via Telegram with quick-action buttons.
9696

9797
**Version string lives in TWO places**`pyproject.toml` (`version`) and `src/__init__.py` (`__version__`). Both must be bumped together on release; `tests/test_version.py` guards against drift. `BOT_VERSION` (in `src/bot/health_command.py`) resolves from installed package metadata when available, else falls back to `src.__version__` — the package is not installed in the Docker image, so the `__version__` fallback is what runs in production.
9898

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "unraid-monitor-bot"
3-
version = "0.14.0"
3+
version = "0.14.1"
44
requires-python = ">=3.11"
55
dependencies = [
66
"docker>=7.0.0,<8.0.0",

src/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.14.0"
1+
__version__ = "0.14.1"

src/main.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ async def on_wizard_complete() -> None:
8383
"""Called when the wizard saves config.yaml for the first time."""
8484
logger.info("Setup wizard complete -- restarting to apply config")
8585
await restart_bot(
86-
bot, dp, chat_id_store,
86+
bot, chat_id_store,
8787
notice="✅ Setup complete! Restarting to apply configuration...",
8888
)
8989

@@ -123,7 +123,7 @@ async def on_rerun_complete() -> None:
123123
"""Called when a /setup re-run saves updated config."""
124124
logger.info("Setup wizard re-run complete -- restarting to apply config")
125125
await restart_bot(
126-
bot, dp, chat_id_store,
126+
bot, chat_id_store,
127127
notice="✅ Configuration updated! Restarting to apply changes...",
128128
)
129129

src/restart.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,47 @@
55
src.main`` so the supervisor (Docker) sees the same process.
66
"""
77

8-
import asyncio
98
import logging
109
import os
1110
import sys
1211
from typing import TYPE_CHECKING
1312

1413
if TYPE_CHECKING:
15-
from aiogram import Bot, Dispatcher
14+
from aiogram import Bot
1615
from src.alerts.manager import ChatIdStore
1716

1817
logger = logging.getLogger(__name__)
1918

2019

2120
async def restart_bot(
2221
bot: "Bot",
23-
dp: "Dispatcher",
2422
chat_id_store: "ChatIdStore",
2523
notice: str | None = None,
2624
) -> None:
27-
"""Stop polling and re-exec the process so new config takes effect.
25+
"""Replace the running process with a fresh ``python -m src.main``.
2826
2927
Args:
3028
bot: The aiogram Bot, used to broadcast ``notice`` if provided.
31-
dp: The dispatcher to stop polling on.
3229
chat_id_store: Source of chat IDs for the optional broadcast.
3330
notice: If set, sent to every known chat before restarting. Callers
3431
that have already messaged the user (e.g. an inline-button handler
3532
editing its own message) should pass ``None``.
33+
34+
We deliberately do NOT call ``dp.stop_polling()`` or ``bot.session.close()``
35+
first: either would unwind ``main()``'s polling loop and run its
36+
``finally`` (``bg.shutdown()`` + loop teardown), which cancels this
37+
coroutine *before* it reaches ``os.execv`` — the restart then silently
38+
degrades to a plain exit that only Docker's restart policy (if any) would
39+
recover. ``os.execv`` replaces the whole process image, so polling, tasks
40+
and sockets all stop regardless. Python marks file descriptors
41+
close-on-exec by default, so the in-flight long-poll connection drops on
42+
exec and Telegram frees the getUpdates slot immediately (no 409 conflict
43+
for the fresh process).
44+
45+
The broadcast ``await`` below resolves once Telegram has the message, and
46+
there is intentionally no awaitable between it and ``execv`` so nothing can
47+
preempt the restart. ``execv`` only returns on failure, in which case the
48+
bot keeps running normally.
3649
"""
3750
logger.info("Restarting bot to apply configuration changes")
3851
if notice:
@@ -41,6 +54,4 @@ async def restart_bot(
4154
await bot.send_message(cid, notice)
4255
except Exception:
4356
pass
44-
await dp.stop_polling()
45-
await asyncio.sleep(1)
4657
os.execv(sys.executable, [sys.executable, "-m", "src.main"])

src/startup.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ async def start_monitoring(
463463
)
464464

465465
async def _restart_to_apply() -> None:
466-
await restart_bot(bot, dp, chat_id_store)
466+
await restart_bot(bot, chat_id_store)
467467

468468
controller, diagnostic_service = register_commands(
469469
dp,

tests/test_restart.py

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,54 +8,75 @@
88

99

1010
@pytest.mark.asyncio
11-
async def test_restart_broadcasts_notice_stops_polling_and_execs():
11+
async def test_restart_broadcasts_notice_and_execs():
1212
bot = MagicMock()
1313
bot.send_message = AsyncMock()
14-
dp = MagicMock()
15-
dp.stop_polling = AsyncMock()
1614
store = MagicMock()
1715
store.get_all_chat_ids.return_value = [1, 2]
1816

19-
with patch("src.restart.os.execv") as execv, \
20-
patch("src.restart.asyncio.sleep", new=AsyncMock()):
21-
await restart_bot(bot, dp, store, notice="restarting")
17+
with patch("src.restart.os.execv") as execv:
18+
await restart_bot(bot, store, notice="restarting")
2219

2320
assert bot.send_message.await_count == 2
24-
dp.stop_polling.assert_awaited_once()
2521
execv.assert_called_once()
2622

2723

2824
@pytest.mark.asyncio
2925
async def test_restart_without_notice_skips_broadcast():
3026
bot = MagicMock()
3127
bot.send_message = AsyncMock()
32-
dp = MagicMock()
33-
dp.stop_polling = AsyncMock()
3428
store = MagicMock()
3529
store.get_all_chat_ids.return_value = [1, 2]
3630

37-
with patch("src.restart.os.execv") as execv, \
38-
patch("src.restart.asyncio.sleep", new=AsyncMock()):
39-
await restart_bot(bot, dp, store)
31+
with patch("src.restart.os.execv") as execv:
32+
await restart_bot(bot, store)
4033

4134
bot.send_message.assert_not_awaited()
42-
dp.stop_polling.assert_awaited_once()
35+
execv.assert_called_once()
36+
37+
38+
@pytest.mark.asyncio
39+
async def test_restart_does_not_stop_polling_or_close_session():
40+
"""Regression: stopping polling/closing the session unwinds main()'s loop
41+
and cancels this coroutine before execv runs (the restart silently becomes
42+
a clean exit). restart_bot must touch neither."""
43+
bot = MagicMock()
44+
bot.send_message = AsyncMock()
45+
bot.session = MagicMock()
46+
bot.session.close = AsyncMock()
47+
store = MagicMock()
48+
store.get_all_chat_ids.return_value = [1]
49+
50+
with patch("src.restart.os.execv") as execv:
51+
await restart_bot(bot, store, notice="hi")
52+
53+
bot.session.close.assert_not_awaited()
4354
execv.assert_called_once()
4455

4556

4657
@pytest.mark.asyncio
4758
async def test_restart_swallows_send_failures():
4859
bot = MagicMock()
4960
bot.send_message = AsyncMock(side_effect=Exception("network"))
50-
dp = MagicMock()
51-
dp.stop_polling = AsyncMock()
5261
store = MagicMock()
5362
store.get_all_chat_ids.return_value = [1]
5463

55-
with patch("src.restart.os.execv") as execv, \
56-
patch("src.restart.asyncio.sleep", new=AsyncMock()):
57-
await restart_bot(bot, dp, store, notice="hi")
64+
with patch("src.restart.os.execv") as execv:
65+
await restart_bot(bot, store, notice="hi")
5866

5967
# A failed broadcast must not prevent the restart
60-
dp.stop_polling.assert_awaited_once()
6168
execv.assert_called_once()
69+
70+
71+
@pytest.mark.asyncio
72+
async def test_restart_execs_current_module():
73+
bot = MagicMock()
74+
store = MagicMock()
75+
store.get_all_chat_ids.return_value = []
76+
77+
with patch("src.restart.os.execv") as execv:
78+
await restart_bot(bot, store)
79+
80+
args = execv.call_args.args
81+
# os.execv(python, [python, "-m", "src.main"])
82+
assert args[1][-2:] == ["-m", "src.main"]

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)