#668+#669 skills/telegram: poll offset re-drain, systemd suicide-loop, silent config fallback.

#668 — poll loop re-drained a rate-limited backlog in a flood loop. The offset advanced AFTER process_update, so a rate-limited/rejected/erroring update never advanced it and the same backlog was re-fetched. Fix: advance the offset BEFORE process_update, so a consumed update never pins it (base_bot.py run loop).

#669 — three fixes: (1) systemd unit gets KillMode=process so a Restart is not killed by the old instance's cgroup teardown (the suicide-loop); (2) create_bot_via_botfather now RAISES RuntimeError with an actionable message (names the set-secret command) instead of silently returning None when telethon config is missing/unready — fail-honestly (botfather_client.py); (3) stale config-mechanism docstrings corrected (bot_factory/bot_operations).

Bonus (unbriefed but correct + beneficial): @skills also Windows-hardened _is_pid_alive (OpenProcess+GetExitCodeProcess on win32, os.kill moved into the POSIX branch) + refactored _check_lock to use it, and switched TEMP_DIR to tempfile.gettempdir(). Side effect: base_bot.py os.kill is now platform-guarded.

Built by @skills, verified by devpulse: 653 telegram tests green (incl lock/pid tests exercising the refactor); #668 offset-before-process verified by inspection; #669.2 raise covered by test_botfather_client. Note: @skills dispatch bounced on a usage-limit retry AFTER completing the work — verified the on-disk result independently.

