fix(update): tolerate a corrupt worker_pid in the in-progress guard

_update_is_stale coerced the stored worker_pid with int() and no guard, so a
status file carrying a non-numeric value raised out of the "an update is
already in progress" check in start_update:

    ValueError: invalid literal for int() with base 10: 'abc'

The function exists so that a running status left behind by a dead worker
does not block every later update, which is exactly the situation where the
status file is least trustworthy. read_update_status above it already
tolerates a missing file, an OSError, invalid JSON and a non-dict payload, so
the module validated the container but not the fields it then coerced.

Fall through to the timestamp check when the pid will not parse. Reporting
the run stale instead would unblock an update that may still be running,
which is the opposite of what the guard is for: a corrupt pid is no evidence
the worker died. A fresh run therefore still blocks, and an old one is still
reported stale.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Rudy CelekliandClaude Opus 5 committed 2026-10-22 00:31:00 -04:00
1 parent 20522e870b
commit 9755a67014
2 files changed
+31 -1

No files matched your search

+7 -1
View File
@@ -403,7 +403,13 @@ def _worker_alive(pid: int) -> bool:
def _update_is_stale(status: Dict[str, Any]) -> bool:
"""A running/restarting status whose worker is gone (killed, OOM, reboot)
would otherwise block every later update."""
worker_pid = int(status.get("worker_pid") or 0)
try:
worker_pid = int(status.get("worker_pid") or 0)
except (TypeError, ValueError):
# A corrupt pid tells us nothing about the worker, so fall through to the
# timestamp check instead of raising. Reporting the run stale here would
# unblock an update that may still be running.
worker_pid = 0
if worker_pid and not _worker_alive(worker_pid):
return True
started = status.get("started_at") or status.get("updated_at")
+24
View File
@@ -264,6 +264,30 @@ def test_a_running_status_left_by_a_dead_worker_does_not_block(tmp_path, monkeyp
) is True
def test_a_corrupt_worker_pid_does_not_crash_the_in_progress_guard(tmp_path, monkeypatch):
"""A status file carrying a non-numeric worker_pid must not raise.
read_update_status already tolerates a missing file, unreadable bytes, invalid
JSON and a non-dict payload, but it does not validate the fields it hands on.
int() on a corrupt pid raised ValueError out of the "already in progress" check.
"""
from cli import update_service
monkeypatch.setattr(update_service, "_worker_alive", lambda pid: True)
fresh = update_service.datetime.now(update_service.timezone.utc).isoformat()
for bad in ("abc", "", "12abc", [], {}, 1.5e3):
status = {"state": "running", "worker_pid": bad, "started_at": fresh}
# A corrupt pid is no evidence the worker died, so a fresh run still blocks.
assert update_service._update_is_stale(status) is False, bad
# ... and an old run is still reported stale rather than blocking forever.
for bad in ("abc", [], {}):
assert update_service._update_is_stale(
{"state": "running", "worker_pid": bad, "started_at": "2020-01-01T00:00:00+00:00"}
) is True, bad
def test_web_handlers_auth_and_no_github_on_version(monkeypatch):
if "web" not in sys.modules:
web_stub = types.ModuleType("web")