fix(runtime): contain and supervise detached Bash jobs (ODY-145)

src/bg_jobs.py

@@ -22,18 +22,16 @@ from __future__ import annotations
 
 import json
 import os
-import shlex
+import sys
 import subprocess
 import time
 import uuid
 from pathlib import Path
 from typing import Any, Dict, List, Optional
 
-from core.atomic_io import atomic_write_json
+from core.atomic_io import atomic_write_json, store_transaction
 from core.platform_compat import (
     detached_popen_kwargs,
-    find_bash,
-    git_bash_path,
     kill_process_tree,
     pid_alive,
 )
@@ -53,6 +51,7 @@ _MAX_OUTPUT_CHARS = 16000
 # files) is kept before pruning, so neither the store nor data/bg_jobs/ grows
 # without bound. The agent has already consumed the result by then.
 _RETENTION_S = 3600  # 1 hour after follow-up
+_LIVE_PROCS: dict[int, subprocess.Popen] = {}
 
 
 def _load() -> Dict[str, Dict[str, Any]]:
@@ -79,66 +78,50 @@ def _pid_alive(pid: Optional[int]) -> bool:
     return pid_alive(pid)
 
 
+@store_transaction(lambda: _STORE)
 def launch(command: str, session_id: str, cwd: Optional[str] = None,
-           max_runtime_s: int = DEFAULT_MAX_RUNTIME_S) -> Dict[str, Any]:
+           max_runtime_s: int = DEFAULT_MAX_RUNTIME_S, env: Optional[dict] = None) -> Dict[str, Any]:
     """Launch `command` detached. Returns the job record (status='running').
 
-    Output + the final exit code are written to files so status survives a
-    server restart. The process is put in its own session (setsid) so it
-    outlives the request/stream that started it.
+    A trusted detached supervisor owns the shared containment runner, output,
+    wall clock and exit metadata, independently of the request/server lifetime.
     """
     _JOBS_DIR.mkdir(parents=True, exist_ok=True)
     job_id = uuid.uuid4().hex[:12]
     log_path = _JOBS_DIR / f"{job_id}.log"
     exit_path = _JOBS_DIR / f"{job_id}.exit"
 
