[eric] merge eric/app-orphans: a second app instance was classified as a dead app and torn off the canvas

backend/apps/workflows/storage.py

@@ -10,6 +10,7 @@ last_run_* / next_run_at summary fields; full history lives in the runs file.
 """
 
 import json
+import logging
 import os
 import tempfile
 from threading import Lock
@@ -18,6 +19,8 @@ from typing import Optional
 from backend.config.paths import DATA_ROOT
 from backend.apps.workflows.models import Workflow, WorkflowRun, MissedRun
 
+logger = logging.getLogger(__name__)
+
 DATA_DIR = os.path.join(DATA_ROOT, "workflows")
 RUNS_DIR = os.path.join(DATA_DIR, "runs")
 PAUSED_FILE = os.path.join(DATA_DIR, "paused.json")
@@ -99,7 +102,9 @@ def _load_all_from_disk() -> None:
             if wf.schedule.timezone == "local":
                 wf.schedule.timezone = host_tz
             _workflow_cache[wf.id] = wf
-        except Exception:
+        except Exception as e:
+            # A dropped file is a workflow that vanished from the UI while sitting right there on disk; say so or nobody can ever explain it.
+            logger.warning("Skipping unreadable workflow file %s: %s", fname, e)
             continue
     if os.path.exists(RUNS_DIR):
         for fname in os.listdir(RUNS_DIR):

backend/tests/test_workflows_storage.py

@@ -11,9 +11,13 @@ Run:
 
 from __future__ import annotations
 
+import io
 import json
+import logging
 import os
+from contextlib import contextmanager
 from datetime import datetime, timedelta, timezone
+from typing import Iterator
 
 import pytest
 
@@ -23,6 +27,21 @@ def _wf_env(isolated_workflows_data):
     yield
 
 
+@contextmanager
+def p_storage_warnings() -> Iterator[io.StringIO]:
+    """Not caplog: backend/main.py pins propagate=False on the 'backend' logger, and
+    caplog listens at the root, so these records only exist if you sit on the logger itself."""
+    from backend.apps.workflows import storage
+    buf = io.StringIO()
+    handler = logging.StreamHandler(buf)
+    handler.setLevel(logging.WARNING)
+    storage.logger.addHandler(handler)
+    try:
+        yield buf
+    finally:
+        storage.logger.removeHandler(handler)
+
+
 # --- atomic write ------------------------------------------------------------
 
 def test_atomic_write_round_trips(tmp_path):
@@ -60,7 +79,7 @@ def test_atomic_write_leaves_no_temp_and_preserves_old_on_failure(monkeypatch):
 
 def test_corrupt_workflow_record_is_skipped_not_fatal(make_wf):
     """A truncated <id>.json must not take down the whole load; the bad record
-    silently drops out and the good ones still come back."""
+    drops out and the good ones still come back."""
     from backend.apps.workflows import storage
     good = make_wf(title="good")
     storage.save_workflow(good)
@@ -74,6 +93,37 @@ def test_corrupt_workflow_record_is_skipped_not_fatal(make_wf):
     assert "broken" not in ids
 
 
+def test_dropped_workflow_record_names_the_file_it_dropped(make_wf):
+    """The drop is what makes a workflow vanish from the UI with its file still
+    on disk. If it happens without naming the file, nobody can ever diagnose it."""
+    from backend.apps.workflows import storage
+    storage.save_workflow(make_wf(title="good"))
+    storage._ensure_dirs()
+    with open(os.path.join(storage.DATA_DIR, "broken.json"), "w") as f:
+        f.write('{"id": "broken", "title": "trunc')
+
+    storage._cache_loaded = False
+    with p_storage_warnings() as logged:
+        storage.list_workflows()
+    assert "broken.json" in logged.getvalue()
+
+
+def test_record_a_newer_build_wrote_is_reported_when_it_fails_to_load():
+    """Every Workflow field has a default, so unknown keys survive a downgrade
+    fine. A field whose VALUE the running build's schema rejects does not: the
+    record takes the same exit as a truncated file, and must say so."""
+    from backend.apps.workflows import storage
+    storage._ensure_dirs()
+    with open(os.path.join(storage.DATA_DIR, "fromfuture.json"), "w") as f:
+        json.dump({"id": "fromfuture", "execution_target": "orbital-relay"}, f)
+
+    storage._cache_loaded = False
+    with p_storage_warnings() as logged:
+        ids = {w.id for w in storage.list_workflows()}
+    assert "fromfuture" not in ids
+    assert "fromfuture.json" in logged.getvalue()
+
+
 def test_corrupt_runs_file_yields_empty_history(make_wf):
     from backend.apps.workflows import storage
     wf = make_wf()

