Merge pull request #186 from FreeshardBase/fix/clayde/revive-exited-apps-on-access

Fix #185: revive exited apps on access via a real-state start primitive

Author: max-tet

agents.md

@@ -25,7 +25,7 @@ shard_core/
   service/            → Business logic
     app_installation/   App install/uninstall/reinstall + background worker queue
     app_lifecycle.py    Two-tier idle control: RUNNING -> PAUSED -> STOPPED, PSI-driven LRU demotion, wake-on-request
-    app_tools.py        Docker CLI wrapper functions (start/stop/pause/unpause/down)
+    app_tools.py        Docker CLI wrappers (pause/unpause/stop/down); start_app is the idempotent revive primitive that decides unpause vs up from real container state
     memory_pressure.py  PSI parsing (/host/pressure/memory), cgroup v2 memory.reclaim page-out
     pause_metrics.py    In-memory pause-tier telemetry accumulators (transitions, latencies, PSI snapshots)
     pairing.py          Terminal pairing (JWT creation, code generation)

shard_core/data_model/backend/shard_model.py

@@ -4,7 +4,7 @@
 from enum import StrEnum, auto
 from typing import List
 
-from pydantic import BaseModel
+from pydantic import BaseModel, field_validator
 
 from .permission_model import PermissionHolder
 from .subscription_model import SubscriptionStatus
@@ -102,6 +102,8 @@ class ShardBase(BaseModel):
     price_cents: int | None = None
     pending_vm_size: VmSize | None = None
     pending_price_cents: int | None = None
+    config_overrides_desired: dict[str, str] = {}
+    config_overrides_applied: dict[str, str] = {}
 
     @property
     def short_id(self) -> str:
@@ -226,6 +228,29 @@ class PairingCodeResponse(BaseModel):
     valid_until: datetime
 
 
+class ConfigOverrideKey(StrEnum):
+    """Allowlist of override keys an operator may set. Typing the request key as
+    this enum is the single source of truth: it rejects unknown keys with 422 and
+    surfaces the allowed set in the OpenAPI schema, so the frontend dropdown is
+    generated from it. Adding an override key = one entry here. Each value is a
+    valid env-var name by construction, so no separate format validation is
+    needed."""
+
+    PAUSE_ENABLED = "PAUSE_ENABLED"
+
+
+class ConfigOverrideRequest(BaseModel):
+    key: ConfigOverrideKey
+    value: str
+
+    @field_validator("value")
+    @classmethod
+    def _value_has_no_newline(cls, v: str) -> str:
+        if "\n" in v or "\r" in v:
+            raise ValueError("value must not contain newlines")
+        return v
+
+
 class CoreUpdateRequest(BaseModel):
     target_version: str
 

shard_core/service/app_installation/worker.py