-    # The user command goes in its OWN script file, run as a child `bash`. This
-    # is what isolates it: an `exit` inside it only ends that child (so the
-    # wrapper still records the exit code), and — unlike textually wrapping the
-    # command in `( … )` — the wrapper can't be broken by an unbalanced paren or
-    # a trailing line-continuation in the command. `$?` is the child's real
-    # exit status.
-    bash = find_bash()
-    if bash:
-        # POSIX, or Windows with Git Bash/WSL. The user command goes in its OWN
-        # script file, run as a child `bash` — an `exit` inside it only ends
-        # that child (so the wrapper still records the exit code), and an
-        # unbalanced paren / trailing line-continuation in the command can't
-        # break the wrapper. `$?` is the child's real exit status. Paths are
-        # emitted as POSIX (forward-slash) + shell-quoted so Git Bash on Windows
-        # handles drive paths and spaces correctly.
-        cmd_path = _JOBS_DIR / f"{job_id}.cmd.sh"
-        cmd_path.write_text(command + "\n", encoding="utf-8")
-        lp, xp, cp = (shlex.quote(git_bash_path(p)) for p in (log_path, exit_path, cmd_path))
-        script_path = _JOBS_DIR / f"{job_id}.sh"
-        script_path.write_text(
-            f"bash {cp} > {lp} 2>&1\n"
-            f"echo $? > {xp}\n",
-            encoding="utf-8",
-        )
-        argv = [bash, str(script_path)]
-    else:
-        # Windows without any bash installed: cmd.exe wrapper. The command runs
-        # in its own child .cmd so %ERRORLEVEL% is the command's real exit code.
-        child_path = _JOBS_DIR / f"{job_id}.child.cmd"
-        child_path.write_text("@echo off\r\n" + command + "\r\n", encoding="utf-8")
-        script_path = _JOBS_DIR / f"{job_id}.cmd"
-        script_path.write_text(
-            "@echo off\r\n"
-            f'call "{child_path}" > "{log_path}" 2>&1\r\n'
-            f'echo %ERRORLEVEL%> "{exit_path}"\r\n',
-            encoding="utf-8",
-        )
-        argv = [os.environ.get("ComSpec", "cmd.exe"), "/c", str(script_path)]
-
-    proc = subprocess.Popen(
-        argv,
-        stdout=subprocess.DEVNULL,
-        stderr=subprocess.DEVNULL,
-        stdin=subprocess.DEVNULL,
-        cwd=cwd or None,
-        **detached_popen_kwargs(),  # detach from the request lifecycle (setsid / DETACHED_PROCESS)
-    )
+    from src import containment
+    from src.agent_tools.subprocess_tools import _owned_spec, _replace_workspace_alias
+    spec = _owned_spec(cwd or os.getcwd(), env, max_runtime_s)
+    grant = containment.acquire(spec, owner=f"bg:{session_id}")
+    bounded_command = command
+    if containment.FILESYSTEM not in grant.enforced:
+        bounded_command = _replace_workspace_alias(command, grant.workspace)
+    result_path = _JOBS_DIR / f"{job_id}.result.json"
+    payload = {
+        "store_path": str(containment._store_path().resolve()),
+        "grant": {**grant.to_dict(), "owner": grant.owner},
+        "spec": {
+            "workspace": spec.workspace, "env": dict(spec.env), "wall_clock_s": spec.wall_clock_s,
+            "required": sorted(spec.required), "network": spec.network,
+            "readonly_extra": list(spec.readonly_extra), "writable_extra": list(spec.writable_extra),
+            "max_output_bytes": spec.max_output_bytes,
+        },
+        "command": bounded_command, "log_path": str(log_path.resolve()),
+        "result_path": str(result_path.resolve()), "exit_path": str(exit_path.resolve()),
+    }
+    try:
+        with open(log_path, "ab") as bootstrap_log:
+            proc = subprocess.Popen(
+                [sys.executable, str(Path(containment.__file__).with_name("containment_worker.py"))],
+                stdin=subprocess.PIPE, stdout=subprocess.DEVNULL, stderr=bootstrap_log,
+                cwd=str(Path(containment.__file__).resolve().parent.parent),
+                **detached_popen_kwargs(),
+            )
+    except BaseException:
+        containment.release(grant, grace_s=0)
+        raise
 
     rec = {
         "id": job_id,
@@ -153,15 +136,31 @@ def launch(command: str, session_id: str, cwd: Optional[str] = None,
         "followed_up": False,       # has the agent been re-invoked with the result?
         "log_path": str(log_path),
         "exit_path": str(exit_path),
+        "result_path": str(result_path),
+        "containment_id": grant.id,
+        "containment": {**grant.to_dict(), "contained": False, "enforced": [], "pending": True, "executed": False},
+        "pgid": None if os.name == "nt" else proc.pid,
         # Identity, not just a slot. The pid above is reused by the kernel, and
         # this record outlives the process and the server; the token is what a
         # later run compares before it signals anything. See
         # src/process_ownership.py.
         "start_token": process_ownership.capture(proc.pid)["start_token"],
     }
-    jobs = _load()
-    jobs[job_id] = rec
-    _save(jobs)
+    try:
+        containment._update_record(grant.id, lifetime="background", supervisor_pid=proc.pid,
+                                   supervisor_token=rec["start_token"])
+        jobs = _load()
+        jobs[job_id] = rec
+        _save(jobs)
+        # The supervisor cannot execute until the identity and job record are durable.
+        proc.stdin.write(json.dumps(payload).encode("utf-8"))
+        proc.stdin.close()
+    except BaseException:
+        kill_process_tree(proc.pid)
+        proc.wait(timeout=5)
+        containment.release(grant, grace_s=0)
+        raise
+    _LIVE_PROCS[proc.pid] = proc
     return rec
 
 
@@ -194,10 +193,14 @@ def _prune(jobs: Dict[str, Dict[str, Any]], now: float) -> bool:
     return bool(stale)
 
 
+@store_transaction(lambda: _STORE)
 def refresh() -> Dict[str, Dict[str, Any]]:
     """Reconcile every running job against disk. Marks done/failed (incl.
     timeout). Idempotent — safe to call from a poll loop. Returns the store."""
     jobs = _load()
+    for pid, proc in list(_LIVE_PROCS.items()):
+        if proc.poll() is not None:
+            _LIVE_PROCS.pop(pid, None)
     changed = False
     now = time.time()
     for rec in jobs.values():
@@ -212,13 +215,24 @@ def refresh() -> Dict[str, Dict[str, Any]]:
             rec["exit_code"] = code
             rec["status"] = "done" if code == 0 else "failed"
             rec["ended_at"] = now
+            if rec.get("result_path"):
+                try:
+                    report = json.loads(Path(rec["result_path"]).read_text(encoding="utf-8"))
+                    rec.update(report)
+                except (OSError, ValueError):
+                    rec["status"], rec["exit_code"] = "failed", 1
+                    rec["result_unavailable"] = True
             changed = True
         elif (now - rec.get("started_at", now)) > rec.get("max_runtime_s", DEFAULT_MAX_RUNTIME_S):
             # Runaway / stuck — reap it but STILL surface a follow-up.
-            _kill(rec.get("pid"))
-            rec["status"] = "failed"
-            rec["exit_code"] = -1
-            rec["ended_at"] = now
+            outcome = _kill_record(rec)
+            rec["teardown"] = outcome.to_dict()
+            if outcome.dead:
+                rec["status"] = "failed"
+                rec["exit_code"] = -1
+                rec["ended_at"] = now
+            else:
+                rec["kill_failed"] = True
             rec["timed_out"] = True
             changed = True
         elif not _pid_alive(rec.get("pid")) and not exit_path.exists():
@@ -236,9 +250,33 @@ def refresh() -> Dict[str, Dict[str, Any]]:
     return jobs
 
 
-def _kill(pid: Optional[int]) -> None:
+def _kill(pid: Optional[int], **kwargs):
     # Cross-platform process-tree teardown (POSIX killpg / Windows taskkill /T).
-    kill_process_tree(pid)
+    return kill_process_tree(pid, **kwargs)
+
+
+def _kill_record(rec):
+    from src import containment
+    verdict = process_ownership.verify(rec.get("pid"), rec.get("start_token"))
+    if verdict in (process_ownership.FOREIGN, process_ownership.UNVERIFIABLE):
+        return containment.ReleaseOutcome(dead=False, escalated=False, ownership=verdict)
+    if rec.get("containment_id"):
+        record = containment._load_records().get(rec["containment_id"])
+        if record and record.get("pid"):
+            outcome = containment.reap_record(record)
+            if not outcome.dead:
+                return outcome
+    outcome = _kill(rec.get("pid"), start_token=rec.get("start_token"),
+                    pgid=rec.get("pgid"), require_identity=True)
+    proc = _LIVE_PROCS.get(rec.get("pid"))
+    if proc and outcome.dead:
+        proc.wait(timeout=5)
+        _LIVE_PROCS.pop(proc.pid, None)
+    if outcome.dead and rec.get("containment_id"):
+        record = containment._load_records().get(rec["containment_id"])
+        if record and not record.get("pid"):
+            containment.reap_record(record)
+    return outcome
 
 
 def pending_followups() -> List[Dict[str, Any]]:
@@ -249,6 +287,7 @@ def pending_followups() -> List[Dict[str, Any]]:
             if r.get("status") in ("done", "failed") and not r.get("followed_up")]
 
 