frontend/src/app/pages/Dashboard/hooks/lifecycle/orphanViewCardKeys.test.ts

@@ -0,0 +1,38 @@
+// Run: node --test frontend/src/app/pages/Dashboard/hooks/lifecycle/orphanViewCardKeys.test.ts
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { orphanViewCardKeys } from './orphanViewCardKeys.ts';
+
+const app = { id: 'app1', name: 'Calculator' };
+
+test('a live app keeps its primary card', () => {
+  const cards = { app1: { output_id: 'app1' } };
+  assert.deepEqual(orphanViewCardKeys(cards, { app1: app }), []);
+});
+
+test('a second instance of a live app is not an orphan', () => {
+  const cards = { app1: { output_id: 'app1' }, 'app1#2': { output_id: 'app1' } };
+  assert.deepEqual(orphanViewCardKeys(cards, { app1: app }), []);
+});
+
+test('deleting the app orphans every instance of it', () => {
+  const cards = {
+    app1: { output_id: 'app1' },
+    'app1#2': { output_id: 'app1' },
+    'app1#3': { output_id: 'app1' },
+  };
+  assert.deepEqual(orphanViewCardKeys(cards, {}), ['app1', 'app1#2', 'app1#3']);
+});
+
+test('one deleted app does not take a surviving app down with it', () => {
+  const cards = {
+    app1: { output_id: 'app1' },
+    'app1#2': { output_id: 'app1' },
+    app2: { output_id: 'app2' },
+  };
+  assert.deepEqual(orphanViewCardKeys(cards, { app2: { id: 'app2' } }), ['app1', 'app1#2']);
+});
+
+test('no cards means nothing to prune', () => {
+  assert.deepEqual(orphanViewCardKeys({}, { app1: app }), []);
+});

frontend/src/app/pages/Dashboard/hooks/lifecycle/orphanViewCardKeys.ts

@@ -0,0 +1,11 @@
+// A view card's record key is the bare output id ONLY for the primary; every extra instance is
+// `${output_id}#N`. Reading the key as if it were an output id therefore pronounced every secondary
+// instance dead on arrival and swept it off the canvas. Always resolve through output_id.
+export function orphanViewCardKeys(
+  viewCards: Readonly<Record<string, { output_id: string }>>,
+  outputs: Readonly<Record<string, unknown>>,
+): string[] {
+  return Object.entries(viewCards)
+    .filter(([, card]) => !outputs[card.output_id])
+    .map(([cardKey]) => cardKey);
+}

frontend/src/app/pages/Dashboard/hooks/lifecycle/useDashboardLifecycle.ts

@@ -31,6 +31,7 @@ import { initBrowserCommandHandler } from '@/shared/browserCommandHandler';
 import { getKeepAliveBrowserIds } from '@/shared/browserFocus';
 import { prepareDashboardSwitch } from '@/shared/dashboardSwitchTeardown';
 import { removeViewCardCleanly } from '@/shared/viewTeardown';
+import { orphanViewCardKeys } from './orphanViewCardKeys';
 import { clearPendingBrowserUrl, clearPendingFocusAgentId } from '@/shared/state/tempStateSlice';
 import { API_BASE } from '@/shared/config';
 import type { CanvasActions } from '../interaction/useCanvasControls';
@@ -313,7 +314,7 @@ export function useDashboardLifecycle({
   // Prune orphan view cards whose underlying output was deleted (e.g. via the Views page). Without this, the layout entry persists in the minimap and contentBounds even though DashboardViewCard renders nothing. Gated on outputsRefetched (THIS open's fresh fetch), NOT the sticky global outputsLoaded: on a freshly-imported dashboard the global flag is already true from a prior dashboard, so the old gate pruned the just-imported app card against a stale apps list and the debounced save persisted the wipe.
   useEffect(() => {
     if (!layoutInitialized || !outputsRefetched) return;
-    const orphans = Object.keys(viewCards).filter((id) => !outputs[id]);
+    const orphans = orphanViewCardKeys(viewCards, outputs);
     if (orphans.length === 0) return;
     // Serialize teardown (quiesce each GPU surface first): pruning several orphaned app cards in one
     // pass would rip their webviews out simultaneously, the same GPU-process SIGSEGV as mass delete.