@@ -12,6 +12,7 @@
 from shard_core.database import installed_apps as db_installed_apps
 from shard_core.data_model.app_meta import Status
 from shard_core.service.app_tools import (
+    app_op_lock,
     get_installed_apps_path,
     docker_create_app_containers,
     docker_stop_app,
@@ -123,31 +124,36 @@ async def _install_app_from_existing_zip(app_name: str):
 
 
 async def _uninstall_app(app_name: str):
-    try:
-        installed_app = await get_app_from_db(app_name)
-        if installed_app.status == Status.PAUSED:
-            # unfreeze while the status still says PAUSED — once it flips to
-            # UNINSTALLING nothing knows the containers are frozen, and a
-            # frozen stack can be neither stopped nor removed
-            await docker_unpause_app(app_name)
-        await update_app_status(app_name, Status.UNINSTALLING)
-    except KeyError:
-        log.warning(f"during uninstallation of {app_name}: app not found in database")
-
-    try:
-        await docker_stop_app(app_name)
-        await docker_shutdown_app(app_name)
-    except Exception as e:
-        log.error(f"Error while shutting down app {app_name}: {e!r}")
-
-    log.debug(f"deleting app data for {app_name}")
-    shutil.rmtree(Path(get_installed_apps_path() / app_name), ignore_errors=True)
-    log.debug(f"removing app {app_name} from database")
-    async with db_conn() as conn:
-        await db_installed_apps.remove(conn, app_name)
-    await write_traefik_dyn_config()
-    await signals.on_apps_update.send_async()
-    log.info(f"uninstalled {app_name}")
+    # Held across the whole teardown so a concurrent wake/control-tick revive
+    # (start_app) can't recreate a container between our stop and row removal.
+    async with app_op_lock(app_name):
+        try:
+            installed_app = await get_app_from_db(app_name)
+            if installed_app.status == Status.PAUSED:
+                # unfreeze while the status still says PAUSED — once it flips to
+                # UNINSTALLING nothing knows the containers are frozen, and a
+                # frozen stack can be neither stopped nor removed
+                await docker_unpause_app(app_name)
+            await update_app_status(app_name, Status.UNINSTALLING)
+        except KeyError:
+            log.warning(
+                f"during uninstallation of {app_name}: app not found in database"
+            )
+
+        try:
+            await docker_stop_app(app_name)
+            await docker_shutdown_app(app_name)
+        except Exception as e:
+            log.error(f"Error while shutting down app {app_name}: {e!r}")
+
+        log.debug(f"deleting app data for {app_name}")
+        shutil.rmtree(Path(get_installed_apps_path() / app_name), ignore_errors=True)
+        log.debug(f"removing app {app_name} from database")
+        async with db_conn() as conn:
+            await db_installed_apps.remove(conn, app_name)
+        await write_traefik_dyn_config()
+        await signals.on_apps_update.send_async()
+        log.info(f"uninstalled {app_name}")
 
 
 async def _reinstall_app(app_name: str):

shard_core/service/app_lifecycle.py

@@ -8,10 +8,9 @@
 from shard_core.data_model.app_meta import InstalledApp, Status
 from shard_core.service import disk, memory_pressure, pause_metrics
 from shard_core.service.app_tools import (
-    docker_start_app,
+    start_app,
     docker_stop_app,
     docker_pause_app,
-    docker_unpause_app,
     get_app_metadata,
     size_is_compatible,
 )
@@ -35,16 +34,12 @@ async def ensure_app_is_running(app: InstalledApp):
     if await size_is_compatible(app_meta.minimum_portal_size):
         global last_access_dict
         last_access_dict[app.name] = time.time()
-        # 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
-        # the wake. This dispatch stays active even with pause_enabled=false,
-        # so already-paused apps still wake after a backout.
-        if app.status == Status.PAUSED:
-            coro = docker_unpause_app(app.name)
-        else:
-            coro = docker_start_app(app.name)
-        task = asyncio.create_task(coro, name=f"ensure {app.name} is running")
+        # One idempotent revive primitive decides unpause vs up from the real
+        # container state, so a paused stack unfreezes and an out-of-band exit
+        # (crash, OOM, core-upgrade converge) still starts instead of 502-looping.
+        task = asyncio.create_task(
+            start_app(app.name), name=f"ensure {app.name} is running"
+        )
         background_tasks.add(task)
         task.add_done_callback(background_tasks.discard)
 
@@ -84,7 +79,7 @@ async def _control_app_time(app: InstalledApp, pause_enabled: bool):
         if app.status != Status.RUNNING and await size_is_compatible(
             app_meta.minimum_portal_size
         ):
-            await docker_start_app(app.name)
+            await start_app(app.name)
         return
 
     idle = time.time() - last_access_dict.get(app.name, 0.0)

shard_core/service/app_tools.py

@@ -2,7 +2,9 @@
 import json
 import logging
 import time
+from collections import defaultdict
 from pathlib import Path
+from typing import Literal
 
 import shard_core.data_model.profile
 from shard_core.database.connection import db_conn
@@ -31,41 +33,135 @@ async def docker_create_app_containers(name: str):
     await subprocess(*_app_compose(name), "up", "--no-start")
 
 
-@throttle(5)
-async def docker_start_app(name: str):
-    async with db_conn() as conn:
-        app = await db_installed_apps.get_by_name(conn, name)
-        app_status = app["status"] if app else None
+ContainerState = Literal["running", "paused", "exited", "missing"]
 
-    if app_status == Status.PAUSED:
-        # a paused app wakes by unfreezing, not by compose up
-        await docker_unpause_app(name)
-        return
+# Statuses a lifecycle revive may act on. Transitional/terminal ones
+# (INSTALLING, *_QUEUED, UNINSTALLING, REINSTALLING, ERROR) must never be
+# started, or the control tick recreates containers mid-uninstall/reinstall.
+_REVIVABLE_STATUS = (Status.STOPPED, Status.RUNNING, Status.DOWN, Status.PAUSED)
 
-    if app_status in [Status.STOPPED, Status.RUNNING, Status.DOWN]:
-        log.debug(f"starting app {name=}")
-        try:
+# Per-app op lock serializing revive against teardown: start_app and the
+# uninstall worker both take it, so a wake-on-access or control-tick revive can
+# never recreate a container the uninstall worker just removed (issue #185).
+_app_op_locks: dict[str, asyncio.Lock] = defaultdict(asyncio.Lock)
+
+
+def app_op_lock(name: str) -> asyncio.Lock:
+    return _app_op_locks[name]
+
+
+async def get_app_container_state(name: str) -> ContainerState:
+    """Real docker state of an app's containers: running | paused | exited | missing.
+
+    Reads the daemon rather than the stored app status, so a caller can revive an
+    app whose containers changed state out-of-band (crash, OOM, core-upgrade
+    converge stop) while the database still says PAUSED or RUNNING.
+    """
+    compose = _app_compose(name)
+    try:
+        ids_out = await subprocess(*compose, "ps", "-a", "-q")
+        ids = [line.strip() for line in ids_out.splitlines() if line.strip()]
+        if not ids:
+            return "missing"
+        states_out = await subprocess(
+            "docker", "inspect", "--format", "{{.State.Status}}", *ids
+        )
+    except SubprocessError:
+        return "missing"
+    states = [line.strip() for line in states_out.splitlines() if line.strip()]
+    if any(s == "paused" for s in states):
+        return "paused"
+    if states and all(s == "running" for s in states):
+        return "running"
+    return "exited"
+
+
+async def _compose_up(name: str):
+    try:
+        await subprocess(*_app_compose(name), "up", "-d")
+    except SubprocessError as e:
+        if "network" in str(e) and "not found" in str(e):
+            log.warning(
+                f"stale network reference for app {name=}, recreating containers"
+            )
+            await subprocess(*_app_compose(name), "down")
+            await subprocess(*_app_compose(name), "up", "-d")
+        elif "Conflict" in str(e) and "already in use" in str(e):
+            log.warning(f"stale containers for app {name=}, removing and recreating")
+            await subprocess(*_app_compose(name), "down")
             await subprocess(*_app_compose(name), "up", "-d")
-        except SubprocessError as e:
-            if "network" in str(e) and "not found" in str(e):
+        else:
+            raise
+
+
+async def _do_unpause(name: str):
+    unpause_started = time.monotonic()
+    await subprocess(*_app_compose(name), "unpause")
+    pause_metrics.record_unpause_latency((time.monotonic() - unpause_started) * 1000)
+    pause_metrics.record_app_transition(name, Status.PAUSED, Status.RUNNING)
+
+
+async def _mark_running(name: str):
+    async with db_conn() as conn:
+        await db_installed_apps.update_status(conn, name, Status.RUNNING)
+    await signals.on_apps_update.send_async()
+
+
+@throttle(5)
+async def start_app(name: str) -> None:
+    """Bring a revivable app up, deciding from the real container state.
+
+    The single revive primitive for wake-on-access and the always-on control
+    tick. It acts only on a revivable app (see _REVIVABLE_STATUS) and dispatches
+    on the real container state, not the stored status, so an app that exited
+    out-of-band while the database still says PAUSED still starts (via up) rather
+    than erroring on unpause. Idempotent: an already-running stack is a no-op.
+
+    Holds the per-app op lock so a revive can never race the uninstall worker's
+    teardown and recreate a container it just removed (issue #185).
+    """
+    async with app_op_lock(name):
+        async with db_conn() as conn:
+            app = await db_installed_apps.get_by_name(conn, name)
+        db_status = app["status"] if app else None
+        if db_status not in _REVIVABLE_STATUS:
+            log.debug(f"app {name=} has {db_status=}, skipping start")
+            return
+
+        state = await get_app_container_state(name)
+
+        if state == "running":
+            if db_status != Status.RUNNING:
                 log.warning(
-                    f"stale network reference for app {name=}, recreating containers"
+                    f"app {name=} container is running but db says {db_status=}; reconciling"
                 )
-                await subprocess(*_app_compose(name), "down")
-                await subprocess(*_app_compose(name), "up", "-d")
-            elif "Conflict" in str(e) and "already in use" in str(e):
+                await _mark_running(name)
+            return
+
+        if state == "paused":
+            if db_status != Status.PAUSED:
                 log.warning(
-                    f"stale containers for app {name=}, removing and recreating"
+                    f"app {name=} container is paused but db says {db_status=}; unpausing"
                 )
-                await subprocess(*_app_compose(name), "down")
-                await subprocess(*_app_compose(name), "up", "-d")
-            else:
-                raise
-        async with db_conn() as conn:
-            await db_installed_apps.update_status(conn, name, Status.RUNNING)
-        await signals.on_apps_update.send_async()
-    else:
-        log.debug(f"app {name=} has status {app_status}, skipping start")
+            log.debug(f"unpausing app {name=}")
+            try:
+                await _do_unpause(name)
+            except SubprocessError:
+                # a partially-paused stack (some containers already exited) can't
+                # be revived by unpause — fall back to a plain start
+                log.warning(f"unpause failed for {name=}, starting instead")
+                await _compose_up(name)
+            await _mark_running(name)
+            return
+
+        # exited / created / missing
+        if db_status in (Status.RUNNING, Status.PAUSED):
+            log.warning(
+                f"app {name=} container is {state} but db says {db_status=}; starting it"
+            )
+        log.debug(f"starting app {name=}")
+        await _compose_up(name)
+        await _mark_running(name)
 
 
 async def docker_pause_app(name: str):
@@ -93,15 +189,8 @@ async def docker_unpause_app(name: str):
         app_status = app["status"] if app else None
     if app_status == Status.PAUSED:
         log.debug(f"unpausing app {name=}")
-        unpause_started = time.monotonic()
-        await subprocess(*_app_compose(name), "unpause")
-        pause_metrics.record_unpause_latency(
-            (time.monotonic() - unpause_started) * 1000
-        )
-        pause_metrics.record_app_transition(name, Status.PAUSED, Status.RUNNING)
-        async with db_conn() as conn:
-            await db_installed_apps.update_status(conn, name, Status.RUNNING)
-        await signals.on_apps_update.send_async()
+        await _do_unpause(name)
+        await _mark_running(name)
     else:
         log.debug(f"app {name=} has {app_status=}, skipping unpause")