Lock stacks during compose runs, cache stats, persist runtime state (0.47.0)

F7 — Nothing stopped two compose operations landing on the same stack. There
was a busy flag, but is_busy() was only ever read to colour the status column;
no lifecycle handler consulted it before acting. Two tabs, or auto-update
picking up a stack somebody had just clicked, both ran pull + up -d against the
same project and raced over recreating containers.

Lifecycle calls, the two deploy WebSockets and the auto-update pass now take a
real lock; a second caller gets 409 (or an error frame and close 4409) and
auto-update skips and retries next cycle. The lock is a row rather than a set
in one worker's memory, so it holds across workers and across a restart, and it
carries an expiry — a worker killed mid-deploy would otherwise strand the stack
with no fix short of editing the database.

F10 — /api/stacks/stats sampled every running container on every call, one
blocking daemon request each, and both the dashboard and the stacks list poll
it every five seconds. Two tabs on a 40-container host meant a sustained ~16
samples a second. Cached for 4s behind a lock so concurrent callers share one
sweep, the same shape dashboard_service already used for its fleet aggregate.

F11 — Three module dicts assumed exactly one uvicorn worker without saying so
and were lost on restart. The busy set is the lock above. The image update
cache is now mirrored to SQLite, so a restart shows the badges immediately
instead of blanking them for up to an hour, and the already-notified marks come
back with them rather than re-announcing the same updates. The login rate
limiter is a table, so it cannot be cleared by getting the process to restart
and no longer multiplies by the worker count.

The constraint that shaped this: compose_service and update_service are shared
with the agent, which has no database. Neither may import one. So the lock is a
separate service the central app enforces at its own entry points, and update
persistence is an opt-in callback the central app registers in its lifespan —
the agent registers nothing and behaves exactly as before. A test asserts
update_service never imports the database, since that is the kind of thing a
later change breaks silently.

Both new nets were checked by reverting the fix: dropping the lock from
_lifecycle fails six tests, removing the stats cache fails the one that names
the behaviour.

Also wires up cache pruning in the same sweep — without it both the dict and
the table grew one entry per image tag ever run, for the life of the install.

31 new tests (729 total).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dk43rmEeRfYi5wsLDbmfyG

backend/tests/test_runtime_state.py