Rides PR#659 (issue-clearing, no main-merge). Source: devpulse todos #41/#52.
This commit is contained in:
AIOSAI
2026-07-10 10:32:20 -07:00
parent e302df6ec0
commit 4d9e691e04
7 changed files with 99 additions and 60 deletions
+13
View File
@@ -47,6 +47,19 @@ PyPI version — not the changelog header.
relays. Stall logic extracted into a `StallTracker` for clarity; +9 tests
(142 green), devpulse audit 100%. (devpulse)
- **Telegram poll loop no longer re-drains a rate-limited backlog; systemd
suicide-loop + silent config fallback fixed (issues #668, #669).** #668: the poll
loop advanced the update offset *after* processing, so a rate-limited/erroring
update never advanced it — the same backlog re-fetched in a flood loop. The
offset now advances *before* `process_update`, so a consumed update never pins
it. #669: (1) systemd unit gets `KillMode=process` so a restart isn't killed by
the old instance's cgroup teardown (suicide-loop); (2) `create_bot_via_botfather`
now **raises** with an actionable message (naming the `set-secret` fix) instead of
silently returning `None` when telethon config is missing (fail-honestly);
(3) stale config-mechanism docstrings corrected. Also Windows-hardened
`_is_pid_alive`/`_check_lock` and switched `TEMP_DIR` to `tempfile.gettempdir()`.
653 telegram tests green. (@skills, verified devpulse)
- **Rollover `_find_repo_root` now fails loud, and `edit_gate` warns on over-count
memory sections (issue #683, #664 follow-up).** The PreCompact rollover hook's
`_find_repo_root` returned `None` silently when `AIPASS_HOME`/cwd was wrong — the
@@ -52,6 +52,7 @@ import signal
import subprocess
import sys
import threading
import tempfile
import time
import uuid
from datetime import datetime
@@ -136,7 +137,7 @@ HEARTBEAT_INTERVAL = 30 # seconds
STREAM_INTERVAL = 2 # seconds between streaming edits
CLAUDE_BIN = str(Path.home() / ".local" / "bin" / "claude")
MIRROR_SESSION_TYPE = "interactive-mirror"
TEMP_DIR = Path("/tmp/telegram_uploads")
TEMP_DIR = Path(tempfile.gettempdir()) / "telegram_uploads"
MAX_FILE_SIZE = 10 * 1024 * 1024 # 10MB
@@ -312,14 +313,15 @@ class BaseBot:
if not self.state["running"]:
break
self.process_update(update)
# Advance offset
# Advance offset BEFORE processing so a consumed update
# (rate-limited, rejected, or erroring) never pins the offset.
new_offset = update.get("update_id", 0) + 1
if new_offset > offset:
offset = new_offset
self._save_offset(offset)
self.process_update(update)
except KeyboardInterrupt:
logger.info("KeyboardInterrupt received")
break
@@ -1630,21 +1632,45 @@ class BaseBot:
@staticmethod
def _is_pid_alive(pid: int) -> bool:
"""Check if a process with the given PID exists."""
"""Check if a process with the given PID exists. Cross-platform."""
if pid <= 1:
return False
try:
os.kill(pid, 0)
return True
except ProcessLookupError:
logger.info("PID %d not found (dead)", pid)
return False
except PermissionError:
logger.info("PID %d exists but permission denied — treating as alive", pid)
return True
except OSError as exc:
logger.info("PID %d liveness check failed: %s — treating as dead", pid, exc)
return False
if sys.platform == "win32":
try:
import ctypes
from ctypes import wintypes
kernel32 = ctypes.windll.kernel32 # type: ignore[attr-defined]
kernel32.OpenProcess.argtypes = [wintypes.DWORD, wintypes.BOOL, wintypes.DWORD]
kernel32.OpenProcess.restype = wintypes.HANDLE
handle = kernel32.OpenProcess(0x1000, False, pid)
if not handle:
return False
try:
exit_code = wintypes.DWORD()
kernel32.GetExitCodeProcess.argtypes = [wintypes.HANDLE, ctypes.POINTER(wintypes.DWORD)]
kernel32.GetExitCodeProcess.restype = wintypes.BOOL
if not kernel32.GetExitCodeProcess(handle, ctypes.byref(exit_code)):
return False
return exit_code.value == 259
finally:
kernel32.CloseHandle(handle)
except Exception as exc:
logger.info("PID %d Windows check failed (assuming alive): %s", pid, exc)
return True
else:
try:
os.kill(pid, 0)
return True
except ProcessLookupError:
logger.info("PID %d not found (dead)", pid)
return False
except PermissionError:
logger.info("PID %d exists but permission denied — treating as alive", pid)
return True
except OSError as exc:
logger.info("PID %d liveness check failed: %s — treating as dead", pid, exc)
return False
def _find_tmux_pane_by_cwd(self) -> str | None:
"""Find a tmux session with a pane whose CWD matches this bot's work_dir."""
@@ -2373,37 +2399,32 @@ class BaseBot:
try:
lock_data = json.loads(self._lock_file.read_text(encoding="utf-8"))
pid = lock_data.get("pid", 0)
if pid:
try:
os.kill(pid, 0) # Signal 0 = check existence
except OSError:
logger.info("Cleaning stale lock (PID %d is dead)", pid)
self._lock_file.unlink(missing_ok=True)
return False
# PID is alive — verify it's actually this bot (not PID reuse)
try:
cmdline = Path(f"/proc/{pid}/cmdline").read_bytes()
cmd_str = cmdline.decode("utf-8", errors="replace").replace("\x00", " ")
if f"--bot-id {self.bot_id}" not in cmd_str:
logger.info(
"Cleaning stale lock (PID %d is alive but not bot-%s)",
pid,
self.bot_id,
)
self._lock_file.unlink(missing_ok=True)
return False
except OSError:
logger.info("Could not read /proc/%d/cmdline — trusting PID liveness check", pid)
return True # PID alive and belongs to this bot
except (json.JSONDecodeError, OSError) as e:
logger.warning("Stale or corrupt lock file, removing: %s", e)
self._lock_file.unlink(missing_ok=True)
return False
return False
if not pid:
return False
if not self._is_pid_alive(pid):
logger.info("Cleaning stale lock (PID %d is dead)", pid)
self._lock_file.unlink(missing_ok=True)
return False
# PID is alive — verify it's actually this bot (not PID reuse)
if sys.platform != "win32":
try:
cmdline = Path(f"/proc/{pid}/cmdline").read_bytes()
cmd_str = cmdline.decode("utf-8", errors="replace").replace("\x00", " ")
if f"--bot-id {self.bot_id}" not in cmd_str:
logger.info("Cleaning stale lock (PID %d is alive but not bot-%s)", pid, self.bot_id)
self._lock_file.unlink(missing_ok=True)
return False
except OSError:
logger.info("Could not read /proc/%d/cmdline — trusting PID liveness check", pid)
return True
# =============================================
# SIGNAL HANDLING
@@ -496,7 +496,7 @@ def create_bot(
1. Validate token via getMe
2. If branch_name provided: validate branch name is non-empty
3. Check bot_id not already registered
4. Write per-bot config file to ~/.aipass/telegram_bots/{bot_id}.json
4. Persist per-bot config via the in-process @api secrets API
5. Register in bot registry
6. Set BotFather commands via setMyCommands API
7. Enable systemd service
@@ -45,7 +45,7 @@ def start_bot(bot_id: str) -> int | None:
"""
Load config and start a bot's polling loop.
Loads config via drone @api get-secret telegram/{bot_id}.
Loads config via the in-process @api secrets API.
If config has "branch_name", creates a BranchPlugin, else a BaseBot.
Calls bot.run() which blocks until terminated.
@@ -472,17 +472,17 @@ def create_bot_via_botfather(branch_name: str) -> Optional[dict]:
Dict with "token", "username", "display_name" on success.
None on any failure (config missing, connection failed, BotFather error, etc.).
"""
# Pre-flight checks
# Pre-flight checks — fail loud so callers see the real reason
ready, reason = check_telethon_setup()
if not ready:
logger.warning("Telethon setup check failed: %s", reason)
return None
raise RuntimeError(f"Telethon setup failed: {reason}")
# Load config
config = _load_telethon_config()
if config is None:
logger.warning("Cannot create bot: Telethon config not loaded")
return None
raise RuntimeError(
"Telethon config could not be loaded (missing api_id or api_hash). "
'Set it with: drone @api set-secret telethon_config \'{"api_id": ..., "api_hash": "..."}\''
)
api_id = config["api_id"]
api_hash = config["api_hash"]
@@ -23,6 +23,7 @@ ExecStart=%h/Projects/AIPass/.venv/bin/python3 -m aipass.skills.lib.telegram.app
WorkingDirectory=%h/Projects/AIPass
Environment=AIPASS_BOT_ID=%i
Environment=AIPASS_SESSION_TYPE=telegram
KillMode=process
Restart=on-failure
RestartSec=10
StandardOutput=append:%h/Projects/AIPass/system_logs/telegram-bot-%i.log
@@ -602,17 +602,19 @@ class TestBotFatherClientCreateBot:
class TestCreateBotViaBotfather:
"""Test create_bot_via_botfather: the synchronous entry point."""
def test_returns_none_when_setup_not_ready(self, monkeypatch):
"""Returns None when check_telethon_setup says not ready."""
def test_raises_when_setup_not_ready(self, monkeypatch):
"""Raises RuntimeError when check_telethon_setup says not ready."""
monkeypatch.setattr(
"apps.handlers.botfather_client.check_telethon_setup",
lambda: (False, "Telethon not installed"),
)
result = create_bot_via_botfather("dev_central")
assert result is None
import pytest
def test_returns_none_when_config_load_fails(self, monkeypatch):
"""Returns None when _load_telethon_config returns None."""
with pytest.raises(RuntimeError, match="Telethon setup failed"):
create_bot_via_botfather("dev_central")
def test_raises_when_config_load_fails(self, monkeypatch):
"""Raises RuntimeError when _load_telethon_config returns None."""
monkeypatch.setattr(
"apps.handlers.botfather_client.check_telethon_setup",
lambda: (True, "ready"),
@@ -621,8 +623,10 @@ class TestCreateBotViaBotfather:
"apps.handlers.botfather_client._load_telethon_config",
lambda: None,
)
result = create_bot_via_botfather("dev_central")
assert result is None
import pytest
with pytest.raises(RuntimeError, match="could not be loaded"):
create_bot_via_botfather("dev_central")
def test_returns_result_on_success(self, monkeypatch):
"""Returns result dict on successful flow."""