.agents/skills/lib_py/herdr_restore.py

@@ -0,0 +1,88 @@
+"""Issue #45 (B): close the restored empty shells of the session being resumed.
+
+With restore-on-restart off (lib_py/herdr_config.py) a restarted MAM server
+still brings every saved pane back - as an empty shell in its old cwd, or in
+$HOME when that directory is gone. A pane keeps the `agent_session` its agent
+reported before the server died. resume_session.sh calls this once, right
+before it spawns, with the id it is about to resume; a pane is closed only when
+all of these hold:
+
+  1. the target is an isolated MAM server (HERDR_SESSION_NAME set, not default);
+  2. herdr sees no live agent in it;
+  3. its agent_session value is that id - so a user's pane, another session's
+     pane and a freshly split pane (no agent_session yet) are never candidates;
+  4. its foreground process is the shell itself (nothing running in it).
+
+The registry is not consulted: a session whose server died stays `running`
+there, so "running" cannot tell a live pane from a restored one.
+
+CLI: `python3 -m lib_py.herdr_restore close-restored-shells --session-id ID
+[--herdr PATH]`. One stderr line per closed pane; exit 1 when herdr cannot be
+queried (the caller only warns).
+"""
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import subprocess
+import sys
+from typing import List, Optional
+
+# A pane exports these; they would point the query at the caller's own server
+# and workspace instead of the --session named here.
+_SCOPE_ENV = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH")
+
+
+def _herdr(herdr: str, session: str, *args: str) -> dict:
+    env = {k: v for k, v in os.environ.items() if k not in _SCOPE_ENV}
+    out = subprocess.run([herdr, "--session", session, *args], env=env,
+                         capture_output=True, text=True, timeout=30, check=True).stdout
+    return json.loads(out).get("result") or {}
+
+
+def _shell_in_foreground(herdr: str, session: str, pane_id: str) -> bool:
+    info = _herdr(herdr, session, "pane", "process-info", "--pane", pane_id).get("process_info") or {}
+    shell = info.get("shell_pid")
+    return shell is not None and info.get("foreground_process_group_id") == shell
+
+
+def close_restored_shells(session_id: str, herdr: str = "herdr",
+                          session: Optional[str] = None) -> List[str]:
+    """Close the matching panes; return their ids."""
+    session = os.environ.get("HERDR_SESSION_NAME", "") if session is None else session
+    if not session_id or session in ("", "default"):
+        return []
+    closed = []
+    for p in _herdr(herdr, session, "pane", "list").get("panes") or []:
+        pane_id = p.get("pane_id")
+        if not pane_id or p.get("agent"):
+            continue
+        if (p.get("agent_session") or {}).get("value") != session_id:
+            continue
+        if not _shell_in_foreground(herdr, session, pane_id):
+            continue
+        _herdr(herdr, session, "pane", "close", pane_id)
+        print(f"closed restored pane {pane_id} (cwd={p.get('cwd', '')}, agent_session={session_id})",
+              file=sys.stderr)
+        closed.append(pane_id)
+    return closed
+
+
+def main(argv=None) -> int:
+    p = argparse.ArgumentParser(description="close a resumed session's restored empty shells (Issue #45)")
+    sub = p.add_subparsers(dest="cmd", required=True)
+    c = sub.add_parser("close-restored-shells")
+    c.add_argument("--session-id", required=True)
+    c.add_argument("--herdr", default="", help="the real herdr binary (not the MAM shim)")
+    args = p.parse_args(argv)
+    try:
+        close_restored_shells(args.session_id, args.herdr or "herdr")
+    except (OSError, subprocess.SubprocessError, ValueError) as e:
+        print(f"close-restored-shells: {e}", file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh

@@ -233,6 +233,14 @@ if [ "$AGENT" = "opencode" ]; then
   fi
 fi
 
+# Issue #45: a restarted server brings this session's old pane back as an empty
+# shell (restore-on-restart is off on MAM servers). The shim cannot reuse it when
+# a CP_PREFIX is passed, nor when it fell back to $HOME, so close it first; the
+# conditions live in lib_py/herdr_restore.py. Never blocks the resume.
+"$(_delegate_py_bin)" -m lib_py.herdr_restore close-restored-shells --session-id "$UUID" \
+  --herdr "$(_resolve_real_herdr_path || true)" \
+  || echo "WARNING: could not close the restored panes of $SESSION_NAME; an empty shell may remain" >&2
+
 # 4. Spawn new agent session (delegates to herdr translation shim)
 _herdr new-session -d -s "$SESSION_NAME" -c "$WORKSPACE" "$CP_PREFIX$CMD_FULL"
 # Issue #24: every agent, the same call create makes. The signatures and accept

tests/test_issue45_restore.py

@@ -0,0 +1,342 @@
+"""Issue #45 against a real herdr server: a restarted MAM server must not relaunch
+saved agents (A), and resume must not leave the session's restored empty shell
+behind (B).
+
+Everything runs on a throwaway herdr session under a short throwaway HOME, so
+the server's socket, session.json and config never touch the user's herdr (the
+running `multi-agent-mux` server included). The agent is a fake `claude`
+script that logs its argv; the user's herdr config sets `[terminal]
+default_shell` to a shell that puts the fake first on PATH, so herdr's own
+restore command (`claude --resume <id>`) reaches the fake too. That is the
+positive control: with restore on, the fake logs `--resume <id>`; through the
+MAM bootstrap it must not. No real claude runs and no API is called.
+
+A crash is `kill -9` of the server after session.json has the pane's
+agent_session (herdr writes it a few seconds after a change).
+"""
+from __future__ import annotations
+
+import json
+import os
+import shutil
+import signal
+import subprocess
+import sys
+import tempfile
+import time
+import uuid
+from pathlib import Path
+
+import pytest
+
+HERDR = shutil.which("herdr")
+pytestmark = pytest.mark.skipif(not HERDR, reason="real herdr is not installed")
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+sys.path.insert(0, str(REPO_ROOT / ".agents" / "skills"))
+from lib_py import herdr_config  # noqa: E402
+
+# Inherited from the pane this suite may run in: they would point herdr at that
+# pane's server and workspace.
+SCOPE_ENV = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH",
+             "HERDR_SESSION", "HERDR_ENV", "HERDR_CONFIG_PATH", "XDG_CONFIG_HOME")
+
+
+class Rig:
+    def __init__(self, sandbox: Path, root: Path, session: str):
+        self.sandbox, self.root, self.session = sandbox, root, session
+        self.bin = root / "bin"
+        self.argv_log = root / "argv.log"
+        self.sdir = root / ".config" / "herdr" / "sessions" / session
+        self.user_cfg = root / ".config" / "herdr" / "config.toml"
+
+    # -- herdr ---------------------------------------------------------------
+    def env(self, **extra) -> dict:
+        env = {k: v for k, v in os.environ.items() if k not in SCOPE_ENV}
+        env["PATH"] = f"{self.bin}:{env['PATH']}"
+        env.update(extra)
+        return env
+
+    def hd(self, *args) -> dict:
+        out = subprocess.run([HERDR, "--session", self.session, *args], env=self.env(),
+                             capture_output=True, text=True, timeout=30)
+        assert out.returncode == 0, f"herdr {args}: {out.stdout}{out.stderr}"
+        return json.loads(out.stdout).get("result") or {} if out.stdout.strip() else {}
+
+    def panes(self) -> list:
+        return self.hd("pane", "list").get("panes") or []
+
+    def server_pids(self) -> list:
+        out = subprocess.run(["pgrep", "-f", "--", f"--session {self.session} server"],
+                             capture_output=True, text=True).stdout
+        return [int(p) for p in out.split()]
+
+    def _wait_socket(self):
+        sock = self.sdir / "herdr.sock"
+        for _ in range(80):
+            if sock.exists():
+                return
+            time.sleep(0.25)
+        raise AssertionError("herdr server socket never appeared")
+
+    def restores(self) -> int:
+        log = self.sdir / "herdr-server.log"
+        return log.read_text().count("session restore evaluated") if log.exists() else 0
+
+    def start_raw(self, config: Path | None = None):
+        """A server as the user would start it: restore on unless `config` says otherwise."""
+        env = self.env(**({"HERDR_CONFIG_PATH": str(config)} if config else {}))
+        subprocess.Popen([HERDR, "--session", self.session, "server"], env=env, start_new_session=True,
+                         stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+        self._wait_socket()
+
+    def run_lib(self, body: str, **extra) -> subprocess.CompletedProcess:
+        lib = self.sandbox / ".agents" / "skills" / "lib.sh"
+        return subprocess.run(["bash", "-c", f'source "{lib}"\n{body}'], cwd=str(self.sandbox),
+                              env=self.env(HERDR_SESSION_NAME=self.session, **extra),
+                              capture_output=True, text=True, timeout=120)
+
+    def start_via_shim(self):
+        """The only way MAM starts a server: the shim's headless bootstrap."""
+        res = self.run_lib("herdr has-session -t nobody-here || true")
+        assert res.returncode == 0, res.stderr
+        assert "restore-on-restart stays enabled" not in res.stderr, res.stderr
+        try:
+            self._wait_socket()
+        except AssertionError:
+            raise AssertionError(f"the shim did not start the server: {res.stderr}")
+
+    def wait_restored(self, before: int):
+        for _ in range(60):
+            if self.restores() > before:
+                time.sleep(3)  # let herdr spawn whatever it is going to spawn
+                return
+            time.sleep(0.25)
+        raise AssertionError("the server never evaluated its saved session")
+
+    # -- panes ---------------------------------------------------------------
+    def wait_shell(self, pane_id: str):
+        for _ in range(40):
+            info = self.hd("pane", "process-info", "--pane", pane_id)["process_info"]
+            if info.get("shell_pid"):
+                return
+            time.sleep(0.25)
+
+    def new_pane(self, cwd: Path, label: str) -> str:
+        cwd.mkdir(parents=True, exist_ok=True)
+        pane = self.hd("workspace", "create", "--cwd", str(cwd), "--label", label, "--no-focus")["root_pane"]["pane_id"]
+        self.wait_shell(pane)
+        return pane
+
+    def run_agent(self, pane_id: str, session_id: str):
+        """The fake claude, plus the session report herdr's claude integration would send."""
+        self.hd("pane", "run", pane_id, f"claude --session-id {session_id}")
+        self.hd("pane", "report-agent", "--source", "herdr:claude", "--agent", "claude",
+                "--state", "idle", "--agent-session-id", session_id, pane_id)
+
+    def crash(self, *session_ids: str):
+        """kill -9 the server once session.json holds every id."""
+        saved = self.sdir / "session.json"
+        for _ in range(80):
+            text = saved.read_text() if saved.exists() else ""
+            if all(s in text for s in session_ids):
+                break
+            time.sleep(0.25)
+        else:
+            raise AssertionError(f"session.json never recorded {session_ids}")
+        for pid in self.server_pids():
+            os.kill(pid, signal.SIGKILL)
+        self.kill_fakes()
+        time.sleep(0.5)
+        (self.sdir / "herdr.sock").unlink(missing_ok=True)
+
+    def kill_fakes(self):
+        subprocess.run(["pkill", "-9", "-f", f"{self.bin}/claude"], capture_output=True)
+
+    def resumed(self, session_id: str) -> bool:
+        return self.argv_log.exists() and f"--resume {session_id}" in self.argv_log.read_text()
+
+
+@pytest.fixture
+def rig(mam_sandbox, monkeypatch):
+    # A pytest tmp_path is too long for a unix socket path under it.
+    root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp"))
+    session = f"mamt45-{os.getpid()}-{uuid.uuid4().hex[:4]}"
+    for k in SCOPE_ENV:
+        monkeypatch.delenv(k, raising=False)
+    monkeypatch.setenv("HOME", str(root))
+    monkeypatch.setenv("HOME_DIR", str(root))
+    r = Rig(mam_sandbox, root, session)
+    r.bin.mkdir()
+    fake = r.bin / "claude"
+    fake.write_text(f'#!/usr/bin/env bash\n'
+                    'if [ "${1:-}" = auth ]; then echo \'{"loggedIn": true}\'; exit 0; fi\n'
+                    f'echo "$*" >> "{r.argv_log}"\n'
+                    'echo "Welcome to Claude Code (fake)"\nwhile :; do sleep 1; done\n')
+    fake.chmod(0o755)
+    shell = root / "shell.sh"
+    shell.write_text(f'#!/usr/bin/env bash\nexport PATH="{r.bin}:$PATH"\nexec bash --norc -i\n')
+    shell.chmod(0o755)
+    r.user_cfg.parent.mkdir(parents=True)
+    r.user_cfg.write_text(f'[terminal]\ndefault_shell = "{shell}"\n')
+    try:
+        yield r
+    finally:
+        subprocess.run([HERDR, "session", "stop", session], env=r.env(), capture_output=True, timeout=30)
+        for pid in r.server_pids():
+            try:
+                os.kill(pid, signal.SIGKILL)
+            except ProcessLookupError:
+                pass
+        r.kill_fakes()
+        subprocess.run(["pkill", "-9", "-f", str(root)], capture_output=True)
+        subprocess.run([HERDR, "session", "delete", session], env=r.env(), capture_output=True, timeout=30)
+        shutil.rmtree(root, ignore_errors=True)
+
+
+def _pane_with(rig: Rig, session_id: str) -> dict | None:
+    return next((p for p in rig.panes() if (p.get("agent_session") or {}).get("value") == session_id), None)
+
+
+# --- A ---------------------------------------------------------------------
+
+@pytest.mark.parametrize("user_session_table", [
+    "",
+    # an existing [session] table: appending a second one would break the TOML
+    # and herdr would silently restore again
+    "[session]\nresume_agents_on_restore = true\n",
+], ids=["plain-user-config", "user-config-with-session-table"])
+def test_a_mam_server_does_not_relaunch_saved_agents(rig, user_session_table):
+    rig.user_cfg.write_text(rig.user_cfg.read_text() + user_session_table)
+    sid = str(uuid.uuid4())
+
+    rig.start_raw()
+    rig.run_agent(rig.new_pane(rig.root / "wsA", "A"), sid)
+    rig.crash(sid)
+
+    # MAM brings the server back: nothing may be relaunched.
+    before = rig.restores()
+    rig.start_via_shim()
+    rig.wait_restored(before)
+    assert not rig.resumed(sid), rig.argv_log.read_text()
+    pane = _pane_with(rig, sid)
+    assert pane is not None and not pane.get("agent"), rig.panes()
+    info = rig.hd("pane", "process-info", "--pane", pane["pane_id"])["process_info"]
+    assert info["foreground_process_group_id"] == info["shell_pid"], info
+    # and it got there through HERDR_CONFIG_PATH
+    environ = Path(f"/proc/{rig.server_pids()[0]}/environ").read_bytes().split(b"\0")
+    assert f"HERDR_CONFIG_PATH={rig.user_cfg.parent / herdr_config.OUTPUT_NAME}".encode() in environ
+
+    # Positive control: the same saved layout under a plain server relaunches
+    # the agent, so the "not resumed" above is not vacuous.
+    rig.crash(sid)
+    before = rig.restores()
+    rig.start_raw()
+    rig.wait_restored(before)
+    assert rig.resumed(sid), rig.argv_log.read_text() if rig.argv_log.exists() else "no argv log"
+
+
+# --- B ---------------------------------------------------------------------
+
+def _close(rig: Rig, session_id: str) -> subprocess.CompletedProcess:
+    env = rig.env(HERDR_SESSION_NAME=rig.session, PYTHONPATH=str(rig.sandbox / ".agents" / "skills"))
+    return subprocess.run([sys.executable, "-m", "lib_py.herdr_restore", "close-restored-shells",
+                           "--session-id", session_id, "--herdr", HERDR],
+                          env=env, capture_output=True, text=True, timeout=60)
+
+
+def test_b_only_the_resumed_sessions_restored_shell_is_closed(rig):
+    ids = {k: str(uuid.uuid4()) for k in ("X", "Y", "Z", "X2")}
+    rig.start_raw()
+    for k in ids:
+        rig.run_agent(rig.new_pane(rig.root / f"ws{k}", k), ids[k])
+    rig.new_pane(rig.root / "wsU", "U")  # a user's plain shell: no agent_session
+    rig.crash(*ids.values())
+    shutil.rmtree(rig.root / "wsY")  # a removed worktree: restored into $HOME
+
+    before = rig.restores()
+    rig.start_via_shim()
+    rig.wait_restored(before)
+    rig.run_agent(_pane_with(rig, ids["Z"])["pane_id"], ids["Z"])  # running again
+    rig.hd("pane", "run", _pane_with(rig, ids["X2"])["pane_id"], "sleep 60")  # busy shell
+    time.sleep(1)
+    assert _pane_with(rig, ids["Y"])["cwd"] == str(rig.root)
+    n = len(rig.panes())
+
+    for k in ("Z", "X2"):
+        res = _close(rig, ids[k])
+        assert res.returncode == 0 and "closed" not in res.stderr, res.stderr
+    res = _close(rig, "")
+    assert res.returncode == 0 and "closed" not in res.stderr, res.stderr
+    assert len(rig.panes()) == n
+
+    x = _pane_with(rig, ids["X"])["pane_id"]
+    res = _close(rig, ids["X"])
+    assert res.returncode == 0, res.stderr
+    assert f"closed restored pane {x}" in res.stderr
+    assert _pane_with(rig, ids["X"]) is None and len(rig.panes()) == n - 1
+
+    res = _close(rig, ids["Y"])
+    assert res.returncode == 0 and "closed restored pane" in res.stderr, res.stderr
+    assert _pane_with(rig, ids["Y"]) is None and len(rig.panes()) == n - 2
+    # the user's shell, the running agent and the busy shell are all still there
+    assert any(p.get("cwd") == str(rig.root / "wsU") for p in rig.panes())
+    assert _pane_with(rig, ids["Z"]) and _pane_with(rig, ids["X2"])
+
+
+def test_b_default_server_is_never_touched(rig):
+    env = rig.env(HERDR_SESSION_NAME="default", PYTHONPATH=str(rig.sandbox / ".agents" / "skills"))
+    # a herdr that would fail loudly if called at all
+    res = subprocess.run([sys.executable, "-m", "lib_py.herdr_restore", "close-restored-shells",
+                          "--session-id", "some-id", "--herdr", "/nonexistent/herdr"],
+                         env=env, capture_output=True, text=True, timeout=30)
+    assert res.returncode == 0 and res.stderr == "", res.stderr
+
+
+def _script(rig: Rig, rel: str, *args: str, timeout: int = 180) -> subprocess.CompletedProcess:
+    """Output goes through files: a server the script bootstraps outlives it."""
+    path = rig.sandbox / ".agents" / "skills" / rel
+    out, err = rig.root / "script.out", rig.root / "script.err"
+    with open(out, "w") as o, open(err, "w") as e:
+        try:
+            p = subprocess.run(["bash", str(path), *args], cwd=str(rig.sandbox), env=rig.env(),
+                               stdout=o, stderr=e, text=True, timeout=timeout)
+        except subprocess.TimeoutExpired:
+            raise AssertionError(f"{rel} timed out:\n{out.read_text()}\n{err.read_text()}")
+    return subprocess.CompletedProcess(p.args, p.returncode, out.read_text(), err.read_text())
+
+
+def test_b_resume_leaves_no_restored_shell_behind(rig):
+    """The real create -> crash -> resume path. The row create writes stays
+    `running` through the crash, as it does in production; resume runs from a
+    worktree whose control plane is the sandbox, so it spawns with a CP_PREFIX
+    and the shim cannot reuse the restored shell (it splits a new pane)."""
+    ws = rig.root / "wsX"
+    ws.mkdir()
+    name = "creator-claude-m45"
+    res = _script(rig, "multi-agent-mux-create/scripts/create_session.sh", "--workspace", str(ws),
+                  "--agent", "claude", "--role", "creator", "--session", name,
+                  "--herdr-session", rig.session, "--no-onboard", timeout=60)
+    assert res.returncode == 0, res.stdout + res.stderr
+    rows = json.loads(rig.run_lib("load_state_json").stdout)["herdr_sessions"]
+    row = next(r for r in rows if r["name"] == name)
+    sid = row["claude_session_id_own"]
+    assert row["status"] == "running" and sid
+    pane = next(p for p in rig.panes() if p.get("cwd") == str(ws))
+    # what herdr's claude integration reports for a real claude (checked on a live server)
+    rig.hd("pane", "report-agent", "--source", "herdr:claude", "--agent", "claude",
+           "--state", "idle", "--agent-session-id", sid, pane["pane_id"])
+    rig.crash(sid)
+
+    res = _script(rig, "multi-agent-mux-resume/scripts/resume_session.sh", "--workspace", str(ws),
+                  "--agent", "claude", "--session", name)
+    assert res.returncode == 0, res.stdout + res.stderr
+    assert not rig.resumed(sid)  # herdr did not relaunch it ...
+    # ... MAM did, with its own command: create's launch plus resume's, both flagged
+    launches = rig.argv_log.read_text().splitlines()
+    assert len(launches) == 2 and all("--dangerously-skip-permissions" in l and sid in l for l in launches), launches
+    in_ws = [p for p in rig.panes() if p.get("cwd") == str(ws)]
+    assert len(in_ws) == 1, in_ws
+    info = rig.hd("pane", "process-info", "--pane", in_ws[0]["pane_id"])["process_info"]
+    assert info["foreground_process_group_id"] != info["shell_pid"], info  # the agent, not an empty shell
+    assert "closed restored pane" in res.stderr, res.stderr