@@ -0,0 +1,284 @@
+"""State that used to live in module dicts (F10, F11).
+
+Three things were kept in process memory, all of them assuming exactly one
+uvicorn worker without saying so, and all of them lost on restart:
+
+* the live stats sweep, which was not cached at all and got polled every five
+  seconds by two pages at once;
+* the registry digests behind the update badges, so a restart blanked every
+  badge for up to an hour and re-announced updates already notified about;
+* the login rate limiter, which an attacker could reset by getting the process
+  to restart, and which multiplied by the worker count.
+"""
+from __future__ import annotations
+
+from datetime import datetime, timedelta, timezone
+
+import pytest
+from sqlmodel import Session, delete, select
+
+
+# --------------------------------------------------------------------------- #
+# F10 — the stats cache
+# --------------------------------------------------------------------------- #
+
+
+@pytest.fixture
+def stats(monkeypatch):
+    from services import stats_service
+
+    stats_service._cache["data"] = None
+    stats_service._cache["ts"] = 0.0
+    return stats_service
+
+
+def test_repeat_calls_inside_the_window_reuse_one_sweep(stats, monkeypatch):
+    """The dashboard and the stacks list both poll this every 5s.
+
+    Each sweep is one blocking daemon call per running container, so serving
+    them from one sample is the whole point.
+    """
+    calls = []
+    monkeypatch.setattr(stats, "_sample", lambda: calls.append(1) or {"a": {}})
+
+    assert stats.stack_stats() == {"a": {}}
+    assert stats.stack_stats() == {"a": {}}
+    assert stats.stack_stats() == {"a": {}}
+    assert len(calls) == 1
+
+
+def test_the_cache_expires(stats, monkeypatch):
+    calls = []
+    monkeypatch.setattr(stats, "_sample", lambda: calls.append(1) or {})
+
+    stats.stack_stats()
+    # Pretend the window has passed rather than sleeping through it.
+    stats._cache["ts"] -= stats.CACHE_TTL + 1
+    stats.stack_stats()
+    assert len(calls) == 2
+
+
+def test_refresh_bypasses_the_cache(stats, monkeypatch):
+    calls = []
+    monkeypatch.setattr(stats, "_sample", lambda: calls.append(1) or {})
+
+    stats.stack_stats()
+    stats.stack_stats(refresh=True)
+    assert len(calls) == 2
+
+
+def test_a_failing_sample_is_not_cached_as_a_success(stats, monkeypatch):
+    """A Docker outage returns {} — it must not stick for the whole window
+    once the daemon is back."""
+    monkeypatch.setattr(stats, "_sample", dict)
+    assert stats.stack_stats() == {}
+
+    monkeypatch.setattr(stats, "_sample", lambda: {"a": {"containers": 1}})
+    assert stats.stack_stats(refresh=True) == {"a": {"containers": 1}}
+
+
+# --------------------------------------------------------------------------- #
+# F11 — the image update cache survives a restart
+# --------------------------------------------------------------------------- #
+
+
+@pytest.fixture
+def clean_image_status(db):
+    from database import engine
+    from models.runtime_state import ImageStatus
+    from services import update_service
+
+    with Session(engine) as session:
+        session.exec(delete(ImageStatus))
+        session.commit()
+    update_service._CACHE.clear()
+    update_service._NOTIFIED.clear()
+    update_service.set_persist_callback(None)
+    yield
+    update_service.set_persist_callback(None)
+    update_service._CACHE.clear()
+    update_service._NOTIFIED.clear()
+
+
+def test_a_saved_status_comes_back_after_a_restart(clean_image_status):
+    """The badge is there immediately instead of blank until the next sweep."""
+    from services import image_status_store, update_service
+
+    status = update_service.UpdateStatus(
+        image="nginx:latest",
+        update_available=True,
+        current_digest="sha256:old",
+        remote_digest="sha256:new",
+        checked_at=123.0,
+    )
+    image_status_store.save(status, notified=True)
+
+    # Simulate a restart: memory is empty, then install() seeds it.
+    update_service._CACHE.clear()
+    update_service._NOTIFIED.clear()
+    assert image_status_store.install() == 1
+
+    cached = update_service.get_cache()["nginx:latest"]
+    assert cached["update_available"] is True
+    assert cached["remote_digest"] == "sha256:new"
+    # And the "already told the user" mark comes back with it, so the restart
+    # does not re-announce the same update.
+    assert "nginx:latest" in update_service._NOTIFIED
+
+
+def test_installing_starts_mirroring_writes(clean_image_status):
+    from database import engine
+    from models.runtime_state import ImageStatus
+    from services import image_status_store, update_service
+
+    image_status_store.install()
+    update_service._store(
+        update_service.UpdateStatus(
+            image="redis:7", update_available=False, current_digest="sha256:a",
+            remote_digest="sha256:a", checked_at=1.0,
+        ),
+        notified=False,
+    )
+    with Session(engine) as session:
+        row = session.get(ImageStatus, "redis:7")
+    assert row is not None and row.update_available is False
+
+
+def test_saving_the_same_image_twice_updates_it(clean_image_status):
+    from database import engine
+    from models.runtime_state import ImageStatus
+    from services import image_status_store, update_service
+
+    def status(remote):
+        return update_service.UpdateStatus(
+            image="app:1", update_available=True, current_digest="sha256:x",
+            remote_digest=remote, checked_at=1.0,
+        )
+
+    image_status_store.save(status("sha256:one"), notified=False)
+    image_status_store.save(status("sha256:two"), notified=True)
+
+    with Session(engine) as session:
+        rows = session.exec(select(ImageStatus).where(ImageStatus.image == "app:1")).all()
+    assert len(rows) == 1
+    assert rows[0].remote_digest == "sha256:two"
+    assert rows[0].notified is True
+
+
+def test_pruning_drops_images_no_stack_uses_any_more(clean_image_status):
+    from database import engine
+    from models.runtime_state import ImageStatus
+    from services import image_status_store, update_service
+
+    for image in ("kept:1", "gone:1"):
+        image_status_store.save(
+            update_service.UpdateStatus(
+                image=image, update_available=False, current_digest=None,
+                remote_digest=None, checked_at=0.0,
+            ),
+            notified=False,
+        )
+
+    assert image_status_store.prune(keep={"kept:1"}) == 1
+    with Session(engine) as session:
+        assert [r.image for r in session.exec(select(ImageStatus)).all()] == ["kept:1"]
+
+
+def test_the_agent_has_no_persistence_wired_up(clean_image_status):
+    """``update_service`` is shared with the agent, which has no database.
+
+    Persistence therefore has to be opt-in, registered by the central app —
+    if this module ever imports the database directly, the agent breaks.
+    """
+    import inspect
+
+    from services import update_service
+
+    source = inspect.getsource(update_service)
+    assert "from database import" not in source
+    assert "import database" not in source
+    assert update_service._persist_cb is None
+
+
+# --------------------------------------------------------------------------- #
+# F11 — the login rate limiter
+# --------------------------------------------------------------------------- #
+
+
+@pytest.fixture
+def clean_attempts(db):
+    from database import engine
+    from models.runtime_state import LoginAttempt
+
+    with Session(engine) as session:
+        session.exec(delete(LoginAttempt))
+        session.commit()
+    yield
+
+
+def test_the_eleventh_attempt_in_a_minute_is_refused(client, clean_attempts):
+    body = {"username": "nobody", "password": "wrong"}
+    for _ in range(10):
+        assert client.post("/api/auth/login", json=body).status_code == 401
+    assert client.post("/api/auth/login", json=body).status_code == 429
+
+
+def test_the_limit_survives_a_restart(db, clean_attempts):
+    """In memory this reset on every restart, so an attacker could clear their
+    own budget by getting the process to fall over."""
+    from database import engine
+    from fastapi import HTTPException
+    from routers import auth as auth_router
+
+    with Session(engine) as session:
+        for _ in range(10):
+            auth_router._check_rate_limit(session, "10.0.0.9")
+
+    # A fresh session stands in for a fresh process — the counter is a table.
+    with Session(engine) as session:
+        with pytest.raises(HTTPException) as excinfo:
+            auth_router._check_rate_limit(session, "10.0.0.9")
+    assert excinfo.value.status_code == 429
+
+
+def test_different_clients_have_separate_budgets(db, clean_attempts):
+    """Only meaningful because uvicorn runs with --proxy-headers; without it
+    every request carries the frontend container's IP and one client would
+    throttle everyone."""
+    from database import engine
+    from fastapi import HTTPException
+    from routers import auth as auth_router
+
+    with Session(engine) as session:
+        for _ in range(10):
+            auth_router._check_rate_limit(session, "10.0.0.1")
+        auth_router._check_rate_limit(session, "10.0.0.2")  # must not raise
+        with pytest.raises(HTTPException):
+            auth_router._check_rate_limit(session, "10.0.0.1")
+
+
+def test_attempts_age_out_of_the_window(db, clean_attempts):
+    from database import engine
+    from models.runtime_state import LoginAttempt
+    from routers import auth as auth_router
+
+    old = datetime.now(timezone.utc) - timedelta(minutes=5)
+    with Session(engine) as session:
+        for _ in range(10):
+            session.add(LoginAttempt(ip="10.0.0.3", at=old))
+        session.commit()
+        auth_router._check_rate_limit(session, "10.0.0.3")  # window has moved on
+
+
+def test_stale_rows_are_pruned(db, clean_attempts):
+    from database import engine
+    from models.runtime_state import LoginAttempt
+    from routers import auth as auth_router
+
+    ancient = datetime.now(timezone.utc) - timedelta(days=2)
+    with Session(engine) as session:
+        session.add(LoginAttempt(ip="10.0.0.4", at=ancient))
+        session.commit()
+        auth_router._check_rate_limit(session, "10.0.0.5")
+        remaining = session.exec(select(LoginAttempt)).all()
+    assert all(r.ip != "10.0.0.4" for r in remaining), "the table must not grow forever"

