Merge pull request #331 from naturallaw777/copilot/fix-manual-backup-failure

Fix Manual Backup exit-code-127: add bash+gawk to service PATH, harden launcher

app/sovran_systemsos_web/server.py

@@ -3742,8 +3742,18 @@ async def api_backup_drives():
 
 
 async def _monitor_backup_subprocess(proc: asyncio.subprocess.Process) -> None:
-    """Mark status FAILED if backup subprocess exits unexpectedly."""
+    """Drain stderr, then mark status FAILED if backup subprocess exits unexpectedly."""
+    stderr_chunks: list[bytes] = []
+
+    async def _drain_stderr() -> None:
+        if proc.stderr is not None:
+            async for line in proc.stderr:
+                stderr_chunks.append(line)
+
+    drain_task = asyncio.create_task(_drain_stderr())
     rc = await proc.wait()
+    await drain_task
+
     if rc == 0:
         return
 
@@ -3752,7 +3762,12 @@ async def _monitor_backup_subprocess(proc: asyncio.subprocess.Process) -> None:
     if status in {"SUCCESS", "FAILED"}:
         return
 
-    msg = f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Backup subprocess exited unexpectedly (code {rc})."
+    detail = ""
+    if stderr_chunks:
+        stderr_text = b"".join(stderr_chunks).decode("utf-8", errors="replace").strip()
+        if stderr_text:
+            detail = f" — stderr: {stderr_text}"
+    msg = f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Backup subprocess exited unexpectedly (code {rc}).{detail}"
     await loop.run_in_executor(None, _append_backup_log, msg)
     await loop.run_in_executor(None, _write_backup_status, "FAILED")
 
@@ -3808,11 +3823,25 @@ async def api_backup_run(target: str = ""):
     env = dict(os.environ)
     env["BACKUP_TARGET"] = selected_target
 
+    bash_path = shutil.which("bash")
+    if bash_path is None:
+        no_bash_msg = (
+            f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Cannot start backup:"
+            " interpreter 'bash' not found on PATH."
+            " Ensure pkgs.bash is in the sovran-hub-web service PATH."
+        )
+        await loop.run_in_executor(None, _append_backup_log, no_bash_msg)
+        await loop.run_in_executor(None, _write_backup_status, "FAILED")
+        raise HTTPException(
+            status_code=500,
+            detail="Backup interpreter (bash) not available. Check service PATH configuration.",
+        )
+
     try:
         proc = await asyncio.create_subprocess_exec(
-            "/usr/bin/env", "bash", BACKUP_SCRIPT,
+            bash_path, BACKUP_SCRIPT,
             stdout=asyncio.subprocess.DEVNULL,
-            stderr=asyncio.subprocess.DEVNULL,
+            stderr=asyncio.subprocess.PIPE,
             env=env,
         )
     except Exception as exc:

app/tests/test_manual_backup_workflow.py

@@ -1,3 +1,4 @@
+import asyncio
 import unittest
 from pathlib import Path
 
@@ -6,6 +7,7 @@ REPO_ROOT = Path(__file__).resolve().parents[2]
 BACKUP_SCRIPT = REPO_ROOT / "app" / "sovran_systemsos_web" / "scripts" / "sovran-hub-backup.sh"
 SERVER_FILE = REPO_ROOT / "app" / "sovran_systemsos_web" / "server.py"
 SUPPORT_JS = REPO_ROOT / "app" / "sovran_systemsos_web" / "static" / "js" / "support.js"
+NIX_HUB_FILE = REPO_ROOT / "modules" / "core" / "sovran-hub.nix"
 
 
 class ManualBackupWorkflowTests(unittest.TestCase):
@@ -36,6 +38,106 @@ class ManualBackupWorkflowTests(unittest.TestCase):
         self.assertIn("asyncio.create_task(_monitor_backup_subprocess(proc))", server_source)
         self.assertIn("result === \"success\" || result === \"failed\"", support_source)
 
