From c75c083f843a733fa6151f8790a30f223d08688b Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 00:21:33 +0000 Subject: [PATCH 1/4] Make the DB last_access column the single source of truth for app access update_last_access previously debounced against the mutable app object and recorded access into an in-memory dict read only by idle-stop, so the installed_apps.last_access column was written but never consulted for lifecycle decisions (issue #133). Route the debounce through a cached get_last_access that reads the DB column, and add a short read cache (read_cache_ttl, 5s) so the forwardAuth hot path for loading app assets stays off the DB (issue #89). The write debounce is unchanged; cache entries are refreshed on every write, so the cached value always equals the last persisted last_access. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/data_model/app_meta.py | 33 +++++++++++++++++++++++++++++-- shard_core/settings.py | 1 + 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/shard_core/data_model/app_meta.py b/shard_core/data_model/app_meta.py index 44cbf5e..92f652d 100644 --- a/shard_core/data_model/app_meta.py +++ b/shard_core/data_model/app_meta.py @@ -1,5 +1,6 @@ import datetime import json +import time from enum import Enum from pathlib import Path as FilePath from typing import Optional, List, Dict, Union @@ -164,20 +165,48 @@ class InstalledAppWithMeta(InstalledApp): meta: AppMeta | None +# The DB column installed_apps.last_access is the single source of truth for +# app last-access. This short-lived cache keeps the per-request debounce read +# off the DB on the forwardAuth hot path (loading app assets), which is fired +# on every request to an app — see issue #133 and the auth-cache rationale +# (issue #89). Entries are refreshed on every write, so the cached value always +# equals the last persisted last_access. +_last_access_cache: Dict[str, tuple[Optional[datetime.datetime], float]] = {} + + +async def get_last_access(app_name: str) -> Optional[datetime.datetime]: + ttl = settings().apps.last_access.read_cache_ttl + cached = _last_access_cache.get(app_name) + if cached is not None and time.monotonic() - cached[1] < ttl: + return cached[0] + + from shard_core.database.connection import db_conn + from shard_core.database import installed_apps as db_installed_apps + + async with db_conn() as conn: + row = await db_installed_apps.get_by_name(conn, app_name) + value = row["last_access"] if row else None + if value is not None and value.tzinfo is None: + value = value.replace(tzinfo=datetime.timezone.utc) + _last_access_cache[app_name] = (value, time.monotonic()) + return value + + @signals.on_request_to_app.connect async def update_last_access(app: InstalledApp): now = datetime.datetime.now(datetime.timezone.utc) max_update_frequency = datetime.timedelta( seconds=settings().apps.last_access.max_update_frequency ) - if app.last_access and now - app.last_access < max_update_frequency: + last_access = await get_last_access(app.name) + if last_access and now - last_access < max_update_frequency: return from shard_core.database.connection import db_conn from shard_core.database import installed_apps as db_installed_apps async with db_conn() as conn: await db_installed_apps.update_last_access(conn, app.name, now) - app.last_access = now + _last_access_cache[app.name] = (now, time.monotonic()) if __name__ == "__main__": diff --git a/shard_core/settings.py b/shard_core/settings.py index 10fb2eb..cf25ce4 100644 --- a/shard_core/settings.py +++ b/shard_core/settings.py @@ -54,6 +54,7 @@ class AppLifecycleSettings(BaseModel): class AppLastAccessSettings(BaseModel): max_update_frequency: int = 60 + read_cache_ttl: int = 5 # seconds the debounce read stays cached off the DB class AppUsageReportingSettings(BaseModel): From 5906add51ed41eed7189dc2f8a381a02205603d7 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 00:21:41 +0000 Subject: [PATCH 2/4] Drive idle-stop and LRU demotion from the DB last_access column control_apps already loads every app (with its last_access) from the DB each cycle, so idle and LRU decisions can read that column directly instead of the in-memory last_access_dict. Remove the dict and the per-request write in ensure_app_is_running; access is now recorded solely by update_last_access. A never-accessed app (last_access None) is treated as maximally idle / oldest, matching the old dict.get(name, 0.0) default. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/app_lifecycle.py | 33 ++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index 79e690e..b1f1c9f 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -1,7 +1,7 @@ import asyncio +import datetime import logging -import time -from typing import Dict, List +from typing import List from shard_core.database.connection import db_conn from shard_core.database import installed_apps as db_installed_apps @@ -20,21 +20,38 @@ log = logging.getLogger(__name__) -last_access_dict: Dict[str, float] = dict() background_tasks = set() # Pressure demotion never touches an app accessed within this window (seconds). RECENT_ACCESS_GRACE = 5 +def _last_access_epoch(app: InstalledApp) -> float: + """Last-access as a POSIX timestamp; 0.0 (oldest) when never accessed.""" + if app.last_access is None: + return 0.0 + last = app.last_access + if last.tzinfo is None: + last = last.replace(tzinfo=datetime.timezone.utc) + return last.timestamp() + + +def _idle_seconds(app: InstalledApp) -> float: + """Seconds since the app was last accessed; inf when never accessed.""" + if app.last_access is None: + return float("inf") + now = datetime.datetime.now(datetime.timezone.utc).timestamp() + return now - _last_access_epoch(app) + + @signals.on_request_to_app.connect async def ensure_app_is_running(app: InstalledApp): if disk.current_disk_usage.disk_space_low: return app_meta = get_app_metadata(app.name) if await size_is_compatible(app_meta.minimum_portal_size): - global last_access_dict - last_access_dict[app.name] = time.time() + # last_access is recorded by update_last_access (app_meta.py), the other + # receiver of on_request_to_app; the DB column is the source of truth. # PAUSED wakes via unpause (ms to ~2s), everything else via the legacy # start path. Calling docker_unpause_app directly also dodges the # global 5s throttle on docker_start_app, which would silently drop @@ -87,7 +104,7 @@ async def _control_app_time(app: InstalledApp, pause_enabled: bool): await docker_start_app(app.name) return - idle = time.time() - last_access_dict.get(app.name, 0.0) + idle = _idle_seconds(app) t2 = ( app_meta.lifecycle.idle_for_stop or settings().apps.lifecycle.default_idle_for_stop @@ -127,7 +144,7 @@ async def _demote_lru(apps: List[InstalledApp]): app_meta = get_app_metadata(app.name) if app_meta.lifecycle.always_on: continue - if time.time() - last_access_dict.get(app.name, 0.0) <= RECENT_ACCESS_GRACE: + if _idle_seconds(app) <= RECENT_ACCESS_GRACE: continue if app.status == Status.RUNNING: running.append(app) @@ -135,7 +152,7 @@ async def _demote_lru(apps: List[InstalledApp]): paused.append(app) def lru(candidates: List[InstalledApp]) -> InstalledApp: - return min(candidates, key=lambda app: last_access_dict.get(app.name, 0.0)) + return min(candidates, key=_last_access_epoch) if running: victim = lru(running) From d4fed47dfc37269a3419d6ed1bfc6b11b07c2ddb Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 00:21:41 +0000 Subject: [PATCH 3/4] Adapt lifecycle tests to the DB column and pin its idle-stop semantics The unit tests drove idle via last_access_dict; set InstalledApp.last_access instead. Add three tests in test_app_lifecycle.py pinning that _control_app_time consults the DB column: a stale last_access stops the app, a recent one leaves it running, and a never-accessed running app is stopped. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_app_lifecycle.py | 64 ++++++++++++++++++++++++++++++- tests/test_app_lifecycle_pause.py | 13 ++----- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/tests/test_app_lifecycle.py b/tests/test_app_lifecycle.py index 45a7a92..28d634e 100644 --- a/tests/test_app_lifecycle.py +++ b/tests/test_app_lifecycle.py @@ -1,7 +1,11 @@ +from datetime import datetime, timedelta, timezone +from unittest.mock import AsyncMock, patch + import docker from fastapi import status -from shard_core.data_model.app_meta import InstalledApp, Status +from shard_core.data_model.app_meta import AppMeta, InstalledApp, Lifecycle, Status +from shard_core.service import app_lifecycle from tests.util import retry_async, wait_until_app_installed @@ -101,3 +105,61 @@ async def assert_app_running(): # todo: test_large_app_does_not_start # todo: test app with size comparison + + +# Idle-stop reads the DB last_access column (installed_apps.last_access), which +# is the single source of truth since #133 replaced the in-memory dict. The +# app object passed to _control_app_time is the one control_apps loads from the +# DB, so setting InstalledApp.last_access here pins the column-driven decision. +# tests/config.toml: default_idle_for_stop=12. + + +def _idle_meta() -> AppMeta: + return AppMeta( + v="1.3", + app_version="1.0.0", + name="test", + pretty_name="Test", + icon="icon.svg", + entrypoints=[], + paths={}, + lifecycle=Lifecycle(), + ) + + +async def test_idle_stop_uses_db_last_access_when_stale(): + app = InstalledApp( + name="a", + status=Status.RUNNING, + last_access=datetime.now(timezone.utc) - timedelta(seconds=20), + ) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_awaited_once_with("a") + + +async def test_idle_stop_leaves_recently_accessed_app_running(): + app = InstalledApp( + name="a", + status=Status.RUNNING, + last_access=datetime.now(timezone.utc), + ) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_not_awaited() + + +async def test_never_accessed_running_app_is_treated_as_idle(): + app = InstalledApp(name="a", status=Status.RUNNING, last_access=None) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_awaited_once_with("a") diff --git a/tests/test_app_lifecycle_pause.py b/tests/test_app_lifecycle_pause.py index 23692fa..cfcbbf2 100644 --- a/tests/test_app_lifecycle_pause.py +++ b/tests/test_app_lifecycle_pause.py @@ -5,7 +5,7 @@ (that lives in the integration tests). """ -import time +from datetime import datetime, timedelta, timezone from unittest.mock import AsyncMock, patch import pytest @@ -47,16 +47,9 @@ def docker_mocks(): yield {"start": start, "stop": stop, "pause": pause, "unpause": unpause} -@pytest.fixture(autouse=True) -def clean_last_access(): - app_lifecycle.last_access_dict.clear() - yield - app_lifecycle.last_access_dict.clear() - - def _app(name: str, status: Status, idle: float) -> InstalledApp: - app_lifecycle.last_access_dict[name] = time.time() - idle - return InstalledApp(name=name, status=status) + last_access = datetime.now(timezone.utc) - timedelta(seconds=idle) + return InstalledApp(name=name, status=status, last_access=last_access) PAUSE_ON = {"apps": {"lifecycle": {"pause_enabled": True}}} From 41a578877283b1afdf5a97fd4ad15ae36bf8decc Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 00:33:57 +0000 Subject: [PATCH 4/4] Address review panel: cache invalidation, cache tests, LRU/None coverage - Invalidate _last_access_cache on on_apps_update, mirroring the auth _app_cache pattern, so an uninstall/reinstall of the same app name never serves a stale time (and tests regain isolation the removed dict fixture provided). - Represent cache entries as a NamedTuple (_LastAccessCacheEntry) so the TTL logic reads by field, not tuple index. - Add test_app_last_access_cache.py pinning that get_last_access serves repeat reads from cache within the TTL and re-reads once it expires. - Add an LRU-demotion test pinning that a never-accessed app sorts as oldest. - Note in app_lifecycle that idle decisions read the column directly, not via the request-path cache; rename the overclaiming unit test. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/data_model/app_meta.py | 25 ++++++++++----- shard_core/service/app_lifecycle.py | 3 ++ tests/test_app_last_access_cache.py | 49 +++++++++++++++++++++++++++++ tests/test_app_lifecycle.py | 2 +- tests/test_app_lifecycle_pause.py | 13 ++++++++ 5 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 tests/test_app_last_access_cache.py diff --git a/shard_core/data_model/app_meta.py b/shard_core/data_model/app_meta.py index 92f652d..424e5bf 100644 --- a/shard_core/data_model/app_meta.py +++ b/shard_core/data_model/app_meta.py @@ -3,7 +3,7 @@ import time from enum import Enum from pathlib import Path as FilePath -from typing import Optional, List, Dict, Union +from typing import Optional, List, Dict, NamedTuple, Union from pydantic import BaseModel, model_validator @@ -165,20 +165,31 @@ class InstalledAppWithMeta(InstalledApp): meta: AppMeta | None +class _LastAccessCacheEntry(NamedTuple): + value: Optional[datetime.datetime] + cached_at: float # time.monotonic() when this entry was stored + + # The DB column installed_apps.last_access is the single source of truth for # app last-access. This short-lived cache keeps the per-request debounce read # off the DB on the forwardAuth hot path (loading app assets), which is fired # on every request to an app — see issue #133 and the auth-cache rationale # (issue #89). Entries are refreshed on every write, so the cached value always -# equals the last persisted last_access. -_last_access_cache: Dict[str, tuple[Optional[datetime.datetime], float]] = {} +# equals the last persisted last_access. Invalidated on on_apps_update so an +# uninstall/reinstall of the same name never serves a stale time. +_last_access_cache: Dict[str, _LastAccessCacheEntry] = {} + + +@signals.on_apps_update.connect +async def _invalidate_last_access_cache(_): + _last_access_cache.clear() async def get_last_access(app_name: str) -> Optional[datetime.datetime]: ttl = settings().apps.last_access.read_cache_ttl cached = _last_access_cache.get(app_name) - if cached is not None and time.monotonic() - cached[1] < ttl: - return cached[0] + if cached is not None and time.monotonic() - cached.cached_at < ttl: + return cached.value from shard_core.database.connection import db_conn from shard_core.database import installed_apps as db_installed_apps @@ -188,7 +199,7 @@ async def get_last_access(app_name: str) -> Optional[datetime.datetime]: value = row["last_access"] if row else None if value is not None and value.tzinfo is None: value = value.replace(tzinfo=datetime.timezone.utc) - _last_access_cache[app_name] = (value, time.monotonic()) + _last_access_cache[app_name] = _LastAccessCacheEntry(value, time.monotonic()) return value @@ -206,7 +217,7 @@ async def update_last_access(app: InstalledApp): async with db_conn() as conn: await db_installed_apps.update_last_access(conn, app.name, now) - _last_access_cache[app.name] = (now, time.monotonic()) + _last_access_cache[app.name] = _LastAccessCacheEntry(now, time.monotonic()) if __name__ == "__main__": diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index b1f1c9f..1701a39 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -26,6 +26,9 @@ RECENT_ACCESS_GRACE = 5 +# control_apps loads every app fresh from the DB each cycle, so idle decisions +# read InstalledApp.last_access directly — not through app_meta's request-path +# read cache, whose TTL must never delay a stop. def _last_access_epoch(app: InstalledApp) -> float: """Last-access as a POSIX timestamp; 0.0 (oldest) when never accessed.""" if app.last_access is None: diff --git a/tests/test_app_last_access_cache.py b/tests/test_app_last_access_cache.py new file mode 100644 index 0000000..6ef26c2 --- /dev/null +++ b/tests/test_app_last_access_cache.py @@ -0,0 +1,49 @@ +"""Unit tests for the last-access read cache (app_meta.get_last_access). + +The DB is mocked — these pin that the cache actually elides DB reads within its +TTL and re-reads once it expires, which is the whole point of the cache added in +#133 (keep the forwardAuth hot path off the DB). +""" + +from contextlib import asynccontextmanager +from datetime import datetime, timezone +from unittest.mock import AsyncMock, patch + +from shard_core.data_model import app_meta +from tests.conftest import settings_override + + +@asynccontextmanager +async def _fake_conn(): + yield None + + +async def test_get_last_access_serves_repeat_reads_from_cache(): + app_meta._last_access_cache.clear() + ts = datetime(2026, 1, 1, tzinfo=timezone.utc) + get_by_name = AsyncMock(return_value={"last_access": ts}) + with ( + settings_override({"apps": {"last_access": {"read_cache_ttl": 3600}}}), + patch("shard_core.database.connection.db_conn", _fake_conn), + patch("shard_core.database.installed_apps.get_by_name", get_by_name), + ): + first = await app_meta.get_last_access("foo") + second = await app_meta.get_last_access("foo") + + assert first == ts + assert second == ts + assert get_by_name.call_count == 1 # second read served from cache + + +async def test_get_last_access_rereads_after_ttl_expiry(): + app_meta._last_access_cache.clear() + get_by_name = AsyncMock(return_value={"last_access": None}) + with ( + settings_override({"apps": {"last_access": {"read_cache_ttl": 0}}}), + patch("shard_core.database.connection.db_conn", _fake_conn), + patch("shard_core.database.installed_apps.get_by_name", get_by_name), + ): + await app_meta.get_last_access("foo") + await app_meta.get_last_access("foo") + + assert get_by_name.call_count == 2 # ttl=0 never serves from cache diff --git a/tests/test_app_lifecycle.py b/tests/test_app_lifecycle.py index 28d634e..4fb03a1 100644 --- a/tests/test_app_lifecycle.py +++ b/tests/test_app_lifecycle.py @@ -127,7 +127,7 @@ def _idle_meta() -> AppMeta: ) -async def test_idle_stop_uses_db_last_access_when_stale(): +async def test_idle_stop_uses_last_access_when_stale(): app = InstalledApp( name="a", status=Status.RUNNING, diff --git a/tests/test_app_lifecycle_pause.py b/tests/test_app_lifecycle_pause.py index cfcbbf2..d01e1da 100644 --- a/tests/test_app_lifecycle_pause.py +++ b/tests/test_app_lifecycle_pause.py @@ -270,3 +270,16 @@ async def test_demote_lru_demotes_exactly_one_app_per_cycle(docker_mocks): await app_lifecycle._demote_lru(apps) assert docker_mocks["pause"].await_count == 1 docker_mocks["pause"].assert_awaited_once_with("a") + + +async def test_demote_lru_treats_never_accessed_app_as_oldest(docker_mocks): + apps = [ + _app("recent", Status.RUNNING, idle=100), + InstalledApp(name="never", status=Status.RUNNING, last_access=None), + ] + with patch.object( + app_lifecycle, "get_app_metadata", return_value=_meta(Lifecycle()) + ): + await app_lifecycle._demote_lru(apps) + docker_mocks["pause"].assert_awaited_once_with("never") + docker_mocks["stop"].assert_not_awaited()