[eric] apps: serve-static no longer strands the global vite boot lock (every app wedged on Starting preview); deleting an app now removes its workspace tree

backend/apps/outputs/outputs.py

@@ -3,6 +3,7 @@ import json
 import os
 import logging
 import mimetypes
+import shutil
 from datetime import datetime
 from typing import Optional
 from contextlib import asynccontextmanager
@@ -649,7 +650,33 @@ async def update_output(output_id: str, body: OutputUpdate):
 
 @outputs.router.delete("/{output_id}")
 async def delete_output(output_id: str):
-    load(output_id)
+    output = load(output_id)
+    # Delete the source tree too. Dropping only the record left the whole app (source, dist, .env)
+    # on disk with nothing in the UI pointing at it: measured 9 orphans and ~0.85GB on one machine,
+    # and a deleted app is supposed to be GONE, not merely hidden. Worse, the orphan recoverer
+    # re-registers anything with a real name in its meta.json, so a cleared marker resurrects work
+    # the user deliberately deleted. SwarmApps.rollback already did this correctly; match it.
+    workspace_id = getattr(output, "workspace_id", None)
+    if workspace_id:
+        from backend.apps.outputs.runtime import manager as p_delete_manager
+        # Every instance of this app, live or parked: rmtree under a running vite leaves the process
+        # alive on a directory that no longer exists. Runtimes are keyed by workspace_path, which is
+        # the same for every instance, so match on that rather than on the key's shape.
+        p_doomed = [rt for rt in [*p_delete_manager.runtimes.values(), *p_delete_manager.idle_lru.values()]
+                    if rt.workspace_id == workspace_id]
+        for p_rt in p_doomed:
+            try:
+                await p_rt.stop()
+            except Exception:
+                logger.debug("could not stop a runtime for %s before delete", workspace_id, exc_info=True)
+        # realpath + component boundary: a workspace_id is stored data, and rmtree is not a call to
+        # take on trust.
+        root = os.path.realpath(WORKSPACE_DIR)
+        target = os.path.realpath(os.path.join(root, workspace_id))
+        if target != root and target.startswith(root + os.sep):
+            shutil.rmtree(target, ignore_errors=True)
+        else:
+            logger.warning("refusing to delete workspace outside the workspace root: %s", workspace_id)
     path = os.path.join(DATA_DIR, f"{output_id}.json")
     if os.path.exists(path):
         os.remove(path)

backend/apps/outputs/runtime.py

@@ -113,6 +113,8 @@ class AppRuntime:
         self.serve_static: bool = False
         # New-mode only: flips True once something is actually listening on frontend_port (we kick off a background poll task in p_start_new_mode). frontend_url returns null until this flips, so the preview pane doesn't try to navigate to an unbound port and show a "Site can't be reached" error mid-npm-install.
         self.p_frontend_ready: bool = False
+        # True only while a live bind-poll task owns the global vite boot lock; start() releases it otherwise.
+        self.p_boot_lock_handed_off: bool = False
         # True while the process tree is SIGSTOP'd in the idle pool. A frozen vite still holds its port but can't answer it, so frontend_url must stay null while suspended (else the webview loads a dead port = the ERR_FAILED on fast app-switching).
         self.p_suspended: bool = False
         self.process: Optional[asyncio.subprocess.Process] = None
@@ -229,15 +231,17 @@ class AppRuntime:
                 # Acquire the module-level boot lock BEFORE the spawn so only one new-mode workspace is mid-bundle at a time. The lock is released by the bind-poll task the moment vite emits "frontend ready" (or its 180s timeout fires), which is the moment the next workspace can start its own vite without competing for the same CPU. See `p_await_frontend_bind` for the release.
                 p_boot_lock = get_vite_boot_lock()
                 await p_boot_lock.acquire()
+                # Releasing is the DEFAULT; only an actual spawn hands the lock to its bind-poll task.
+                # The old shape released on `not ok`, so the serve-static branch (which returns True
+                # without spawning) held the global lock forever and every later app blocked on the
+                # acquire above with no error anywhere. Owning it here means a new early-return in
+                # p_start_new_mode cannot leak the lock, however it exits.
+                self.p_boot_lock_handed_off = False
                 try:
-                    ok = await self.p_start_new_mode()
-                    if not ok:
-                        # Spawn failed before the bind-poll task was created; release synchronously so we don't wedge the next workspace.
+                    return await self.p_start_new_mode()
+                finally:
+                    if not self.p_boot_lock_handed_off:
                         p_boot_lock.release()
-                    return ok
-                except Exception:
-                    p_boot_lock.release()
-                    raise
             return await self.p_start_old_mode()
 
     async def p_start_new_mode(self) -> bool:
@@ -330,6 +334,8 @@ class AppRuntime:
         # Kick off the port-bind poller so frontend_url flips on once Vite is actually accepting connections.
         self.p_frontend_ready = False
         self.p_frontend_ready_task = asyncio.create_task(self.p_await_frontend_bind())
