Merge pull request #166 from FreeshardBase/fix/clayde/traefik-config-skip-broken-apps

fix(startup): never let one broken app row block core boot

Author: max-tet

.claude/skills/freeshard-architecture-contract/SKILL.md

@@ -145,14 +145,15 @@ The four status **allow-lists** live in shard_core/service/app_tools.py:
 
 Plus two **exclusion filters**:
 
-- `write_traefik_dyn_config` skips INSTALLATION_QUEUED and ERROR apps (app_installation/util.py:108,113)
+- `write_traefik_dyn_config` skips apps in `_NOT_ROUTED_STATUS` = INSTALLATION_QUEUED, UNINSTALLING, ERROR (app_installation/util.py:23-28,115). INSTALLING is deliberately NOT in that set: `_install_app_from_zip` writes the traefik config while the status is still INSTALLING, and no further write follows before STOPPED — filtering it would leave freshly installed apps unrouted (worker.py:118-119,184)
 - `control_apps` (idle lifecycle) skips INSTALLATION_QUEUED and INSTALLING (service/app_lifecycle.py:45)
 
 **RULE: when adding or repurposing a status, audit all six sites above.** A status
 missing from an allow-list silently no-ops (`log.debug` + skip); a status missing from
-the exclusion filters gets routed/lifecycled while half-installed —
-`write_traefik_dyn_config` calling `get_app_metadata()` on an app with missing files
-raises `MetadataNotFound` inside the lifespan (app_factory.py:83) → potential boot loop.
+the exclusion filters gets routed/lifecycled while half-installed. Since #158,
+`write_traefik_dyn_config` loads each app's metadata in `try/except` and skips the app
+with a `log.warning` — a missing/malformed `app_meta.json` no longer raises
+`MetadataNotFound` out of the lifespan (app_factory.py:83), so it can no longer boot-loop core.
 
 ## 5. Signals discipline (shard_core/util/signals.py, blinker)
 
@@ -232,7 +233,7 @@ All verified 2026-07-03:
 | Weakness | Evidence | Status |
 |---|---|---|
 | No startup reconciliation of stranded *_QUEUED / INSTALLING rows; a crash mid-install strands the row forever (manual uninstall works because `_uninstall_app` asserts nothing) | lifespan app_factory.py:78-108 has no such step; worker.py:124-128 | Open, no issue filed |
-| Stranded INSTALLING/UNINSTALLING row + missing files → `MetadataNotFound` raised uncaught in lifespan step 3 → boot loop | app_installation/util.py:110-114 filters only INSTALLATION_QUEUED and ERROR | Open |
+| Stranded INSTALLING/UNINSTALLING row + missing files → `MetadataNotFound` raised uncaught in lifespan step 3 → boot loop | app_installation/util.py:117-124 now loads metadata per app in `try/except` and skips with a warning | Fixed 2026-07-14, https://github.com/FreeshardBase/freeshard/issues/158 |
 | backup.py blocks the event loop: sync `subprocess.run(["rclone","obscure",...])` (backup.py:186-188) and sync Azure `BlobClient.upload_blob` marker write (backup.py:133-151); rclone command built via `str.split()` breaks on whitespace in SAS URL/paths (backup.py:169) | file:lines cited | Open |
 | Backup task is fire-and-forget with no strong reference — `task` is a local var and the done-callback spawns another unreferenced task (asyncio weak-ref GC footgun); contrast the correct `background_tasks` set pattern in app_lifecycle.py:32-36 | backup.py:62-78 | Open |
 | rclone exit code never checked — success inferred from parsing the last stderr line as JSON; a failure ending in valid JSON records as success, a non-JSON last line raises `JSONDecodeError` | backup.py:168-181 | Open; known trouble spot (repeated churn: commits b9dfcfd, 935250b, d6560a0) |

shard_core/service/app_installation/util.py

@@ -20,6 +20,13 @@
 
 log = logging.getLogger(__name__)
 
