Unify app last-access on the DB column (fixes #133)

[ClaydeCode]

Closes #133.

What

Make installed_apps.last_access (the DB column) the single source of truth for app last-access, and delete the parallel in-memory last_access_dict that idle-stop used to read. Per the maintainer decision on the issue: keep the write debounce, and add a small time-bound cache in front of the DB read so the forwardAuth hot path (loading app assets) stays off the DB.

How

• update_last_access (app_meta.py) now debounces against a cached DB read (get_last_access) instead of the mutable request object, so the DB column is authoritative. The cache is TTL-bounded (apps.last_access.read_cache_ttl, default 5s) and refreshed on every write, so the cached value always equals the last persisted last_access. It is cleared on on_apps_update (mirroring the existing auth _app_cache), so an uninstall/reinstall never serves a stale time.
• app_lifecycle.py idle-stop and LRU demotion read InstalledApp.last_access directly off the objects control_apps already loads from the DB each cycle — no hot-path and no cache in that path (its TTL must never delay a stop). The last_access_dict and the per-request write in ensure_app_is_running are gone. A never-accessed app (last_access = None) is treated as maximally idle / oldest, matching the old dict.get(name, 0.0) default.

Behavior note (intentional tradeoff)

Because idle-detection now reads the debounced column, an app's observed idle can lag real access by up to max_update_frequency. In the shipped config this is immaterial: pause_enabled = false, so the only active tier is stop at default_idle_for_stop = 10800s (3h) — a ≤60s lag is 0.5%. When the pause tier is rolled out, keep max_update_frequency comfortably below the smallest active idle_for_pause (both currently default to 60), or an actively-used app could pause up to max_update_frequency seconds early (it unpauses on the next request, ms–2s). This lag is the direct consequence of the three constraints in the issue decision (column = source of truth, remove the dict, keep the debounce); the only way to remove it is to reintroduce a per-request in-memory map, which the issue explicitly asked to delete.

Tests

• tests/test_app_lifecycle.py: three unit tests pinning column-driven idle-stop (stale → stop, recent → keep running, never-accessed → stop).
• tests/test_app_last_access_cache.py: DB-mocked tests pinning that get_last_access serves repeat reads from cache within the TTL and re-reads after expiry.
• tests/test_app_lifecycle_pause.py: adapted to set InstalledApp.last_access; added an LRU test pinning never-accessed = oldest.
• Existing tests/test_app_last_access.py (debounce end-to-end) still green.

Review panel

Three adversarial reviewers ran (adversarial correctness, test adversary, DevEx/readability). Specialists not run: security (no auth-decision/token/secret logic changed), DB/migration (no schema/SQL/migration), API-contract (no route change), UX (not user-visible).

Blocking finding — idle-detection inherits the write-debounce lag (correctness reviewer). An actively-used app could be paused up to max_update_frequency early once the pause tier is enabled. Resolution: dismissed as a merge blocker, with reasoning — it is the logically necessary consequence of the three constraints the issue decision fixed; the proposed fix reintroduces the per-request in-memory map #133 asked to remove; and the shipped default (pause_enabled = false, stop at 3h) makes it immaterial. Surfaced explicitly in the "Behavior note" above with rollout guidance rather than silently absorbed.

Blocking-ish finding — _last_access_cache had no invalidation / no test-scoped reset (both correctness and test reviewers). Uninstall/reinstall within the TTL could serve a stale time; removing the old clean_last_access fixture dropped test isolation for test_app_last_access.py. Resolution: fixed in 41a5788 — added an on_apps_update receiver that clears the cache (mirrors the auth _app_cache pattern), which also restores per-test isolation because app install fires on_apps_update.

Advisory — the cache mechanism itself was untested (test reviewer). Fixed in 41a5788 (test_app_last_access_cache.py; verified it fails if the TTL check is removed).

Advisory — never-accessed app not pinned for LRU victim selection (test reviewer). Fixed in 41a5788 (test_demote_lru_treats_never_accessed_app_as_oldest).

Advisory — cache-entry tuple read positionally; misleading test name; cache-vs-column relationship unstated (DevEx reviewer). Fixed in 41a5788 — NamedTuple cache entry, test renamed, clarifying comment added.

Advisory — tz-normalize duplicated in a few spots. Noted, not changed: each copy is correct and dedup would span two modules; left surgical.

Recommended reading order

1. shard_core/settings.py — new read_cache_ttl option
2. shard_core/data_model/app_meta.py — cache + debounce (source of truth)
3. shard_core/service/app_lifecycle.py — idle/LRU read the column
4. tests/test_app_last_access_cache.py, tests/test_app_lifecycle.py, tests/test_app_lifecycle_pause.py

🤖 Generated with Claude Code