+@store_transaction(lambda: _STORE)
 def mark_followed_up(job_id: str) -> None:
     jobs = _load()
     if job_id in jobs:
@@ -269,6 +308,7 @@ def list_for_session(session_id: str) -> List[Dict[str, Any]]:
     return [r for r in refresh().values() if r.get("session_id") == session_id]
 
 
+@store_transaction(lambda: _STORE)
 def kill(job_id: str) -> Optional[Dict[str, Any]]:
     """Terminate a running job's process tree and mark it killed. Returns the
     updated record, or None if the id is unknown. Idempotent: a job that already
@@ -279,16 +319,21 @@ def kill(job_id: str) -> Optional[Dict[str, Any]]:
     if rec is None:
         return None
     if rec.get("status") == "running":
-        _kill(rec.get("pid"))
-        rec["status"] = "failed"
-        rec["exit_code"] = -1
-        rec["ended_at"] = time.time()
-        rec["killed"] = True
-        rec["followed_up"] = True
+        outcome = _kill_record(rec)
+        rec["teardown"] = outcome.to_dict()
+        if outcome.dead:
+            rec["status"] = "failed"
+            rec["exit_code"] = -1
+            rec["ended_at"] = time.time()
+            rec["killed"] = True
+            rec["followed_up"] = True
+        else:
+            rec["kill_failed"] = True
         _save(jobs)
     return rec
 
 
+@store_transaction(lambda: _STORE)
 def disown_unverified() -> Dict[str, Any]:
     """Stop tracking running jobs whose process can no longer be proven ours.