+# apps in these states have no complete installation on disk to route to
+_NOT_ROUTED_STATUS = {
+    Status.INSTALLATION_QUEUED.value,
+    Status.UNINSTALLING.value,
+    Status.ERROR.value,
+}
+
 
 async def get_app_from_db(app_name: str) -> InstalledApp:
     async with db_conn() as conn:
@@ -105,13 +112,16 @@ async def write_traefik_dyn_config():
     async with db_conn() as conn:
         all_apps = await db_installed_apps.get_all(conn)
     installed_apps = [
-        InstalledApp(**a) for a in all_apps if a["status"] != Status.INSTALLATION_QUEUED
-    ]
-    app_infos = [
-        AppInfo(get_app_metadata(a.name), installed_app=a)
-        for a in installed_apps
-        if a.status != Status.ERROR
+        InstalledApp(**a) for a in all_apps if a["status"] not in _NOT_ROUTED_STATUS
     ]
+    app_infos = []
+    for app in installed_apps:
+        try:
+            app_infos.append(AppInfo(get_app_metadata(app.name), installed_app=app))
+        except Exception as e:
+            log.warning(
+                f"skipping app {app.name} in traefik config, its metadata could not be loaded: {e!r}"
+            )
 
     async with db_conn() as conn:
         default_row = await db_identities.get_default(conn)

tests/test_traefik_dyn_spec.py

@@ -1,7 +1,14 @@
+import json
+import logging
 from pathlib import Path
 
 import yaml
 
+from shard_core.data_model.app_meta import InstallationReason, Status
+from shard_core.database import installed_apps as db_installed_apps
+from shard_core.database.connection import db_conn
+from shard_core.service.app_installation.util import write_traefik_dyn_config
+from shard_core.service.app_tools import get_installed_apps_path
 from shard_core.settings import settings
 
 
@@ -46,3 +53,73 @@ async def test_template_is_written(api_client):
             "immich_http",
         }
         assert out_routers_http["filebrowser_http"]["service"] == "filebrowser_http"
+
+
+def _write_app_meta(app_name: str):
+    app_path = get_installed_apps_path() / app_name
+    app_path.mkdir(parents=True, exist_ok=True)
+    (app_path / "app_meta.json").write_text(
+        json.dumps(
+            {
+                "v": "1.0",
+                "app_version": "0.1.0",
+                "name": app_name,
+                "pretty_name": app_name,
+                "icon": "icon.svg",
+                "entrypoints": [
+                    {
+                        "container_name": app_name,
+                        "container_port": 80,
+                        "entrypoint_port": "http",
+                    }
+                ],
+                "paths": {"": {"access": "public"}},
+            }
+        )
+    )
+
+
+async def _add_app_to_db(app_name: str, status: Status):
+    async with db_conn() as conn:
+        await db_installed_apps.insert(
+            conn,
+            {
+                "name": app_name,
+                "installation_reason": InstallationReason.CUSTOM.value,
+                "status": status.value,
+                "last_access": None,
+            },
+        )
+
+
+def _read_traefik_dyn_config() -> dict:
+    with open(
+        Path(settings().path_root) / "core" / "traefik_dyn" / "traefik_dyn.yml", "r"
+    ) as f:
+        return yaml.safe_load(f)
+
+
+async def test_app_with_missing_metadata_is_skipped(app_client, memory_logger):
+    _write_app_meta("healthy_app")
+    await _add_app_to_db("healthy_app", Status.RUNNING)
+    await _add_app_to_db("broken_app", Status.RUNNING)
+
+    await write_traefik_dyn_config()
+
+    routers = _read_traefik_dyn_config()["http"]["routers"]
+    assert "healthy_app_http" in routers
+    assert "broken_app_http" not in routers
+    assert [
+        r
+        for r in memory_logger.records
+        if r.levelno == logging.WARNING and "broken_app" in r.getMessage()
+    ]
+
+
+async def test_uninstalling_app_is_not_routed(app_client):
+    _write_app_meta("uninstalling_app")
+    await _add_app_to_db("uninstalling_app", Status.UNINSTALLING)
+
+    await write_traefik_dyn_config()
+
+    assert "uninstalling_app_http" not in _read_traefik_dyn_config()["http"]["routers"]