fix(ui): surface kept_from_previous_deployment in the deploy result (CashPilot-23yb) (#328)

* fix(ui): surface kept_from_previous_deployment in the deploy result (CashPilot-23yb)

A redeploy rebuilds from the RECORDED spec where it diverges from the
catalog, and the API has always reported what it kept — but nothing
rendered the field, so the operator saw a plain success while their
typed-in values were quietly superseded. The divergence list now lands
as a warning toast per deploy, e.g. 'storj: kept from the previous
deployment — resources: keeping the limits this service was deployed
with'.

* fix(deploy): let the deploy response carry kept_from_previous_deployment

The route was annotated -> dict[str, str], and FastAPI validates the
response against that annotation AFTER the deploy has run and persisted:
exactly the divergent redeployments that had something to tell the user
500'd, and a retry could deploy again. Proven through HTTP before the
fix; negative control pins that a plain deploy carries no kept key.

Author: GeiserX

app/main.py

@@ -1567,7 +1567,11 @@ async def api_deploy(
     body: DeployRequest,
     worker_id: int | None = None,
     _auth: dict[str, Any] = Depends(_require_owner),
-) -> dict[str, str]:
+) -> dict[str, Any]:
+    # dict[str, Any], not dict[str, str]: kept_from_previous_deployment is a
+    # list, and FastAPI validates the response against this annotation AFTER
+    # the deploy has run and persisted — a stricter type turns the successful
+    # deployments that had something to report into 500s.
     worker_id = await _resolve_worker_id(worker_id)
     svc = catalog.get_service(slug)
     if not svc:

app/static/js/app.js

@@ -2386,8 +2386,16 @@ const CP = (() => {
     let ok = 0, fail = 0;
     for (const wid of workerIds) {
       try {
-        await api(`/api/deploy/${slug}?worker_id=${wid}`, { method: 'POST', body: { env } });
+        const res = await api(`/api/deploy/${slug}?worker_id=${wid}`, { method: 'POST', body: { env } });
         ok++;
+        // A redeploy rebuilds from the RECORDED spec where it diverges from
+        // the catalog. The API has always reported what it kept; nothing
+        // rendered it, so the operator saw a plain success while e.g. their
+        // typed-in catalog values were quietly superseded (CashPilot-23yb).
+        const kept = res && res.kept_from_previous_deployment;
+        if (Array.isArray(kept) && kept.length) {
+          toast(`${slug}: kept from the previous deployment — ${kept.join('; ')}`, 'warning');
+        }
       } catch (err) {
         fail++;
         toast(`Deploy to worker ${wid} failed: ${err.message}`, 'error');

tests/test_main_deploy_routes.py

@@ -135,6 +135,83 @@ def test_deploy_success(self, client):
             data = resp.json()
             assert data["status"] == "deployed"
 
+    def test_divergent_redeploy_returns_the_kept_list_over_http(self, client):
+        """CashPilot-23yb: the deploy response must survive FastAPI's response model.
+
+        `kept_from_previous_deployment` is a list, and the route used to be
+        annotated `-> dict[str, str]` — so the very deployments that had
+        something to tell the user 500'd AFTER deploying and persisting, and a
+        retry could deploy again. Exercised through HTTP because a direct call
+        to the function bypasses response validation entirely.
+        """
+        svc = {
+            "slug": "honeygain",
+            "name": "Honeygain",
+            "docker": {
+                "image": "honeygain/honeygain:latest",
+                "env": [{"key": "EMAIL", "default": "user@test.com"}],
+                "ports": ["8080:80/tcp"],
+                "volumes": ["/data:/app/data"],
+            },
+        }
+        # The recorded container ran with a command the catalog no longer has:
+        # _merge_recorded_spec keeps it and reports the divergence.
+        recorded = {"image": "honeygain/honeygain:latest", "command": "--legacy-flag"}
+        worker = _online_worker()
+
+        async def _fake_deploy(worker_id, slug, spec):
+            return {"container_id": "abc123"}
+
+        with (
+            _auth_owner(),
+            patch("app.main.database.list_workers", new_callable=AsyncMock, return_value=[worker]),
+            patch("app.main.catalog.get_service", return_value=svc),
+            patch("app.main.database.get_worker", new_callable=AsyncMock, return_value=worker),
+            patch(
+                "app.main.database.get_deployment_spec",
+                new_callable=AsyncMock,
+                return_value=recorded,
+            ),
+            patch("app.main._proxy_worker_deploy", side_effect=_fake_deploy),
+            patch("app.main.database.save_deployment", new_callable=AsyncMock),
+            patch("app.main.database.record_health_event", new_callable=AsyncMock),
+            patch("app.main._run_collection", new_callable=AsyncMock),
+        ):
+            resp = client.post("/api/deploy/honeygain", json={"env": {}})
+            assert resp.status_code == 200, resp.text
+            data = resp.json()
+            assert data["status"] == "deployed"
+            kept = data["kept_from_previous_deployment"]
+            assert isinstance(kept, list) and kept
+            assert any("command" in line for line in kept)
+
+    def test_plain_deploy_response_has_no_kept_key(self, client):
+        # Negative control: without a recorded spec there is no divergence and
+        # the key must be absent — the toast only fires when there is news.
+        svc = {
+            "slug": "honeygain",
+            "name": "Honeygain",
+            "docker": {"image": "honeygain/honeygain:latest", "env": []},
+        }
+        worker = _online_worker()
+
+        async def _fake_deploy(worker_id, slug, spec):
+            return {"container_id": "abc123"}
+
+        with (
+            _auth_owner(),
+            patch("app.main.database.list_workers", new_callable=AsyncMock, return_value=[worker]),
+            patch("app.main.catalog.get_service", return_value=svc),
+            patch("app.main.database.get_worker", new_callable=AsyncMock, return_value=worker),
+            patch("app.main._proxy_worker_deploy", side_effect=_fake_deploy),
+            patch("app.main.database.save_deployment", new_callable=AsyncMock),
+            patch("app.main.database.record_health_event", new_callable=AsyncMock),
+            patch("app.main._run_collection", new_callable=AsyncMock),
+        ):
+            resp = client.post("/api/deploy/honeygain", json={"env": {}})
+            assert resp.status_code == 200, resp.text
+            assert "kept_from_previous_deployment" not in resp.json()
+
     def test_deploy_service_not_found(self, client):
         with (
             _auth_owner(),