From 571aee4a611b062ac74d5577d85e9d5924bdf8c1 Mon Sep 17 00:00:00 2001
From: Arthur <hello@arthware.dev>
Date: Tue, 15 Sep 2026 22:01:19 +0200
Subject: [PATCH] fix(cli): re-render .env on down so a new compose var can't
 wedge stop

A stale .env missing a merged-in variable (MEDIA_ARCHIVE_DIR) made
compose reject every command; down now refreshes env like up, and
surfaces the compose error instead of 'unknown error'.
---
 lib/stack/cli.py                  | 24 ++++++++++++++------
 tests/framework/test_cli_class.py | 37 +++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/lib/stack/cli.py b/lib/stack/cli.py
index 1e730aed..1d777b6c 100644
--- a/lib/stack/cli.py
+++ b/lib/stack/cli.py
@@ -319,14 +319,24 @@ def down(self, stacklet_id: str) -> dict:
         stacklet_dir = Path(result.get("path", ""))
         compose_file = docker.find_compose_file(stacklet_dir)
         if compose_file:
-            if not (stacklet_dir / ".env").exists():
-                try:
-                    self.stack.refresh_env(stacklet_id)
-                except ValueError:
-                    pass
+            # Always re-render .env first, exactly as up does. compose reads
+            # the whole file before it acts, so a stale .env that is missing a
+            # newly-added variable (a merge that adds a mount, for example)
+            # makes every command invalid, stop included, and the stacklet
+            # becomes unstoppable until env is regenerated by hand. Refreshing
+            # only when .env is absent left that gap.
+            try:
+                self.stack.refresh_env(stacklet_id)
+            except ValueError:
+                pass
             code, output = docker.compose_stop(compose_file)
-            return {"stacklet": stacklet_id, "action": "down",
-                    "success": code == 0, "output": output}
+            result = {"stacklet": stacklet_id, "action": "down",
+                      "success": code == 0, "output": output}
+            if code != 0:
+                # Surface the compose error instead of the caller's generic
+                # "unknown error", which hid the real cause.
+                result["error"] = output or "docker compose stop failed"
+            return result
         return {"stacklet": stacklet_id, "action": "down", "success": True}
 
     def _down_all(self) -> dict:
diff --git a/tests/framework/test_cli_class.py b/tests/framework/test_cli_class.py
index 753cec9f..da6956dd 100644
--- a/tests/framework/test_cli_class.py
+++ b/tests/framework/test_cli_class.py
@@ -216,6 +216,43 @@ def test_unknown_stacklet_fails(self, tmp_path):
         result = cli.down("nope")
         assert "error" in result
 
+    def test_down_refreshes_env_even_when_env_exists(self, tmp_path, monkeypatch):
+        """A stale .env must be re-rendered before compose stop.
+
+        compose reads the whole file up front, so a .env missing a
+        variable a later merge added makes stop fail and the stacklet
+        unstoppable. up already refreshes; down must too. Refreshing only
+        when .env was absent was the gap that made memory unstoppable
+        after MEDIA_ARCHIVE_DIR was added to its compose mounts.
+        """
+        cli, _ = _make_cli(tmp_path, {"myapp": {}})
+        sdir = tmp_path / "stacklets" / "myapp"
+        (sdir / "docker-compose.yml").write_text("services: {}\n")
+        (sdir / ".env").write_text("STALE=1\n")  # exists, but out of date
+        refreshed = []
+        monkeypatch.setattr(cli.stack, "refresh_env",
+                            lambda sid: refreshed.append(sid) or {})
+        with patch("stack.docker.compose_stop", return_value=(0, "")):
+            result = cli.down("myapp")
+        assert result["success"]
+        assert refreshed == ["myapp"], "down must re-render .env before compose acts"
+
+    def test_down_surfaces_the_compose_error(self, tmp_path, monkeypatch):
+        """A failed stop reports the real cause, not "unknown error".
+
+        The result used to carry only `output`, which the caller dropped,
+        so the operator saw "unknown error" for a specific compose fault.
+        """
+        cli, _ = _make_cli(tmp_path, {"myapp": {}})
+        sdir = tmp_path / "stacklets" / "myapp"
+        (sdir / "docker-compose.yml").write_text("services: {}\n")
+        monkeypatch.setattr(cli.stack, "refresh_env", lambda sid: {})
+        fault = "invalid spec: :/vault/media:ro: empty section between colons"
+        with patch("stack.docker.compose_stop", return_value=(1, fault)):
+            result = cli.down("myapp")
+        assert not result["success"]
+        assert fault in result["error"]
+
 
 class TestCLIDownAll:
     """`stack down all` stops every currently-running stacklet in reverse