backend/tests/test_stack_locking.py

@@ -0,0 +1,206 @@
+"""One compose operation per stack (F7), and the lock surviving a restart (F11).
+
+``docker compose`` does no locking of its own. Before 0.47.0 nothing stopped two
+``update`` calls landing on the same project at once — two browser tabs, or the
+auto-update pass picking up a stack somebody had just clicked — and both would
+run ``pull`` then ``up -d`` and fight over recreating the same containers.
+
+There was a busy flag, but it only fed the status column: no handler consulted
+it before acting. The lock is a table now, so it also holds across workers and
+across a restart.
+"""
+from __future__ import annotations
+
+from datetime import datetime, timedelta, timezone
+
+import pytest
+from sqlmodel import Session
+
+
+@pytest.fixture
+def lock_session(db):
+    from database import engine
+
+    with Session(engine) as session:
+        yield session
+
+
+@pytest.fixture(autouse=True)
+def clean_locks(db):
+    """Locks are global state; don't let one test leak into the next."""
+    from database import engine
+    from models.runtime_state import StackLock
+    from sqlmodel import delete
+
+    with Session(engine) as session:
+        session.exec(delete(StackLock))
+        session.commit()
+    yield
+
+
+@pytest.fixture
+def svc():
+    from services import stack_lock_service
+
+    return stack_lock_service
+
+
+# --------------------------------------------------------------------------- #
+# The lock itself
+# --------------------------------------------------------------------------- #
+
+
+def test_a_second_caller_is_refused(svc, lock_session):
+    svc.acquire(lock_session, "demo", "update", "alice")
+    with pytest.raises(svc.StackBusy) as excinfo:
+        svc.acquire(lock_session, "demo", "start", "bob")
+    assert excinfo.value.action == "update", "the error names the operation in flight"
+
+
+def test_releasing_frees_it(svc, lock_session):
+    svc.acquire(lock_session, "demo", "update")
+    svc.release(lock_session, "demo")
+    svc.acquire(lock_session, "demo", "start")  # must not raise
+
+
+def test_releasing_a_lock_nobody_holds_is_harmless(svc, lock_session):
+    svc.release(lock_session, "never-locked")
+
+
+def test_different_stacks_do_not_block_each_other(svc, lock_session):
+    svc.acquire(lock_session, "one", "update")
+    svc.acquire(lock_session, "two", "update")
+
+
+def test_hold_releases_even_when_the_operation_fails(svc, lock_session):
+    """A failed deploy must not leave the stack locked."""
+    with pytest.raises(RuntimeError):
+        with svc.hold(lock_session, "demo", "update"):
+            raise RuntimeError("compose blew up")
+    assert not svc.is_busy(lock_session, "demo")
+
+
+def test_an_expired_lock_is_taken_over(svc, lock_session):
+    """The recovery path for a worker killed mid-deploy.
+
+    Without it the stack would stay locked forever and the only fix would be
+    editing the database by hand.
+    """
+    from models.runtime_state import StackLock
+
+    past = datetime.now(timezone.utc) - timedelta(minutes=5)
+    lock_session.add(
+        StackLock(
+            stack_id="orphaned",
+            action="update",
+            owner="a worker that died",
+            acquired_at=past - timedelta(minutes=30),
+            expires_at=past,
+        )
+    )
+    lock_session.commit()
+
+    assert not svc.is_busy(lock_session, "orphaned")
+    svc.acquire(lock_session, "orphaned", "start")  # must not raise
+
+
+def test_active_lists_current_locks_and_skips_expired(svc, lock_session):
+    from models.runtime_state import StackLock
+
+    svc.acquire(lock_session, "live", "update")
+    lock_session.add(
+        StackLock(
+            stack_id="dead",
+            action="start",
+            expires_at=datetime.now(timezone.utc) - timedelta(seconds=1),
+        )
+    )
+    lock_session.commit()
+
+    assert svc.active(lock_session) == {"live": "update"}
+
+
+def test_prune_removes_only_expired_locks(svc, lock_session):
+    from models.runtime_state import StackLock
+
+    svc.acquire(lock_session, "live", "update")
+    lock_session.add(
+        StackLock(
+            stack_id="dead",
+            action="start",
+            expires_at=datetime.now(timezone.utc) - timedelta(seconds=1),
+        )
+    )
+    lock_session.commit()
+
+    assert svc.prune_expired(lock_session) == 1
+    assert svc.active(lock_session) == {"live": "update"}
+
+
+def test_the_lock_outlives_the_session_that_took_it(svc, db):
+    """It is a row, not a set in one worker's memory — that is the point."""
+    from database import engine
+
+    with Session(engine) as first:
+        svc.acquire(first, "persisted", "update")
+
+    with Session(engine) as second:
+        assert svc.is_busy(second, "persisted")
+        with pytest.raises(svc.StackBusy):
+            svc.acquire(second, "persisted", "start")
+
+
+# --------------------------------------------------------------------------- #
+# Enforcement at the HTTP entry point
+# --------------------------------------------------------------------------- #
+
+
+@pytest.fixture
+def locked_stack(db):
+    """A registered stack that is already mid-operation."""
+    from database import engine
+    from models.stack import Stack
+    from services import compose_service, stack_lock_service
+
+    stack_id = "lock-fixture"
+    compose_service.write_compose(stack_id, "services:\n  a:\n    image: alpine\n")
+    with Session(engine) as session:
+        if not session.get(Stack, stack_id):
+            session.add(Stack(id=stack_id, name="lock fixture"))
+            session.commit()
+        stack_lock_service.acquire(session, stack_id, "update", "someone-else")
+    return stack_id
+
+
+@pytest.mark.parametrize("action", ["start", "stop", "restart", "pull", "update", "down"])
+def test_lifecycle_calls_are_refused_while_a_stack_is_busy(as_admin, locked_stack, action):
+    """409, and crucially *without* reaching Docker — the guard is in front."""
+    response = as_admin.post(f"/api/stacks/{locked_stack}/{action}")
+    assert response.status_code == 409
+    assert "busy" in response.json()["detail"].lower()
+    assert "update in progress" in response.json()["detail"]
+
+
+def test_the_stacks_list_shows_a_busy_stack_as_updating(as_admin, locked_stack):
+    rows = {row["id"]: row for row in as_admin.get("/api/stacks").json()}
+    assert rows[locked_stack]["status"] == "updating"
+
+
+def test_an_unlocked_stack_is_not_refused(as_admin, db):
+    """The guard must not block ordinary use.
+
+    Without a Docker daemon the call fails further in — anything but 409 means
+    it got past the lock, which is what this asserts.
+    """
+    from database import engine
+    from models.stack import Stack
+    from services import compose_service
+
+    stack_id = "unlocked-fixture"
+    compose_service.write_compose(stack_id, "services:\n  a:\n    image: alpine\n")
+    with Session(engine) as session:
+        if not session.get(Stack, stack_id):
+            session.add(Stack(id=stack_id, name="unlocked fixture"))
+            session.commit()
+
+    assert as_admin.post(f"/api/stacks/{stack_id}/start").status_code != 409