+    # ── Regression tests for exit-code-127 / missing-interpreter bug ──────────
+
+    def test_nix_service_path_includes_bash_and_gawk(self):
+        """Regression: pkgs.bash and pkgs.gawk must appear in the sovran-hub-web
+        service path so that the backup shell script and its awk calls can be
+        resolved at runtime without producing exit code 127."""
+        nix_source = NIX_HUB_FILE.read_text()
+        self.assertIn(
+            "pkgs.bash",
+            nix_source,
+            "pkgs.bash must be declared in the sovran-hub-web systemd service path",
+        )
+        self.assertIn(
+            "pkgs.gawk",
+            nix_source,
+            "pkgs.gawk must be declared in the sovran-hub-web systemd service path",
+        )
+
+    def test_server_resolves_bash_via_shutil_which(self):
+        """Regression: server must locate bash with shutil.which() rather than
+        relying on /usr/bin/env bash, so a missing interpreter is caught early
+        with a clear diagnostic instead of a cryptic exit-code-127 failure."""
+        source = SERVER_FILE.read_text()
+        self.assertIn(
+            'shutil.which("bash")',
+            source,
+            "server.py must resolve bash via shutil.which to catch missing interpreter",
+        )
+        self.assertNotIn(
+            '"/usr/bin/env", "bash"',
+            source,
+            "server.py must not use /usr/bin/env bash (relies on PATH, causes code 127)",
+        )
+
+    def test_server_logs_actionable_message_when_bash_missing(self):
+        """Regression: when bash is absent the server must write an actionable log
+        entry and set FAILED status before returning an HTTP error."""
+        source = SERVER_FILE.read_text()
+        self.assertIn(
+            "bash_path is None",
+            source,
+            "server.py must check that bash_path is not None before launching",
+        )
+        self.assertIn(
+            "interpreter",
+            source,
+            "server.py missing-bash error message must reference 'interpreter'",
+        )
+
+    def test_server_captures_stderr_from_backup_subprocess(self):
+        """Regression: the backup subprocess must use stderr=PIPE so that any
+        startup error (e.g. code 127 from a missing command) is captured and
+        surfaced in the backup log rather than being silently discarded."""
+        source = SERVER_FILE.read_text()
+        self.assertIn(
+            "stderr=asyncio.subprocess.PIPE",
+            source,
+            "backup subprocess must use stderr=PIPE to capture diagnostic output",
+        )
+        self.assertIn(
+            "stderr_text",
+            source,
+            "_monitor_backup_subprocess must capture and log stderr content",
+        )
+
+    def test_monitor_includes_stderr_detail_in_failed_message(self):
+        """Regression: _monitor_backup_subprocess must append captured stderr to
+        the FAILED log entry so the UI shows what went wrong (e.g. 'bash: not
+        found') rather than only the raw exit code."""
+        source = SERVER_FILE.read_text()
+        self.assertIn("stderr_chunks", source)
+        self.assertIn("stderr_text", source)
+        # stderr detail is conditionally appended only when non-empty
+        self.assertIn('detail = f" — stderr: {stderr_text}"', source)
+
+    def test_exit_code_127_subprocess_stderr_drain(self):
+        """Behavioral regression: a subprocess that exits 127 must have its
+        stderr drained without deadlock by the async drain loop."""
+
+        async def _run():
+            proc = await asyncio.create_subprocess_exec(
+                "bash", "-c", "echo 'bash: command not found' >&2; exit 127",
+                stdout=asyncio.subprocess.DEVNULL,
+                stderr=asyncio.subprocess.PIPE,
+            )
+            chunks: list[bytes] = []
+
+            async def _drain():
+                async for line in proc.stderr:
+                    chunks.append(line)
+
+            drain_task = asyncio.create_task(_drain())
+            rc = await proc.wait()
+            await drain_task
+            return rc, b"".join(chunks).decode()
+
+        rc, stderr_out = asyncio.run(_run())
+        self.assertEqual(rc, 127)
+        self.assertIn("bash: command not found", stderr_out)
+
 
 if __name__ == "__main__":
     unittest.main()

modules/core/sovran-hub.nix

@@ -382,6 +382,8 @@ in
       };
 
       path = [
+        pkgs.bash
+        pkgs.gawk
         pkgs.qrencode
         pkgs.curl
         pkgs.iproute2