+        # The poll task now owns the boot lock and is the one that releases it; start() must not.
+        self.p_boot_lock_handed_off = True
         return True
 
     def p_resolve_launch(self, env: dict) -> tuple[list[str], str, str]:

backend/tests/test_delete_app_removes_workspace.py

@@ -0,0 +1,95 @@
+"""Deleting an app must take its source tree with it.
+
+Reported live on 1.7.6-exp.2: delete removed only the record JSON and the versions, so the whole app
+(source, dist, .env) stayed on disk with nothing in the UI pointing at it. Measured on one machine:
+9 orphans, ~0.85GB. Two problems, not one. It is silent disk growth, and it means a deletion the user
+made deliberately did not actually delete anything, which the orphan recoverer can then undo by
+re-registering the folder as a fresh app.
+"""
+
+import os
+from typing import Any
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+
+def p_client(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> tuple[TestClient, str, str]:
+    data_dir = tmp_path / "outputs"
+    ws_dir = tmp_path / "outputs_workspace"
+    data_dir.mkdir()
+    ws_dir.mkdir()
+    import backend.apps.outputs.outputs as mod
+    import backend.apps.outputs.workspace_io as wio
+    monkeypatch.setattr(mod, "DATA_DIR", str(data_dir))
+    monkeypatch.setattr(mod, "WORKSPACE_DIR", str(ws_dir))
+    monkeypatch.setattr(wio, "DATA_DIR", str(data_dir), raising=False)
+    app = FastAPI()
+    app.include_router(mod.outputs.router, prefix="/api/outputs")
+    return TestClient(app), str(data_dir), str(ws_dir)
+
+
+def p_seed(data_dir: str, ws_dir: str, output_id: str, workspace_id: str) -> str:
+    """A record plus a workspace with real content in it, the way a built app looks on disk."""
+    ws = os.path.join(ws_dir, workspace_id)
+    os.makedirs(os.path.join(ws, "frontend", "src"), exist_ok=True)
+    os.makedirs(os.path.join(ws, "frontend", "dist"), exist_ok=True)
+    with open(os.path.join(ws, "frontend", "src", "App.tsx"), "w", encoding="utf-8") as f:
+        f.write("export default function App() { return null }\n")
+    with open(os.path.join(ws, "frontend", "dist", "index.html"), "w", encoding="utf-8") as f:
+        f.write("<html></html>\n")
+    with open(os.path.join(ws, ".env"), "w", encoding="utf-8") as f:
+        f.write("FRONTEND_PORT=50416\n")
+    import json
+    with open(os.path.join(data_dir, f"{output_id}.json"), "w", encoding="utf-8") as f:
+        json.dump({"id": output_id, "name": "Stopwatch", "type": "app",
+                   "workspace_id": workspace_id, "files": {}}, f)
+    return ws
+
+
+def test_deleting_an_app_removes_its_workspace_tree(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None:
+    client, data_dir, ws_dir = p_client(tmp_path, monkeypatch)
+    ws = p_seed(data_dir, ws_dir, "out1", "wsp1")
+    assert os.path.isdir(ws)
+
+    assert client.delete("/api/outputs/out1").status_code == 200
+
+    assert not os.path.exists(ws), "the app's source tree survived its own deletion"
+    assert not os.path.exists(os.path.join(data_dir, "out1.json"))
+
+
+def test_deleting_one_app_leaves_every_other_workspace_alone(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None:
+    client, data_dir, ws_dir = p_client(tmp_path, monkeypatch)
+    doomed = p_seed(data_dir, ws_dir, "out1", "wsp1")
+    keeper = p_seed(data_dir, ws_dir, "out2", "wsp2")
+
+    assert client.delete("/api/outputs/out1").status_code == 200
+
+    assert not os.path.exists(doomed)
+    assert os.path.isfile(os.path.join(keeper, "frontend", "src", "App.tsx"))
+
+
+def test_a_record_with_no_workspace_still_deletes_cleanly(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Old records and non-app outputs have no workspace_id; delete must not care."""
+    client, data_dir, ws_dir = p_client(tmp_path, monkeypatch)
+    import json
+    with open(os.path.join(data_dir, "out3.json"), "w", encoding="utf-8") as f:
+        json.dump({"id": "out3", "name": "A chart", "type": "view", "files": {}}, f)
+
+    assert client.delete("/api/outputs/out3").status_code == 200
+    assert not os.path.exists(os.path.join(data_dir, "out3.json"))
+
+
+def test_a_workspace_id_that_escapes_the_root_is_refused(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None:
+    """workspace_id is stored data, and rmtree is not a call to take on trust."""
+    client, data_dir, ws_dir = p_client(tmp_path, monkeypatch)
+    outside = tmp_path / "precious"
+    outside.mkdir()
+    (outside / "keep.txt").write_text("do not delete me", encoding="utf-8")
+    import json
+    with open(os.path.join(data_dir, "out4.json"), "w", encoding="utf-8") as f:
+        json.dump({"id": "out4", "name": "Evil", "type": "app",
+                   "workspace_id": "../precious", "files": {}}, f)
+
+    assert client.delete("/api/outputs/out4").status_code == 200
+    assert (outside / "keep.txt").is_file(), "delete escaped the workspace root"

backend/tests/test_vite_boot_lock_never_leaks.py

@@ -0,0 +1,104 @@
+"""The global vite boot lock must be free again after any start() that did not spawn.
+
+Reported live on 1.7.6-exp.2: the serve-static branch returns True without spawning, the old release
+was guarded by `if not ok`, and the only other releaser is the bind-poll task that a serve-static
+start never creates. So the FIRST app that qualified for serve-static held the process-global lock
+forever and every app opened afterwards blocked on `acquire()` with no error, no traceback and no
+non-200: twelve apps stuck on "Starting preview" until a restart. These assert the lock is free after
+each non-spawning exit, which is the invariant, rather than asserting the shape of any one branch.
+"""
+
+import asyncio
+from typing import Any, List
+import pytest
+from backend.apps.outputs.runtime import AppRuntime, get_vite_boot_lock
+
+
+def p_make_runtime(tmp_path: Any, name: str = "ws") -> AppRuntime:
+    # is_new_mode is a property over the workspace layout: a run.sh at the root is what makes it new.
+    ws = tmp_path / name
+    ws.mkdir(exist_ok=True)
+    (ws / "run.sh").write_text("#!/bin/bash\necho hi\n", encoding="utf-8")
+    rt = AppRuntime(workspace_id=name, workspace_path=str(ws))
+    assert rt.is_new_mode, "the fixture must build a new-mode workspace or start() takes the old path"
+    return rt
+
+
+@pytest.mark.asyncio
+async def test_serve_static_start_leaves_the_boot_lock_free(tmp_path: Any) -> None:
+    rt = p_make_runtime(tmp_path)
+
+    async def p_serve_static_exit() -> bool:
+        rt.serve_static = True
+        return True
+
+    setattr(rt, "p_start_new_mode", p_serve_static_exit)
+    assert await rt.start() is True
+    assert rt.serve_static is True
+    assert not get_vite_boot_lock().locked(), "serve-static start held the global vite boot lock"
+
+
+@pytest.mark.asyncio
+async def test_a_second_app_can_still_start_after_a_serve_static_one(tmp_path: Any) -> None:
+    """The actual user-visible failure: app #2 never gets past acquire()."""
+    first = p_make_runtime(tmp_path)
+    setattr(first, "p_start_new_mode", lambda: p_true_serving(first))
+    await first.start()
+
+    second = p_make_runtime(tmp_path, "ws2")
+    reached: List[str] = []
+
+    async def p_second_body() -> bool:
+        reached.append("spawned")
+        return True
+
+    setattr(second, "p_start_new_mode", p_second_body)
+    # 2s is generous: without the fix this never returns at all.
+    await asyncio.wait_for(second.start(), timeout=2.0)
+    assert reached == ["spawned"], "the second app never reached its spawn body"
+
+
+async def p_true_serving(rt: AppRuntime) -> bool:
+    rt.serve_static = True
+    return True
+
+
+@pytest.mark.asyncio
+async def test_a_failed_start_leaves_the_boot_lock_free(tmp_path: Any) -> None:
+    rt = p_make_runtime(tmp_path)
+
+    async def p_fail() -> bool:
+        return False
+
+    setattr(rt, "p_start_new_mode", p_fail)
+    assert await rt.start() is False
+    assert not get_vite_boot_lock().locked()
+
+
+@pytest.mark.asyncio
+async def test_a_raising_start_leaves_the_boot_lock_free(tmp_path: Any) -> None:
+    rt = p_make_runtime(tmp_path)
+
+    async def p_boom() -> bool:
+        raise RuntimeError("spawn blew up")
+
+    setattr(rt, "p_start_new_mode", p_boom)
+    with pytest.raises(RuntimeError):
+        await rt.start()
+    assert not get_vite_boot_lock().locked()
+
+
+@pytest.mark.asyncio
+async def test_a_real_spawn_hands_the_lock_to_its_poll_task(tmp_path: Any) -> None:
+    """The other direction: a genuine spawn must KEEP the lock, or the serialization it exists for
+    is gone and three apps pre-bundle in parallel again."""
+    rt = p_make_runtime(tmp_path)
+
+    async def p_spawned() -> bool:
+        rt.p_boot_lock_handed_off = True
+        return True
+
+    setattr(rt, "p_start_new_mode", p_spawned)
+    assert await rt.start() is True
+    assert get_vite_boot_lock().locked(), "a real spawn must hold the lock for its bind-poll task"
+    get_vite_boot_lock().release()