diff --git a/.agents/skills/lib_py/agents/adapters/agy.py b/.agents/skills/lib_py/agents/adapters/agy.py index febdef9..8ea8aa5 100644 --- a/.agents/skills/lib_py/agents/adapters/agy.py +++ b/.agents/skills/lib_py/agents/adapters/agy.py @@ -197,11 +197,14 @@ class AgyAgentAdapter(BaseAgentAdapter): # the DB already gone, and those orphans must still be reachable. return any(os.path.exists(p) for p in self._artifact_paths(uuid, ctx)) + def in_use_paths(self, uuid: str, ctx: DiscoveryContext) -> list: + return self._artifact_paths(uuid, ctx) + def conversation_in_use(self, uuid: str, ctx: DiscoveryContext) -> bool: # The holder may keep the presence lock (or a sidecar) open rather # than the DB, and may name the uuid nowhere; the lsof leg of the # check must see every path of the conversation. - return conversation_in_use(uuid, self._artifact_paths(uuid, ctx)) + return conversation_in_use(uuid, self.in_use_paths(uuid, ctx)) def purge_artifacts(self, uuid: str, ctx: DiscoveryContext) -> list: # Issue #10: a purge must leave nothing resumable behind. Besides the diff --git a/.agents/skills/lib_py/agents/adapters/opencode.py b/.agents/skills/lib_py/agents/adapters/opencode.py index 3ea89fb..be3f98f 100644 --- a/.agents/skills/lib_py/agents/adapters/opencode.py +++ b/.agents/skills/lib_py/agents/adapters/opencode.py @@ -13,6 +13,11 @@ _LEGACY_TABLES = (("message", "session_id"), ("messages", "session_id"), ("part" # (binary, HOME, XDG_DATA_HOME) -> the db `debug paths` printed for them. _DB_PATHS = {} + +def _is_shared_service(cmd: str) -> bool: + """The per-user opencode background service (`serve --service`).""" + return "serve --service" in (cmd or "") + class OpenCodeAgentAdapter(BaseAgentAdapter): @property def name(self) -> str: @@ -250,6 +255,13 @@ class OpenCodeAgentAdapter(BaseAgentAdapter): found += rows if i == uuid else [f"{r} [child session {i}]" for r in rows] return found + def in_use_paths(self, uuid: str, ctx: DiscoveryContext) -> list: + db = self._resolve_db(ctx) + return [db] if db else [] + + def _holder_counts(self, cmd: str) -> bool: + return not _is_shared_service(cmd) + def conversation_in_use(self, uuid: str, ctx: DiscoveryContext) -> bool: """A process naming the session, or an opencode process holding the shared database. The per-user background service (`serve --service`) @@ -270,13 +282,14 @@ class OpenCodeAgentAdapter(BaseAgentAdapter): timeout=10).stdout except Exception: return True - if cmd.strip() and "serve --service" not in cmd: + if cmd.strip() and not _is_shared_service(cmd): return True return False def in_use_hints(self, uuid: str, ctx: DiscoveryContext) -> list: + db = self._resolve_db(ctx) return super().in_use_hints(uuid, ctx) + [ - f"another opencode session holding {self._resolve_db(ctx)}: lsof {self._resolve_db(ctx)}"] + f"another opencode session holding {db}: lsof {db}"] # opencode 2.x: the default agent is build (`--agent` is gone from the # TUI), and --standalone keeps the server a child of the pane's TUI diff --git a/.agents/skills/lib_py/agents/base.py b/.agents/skills/lib_py/agents/base.py index 35423e6..7438095 100644 --- a/.agents/skills/lib_py/agents/base.py +++ b/.agents/skills/lib_py/agents/base.py @@ -1,7 +1,7 @@ # base.py — BaseAgentAdapter abstract base class, SpawnSpec, and DiscoveryContext import os, json, sqlite3, shutil, subprocess, tempfile -from typing import Optional, Dict, Any, List +from typing import Optional, Dict, Any, List, Tuple from urllib.parse import quote from lib_py.paths import resolve_home from lib_py.verify_session import verify_session_uuid, workspace_key @@ -86,6 +86,34 @@ def conversation_in_use(uuid: str, paths=()) -> bool: return False +def file_holders(paths=()) -> List[Tuple[int, str]]: + """(pid, command_line) for each process holding one of `paths` open via lsof (#78). + Informational only: returns [] when lsof is absent or fails.""" + existing = [p for p in paths if p and os.path.exists(p)] + if not existing or not shutil.which("lsof"): + return [] + try: + r = subprocess.run(["lsof", "-t"] + existing, capture_output=True, text=True, timeout=10) + except Exception: + return [] + pids: List[int] = [] + for tok in (r.stdout or "").split(): + if tok.isdigit(): + pid = int(tok) + if pid not in pids: + pids.append(pid) + out: List[Tuple[int, str]] = [] + for pid in pids: + try: + cmd = subprocess.run(["ps", "-o", "command=", "-p", str(pid)], + capture_output=True, text=True, timeout=10).stdout.strip() + except Exception: + continue + if cmd: + out.append((pid, cmd)) + return out + + def _plausible_id(uuid: str) -> bool: """An id safe to build a path from and to search for: not empty, not made of dots alone ("." names the parent dir itself), no "/", ".." or "*".""" @@ -354,14 +382,25 @@ class BaseAgentAdapter: path = self.artifact_path(uuid, ctx) return uuid in path and os.path.exists(path) + def in_use_paths(self, uuid: str, ctx: DiscoveryContext) -> list: + """On-disk paths checked with lsof for conversation_in_use and in_use_hints (#78).""" + return [self.artifact_path(uuid, ctx)] + + def _holder_counts(self, cmd: str) -> bool: + return True + def conversation_in_use(self, uuid: str, ctx: DiscoveryContext) -> bool: """True while a live process still references the conversation.""" - return conversation_in_use(uuid, [self.artifact_path(uuid, ctx)]) + return conversation_in_use(uuid, self.in_use_paths(uuid, ctx)) def in_use_hints(self, uuid: str, ctx: DiscoveryContext) -> list: """Where to look for what holds the conversation, shown when a purge is refused because it is in use.""" - return [f"any process with it on its command line (an editor, a grep, ...): ps -axww | grep {uuid}"] + hints = [f"any process with it on its command line (an editor, a grep, ...): ps -axww | grep {uuid}"] + for pid, cmd in file_holders(self.in_use_paths(uuid, ctx)): + if self._holder_counts(cmd): + hints.append(f"held open by pid {pid}: {cmd} (not stopped by MAM; end it yourself if it is safe)") + return hints def spawn_spec(self, binary: str, session_uuid: str = "", use_wrapper: bool = False) -> str: raise NotImplementedError diff --git a/tests/test_issue78_hints_codex.py b/tests/test_issue78_hints_codex.py new file mode 100644 index 0000000..a19d12f --- /dev/null +++ b/tests/test_issue78_hints_codex.py @@ -0,0 +1,251 @@ +"""Issue #78: in_use_hints lists lsof holders, and codex purge removes +tui-thread-reference-capabilities/. +""" +from __future__ import annotations + +import json +import os +import sqlite3 +import subprocess +import sys +import uuid as uuidlib +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS = REPO_ROOT / ".agents" / "skills" +if str(SKILLS) not in sys.path: + sys.path.insert(0, str(SKILLS)) + +from lib_py.agents.base import DiscoveryContext, file_holders # noqa: E402 +from lib_py.agents.registry import get_adapter # noqa: E402 + +pytestmark = pytest.mark.usefixtures("own_workspace_root") + + +def _clean_env(extra: dict | None = None) -> dict: + env = { + k: v + for k, v in os.environ.items() + if not k.startswith("XDG_") + and not k.startswith("HERDR_") + and k + not in ( + "CLAUDE_CODE_SESSION_ID", + "ANTIGRAVITY_CONVERSATION_ID", + "HERMES_SESSION_ID", + "GROK_SESSION_ID", + "OPENCODE_SESSION_ID", + "CLAUDE_CONFIG_DIR", + "CLAUDE_PROJECT_DIR", + "CODEX_HOME", + ) + } + env["PYTHONPATH"] = str(SKILLS) + if extra: + env.update(extra) + return env + + +def _write_fake_lsof(bindir: Path, mapping: dict[str, list[int]]) -> Path: + """Fake lsof that prints mapped PIDs when any argument matches a key substring.""" + bindir.mkdir(parents=True, exist_ok=True) + cases = [] + for needle, pids in mapping.items(): + echoes = "\n".join(f" echo {p}" for p in pids) + cases.append(f" *{needle}*)\n{echoes}\n found=1 ;;") + body = "\n".join(cases) + exe = bindir / "lsof" + exe.write_text( + "#!/bin/sh\n" + "found=0\n" + 'for a in "$@"; do\n' + ' case "$a" in\n' + f"{body}\n" + " esac\n" + "done\n" + '[ "$found" = "1" ] && exit 0\n' + "exit 1\n", + encoding="utf-8", + ) + exe.chmod(0o755) + return exe + + +def _materialize_artifact(agent: str, conv: str, ctx: DiscoveryContext) -> str: + adapter = get_adapter(agent) + if agent == "agy": + # Hold the presence lock so agy's in_use_paths (_artifact_paths) is exercised too. + paths = adapter._artifact_paths(conv, ctx) + target = paths[-1] + else: + target = adapter.artifact_path(conv, ctx) + Path(target).parent.mkdir(parents=True, exist_ok=True) + Path(target).write_text("{}\n", encoding="utf-8") + return target + + +@pytest.mark.parametrize("agent", ["claude", "agy", "codex", "grok", "hermes"]) +def test_in_use_hints_list_lsof_holders(tmp_path, monkeypatch, agent): + home = tmp_path / "home" + ws = tmp_path / "ws" + bindir = tmp_path / "bin" + no_lsof_bin = tmp_path / "no-lsof-bin" + for d in (home, ws, bindir, no_lsof_bin): + d.mkdir(parents=True, exist_ok=True) + + # Populate no_lsof_bin with symlinks to basic tools (ps) but not lsof. + for tool in ("ps", "sh", "bash"): + for sysdir in ("/bin", "/usr/bin"): + cand = Path(sysdir) / tool + if cand.exists() and not (no_lsof_bin / tool).exists(): + (no_lsof_bin / tool).symlink_to(cand) + + conv = str(uuidlib.uuid4()) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("HOME_DIR", str(home)) + monkeypatch.setenv("CODEX_HOME", str(home / ".codex")) + ctx = DiscoveryContext(workspace=str(ws), agent_name=agent, home_dir=str(home)) + target = _materialize_artifact(agent, conv, ctx) + adapter = get_adapter(agent) + + holder = subprocess.Popen(["sleep", "300"]) + try: + _write_fake_lsof(bindir, {conv: [holder.pid]}) + env = _clean_env({ + "HOME": str(home), + "HOME_DIR": str(home), + "CODEX_HOME": str(home / ".codex"), + "PATH": f"{bindir}:{os.environ.get('PATH', '')}", + }) + res = subprocess.run( + [sys.executable, "-m", "lib_py.agents", "in-use", agent, conv, str(ws)], + capture_output=True, + text=True, + env=env, + timeout=30, + ) + assert res.returncode == 4, res.stdout + res.stderr + assert json.loads(res.stdout) == {"agent": agent, "in_use": True} + assert f"ps -axww | grep {conv}" in res.stderr + assert f"held open by pid {holder.pid}:" in res.stderr + assert "sleep 300" in res.stderr + assert "not stopped by MAM" in res.stderr + + monkeypatch.setenv("PATH", env["PATH"]) + hints = adapter.in_use_hints(conv, ctx) + if agent == "claude": + assert hints[0].startswith("a claude process whose session file under ") + assert f"ps -axww | grep {conv}" in hints[1] + else: + assert f"ps -axww | grep {conv}" in hints[0] + assert any(f"held open by pid {holder.pid}:" in h and "sleep 300" in h for h in hints) + assert target in adapter.in_use_paths(conv, ctx) + + # Control: without lsof on PATH, file_holders returns [] and hints match the pre-#78 lines. + monkeypatch.setenv("PATH", str(no_lsof_bin)) + assert file_holders([target]) == [] + hints_no_lsof = adapter.in_use_hints(conv, ctx) + assert not any("held open by pid" in h for h in hints_no_lsof) + assert any(f"ps -axww | grep {conv}" in h for h in hints_no_lsof) + finally: + holder.kill() + holder.wait() + + +def test_in_use_hints_opencode_skip_service(tmp_path, monkeypatch): + home = tmp_path / "home" + ws = tmp_path / "ws" + bindir = tmp_path / "bin" + for d in (home, ws, bindir): + d.mkdir(parents=True, exist_ok=True) + + db = home / ".local" / "share" / "opencode" / "opencode.db" + db.parent.mkdir(parents=True, exist_ok=True) + db.write_text("", encoding="utf-8") + + adapter = get_adapter("opencode") + monkeypatch.setattr(adapter, "_bin", lambda ctx: None) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("HOME_DIR", str(home)) + ctx = DiscoveryContext(workspace=str(ws), agent_name="opencode", home_dir=str(home)) + + service_proc = subprocess.Popen( + [sys.executable, "-c", "import time; time.sleep(300)", "serve", "--service"] + ) + holder_proc = subprocess.Popen(["sleep", "300"]) + try: + _write_fake_lsof(bindir, {"opencode.db": [service_proc.pid, holder_proc.pid]}) + monkeypatch.setenv("PATH", f"{bindir}:{os.environ.get('PATH', '')}") + + hints = adapter.in_use_hints("ses_test78", ctx) + assert hints[0] == "any process with it on its command line (an editor, a grep, ...): ps -axww | grep ses_test78" + assert any(f"held open by pid {holder_proc.pid}:" in h and "sleep 300" in h for h in hints) + assert not any(f"pid {service_proc.pid}" in h or "serve --service" in h for h in hints) + assert hints[-1] == f"another opencode session holding {db}: lsof {db}" + finally: + for p in (service_proc, holder_proc): + p.kill() + p.wait() + + +def test_stop_purge_refusal_prints_holders(mam_sandbox, mock_herdr, monkeypatch): + conv = str(uuidlib.uuid4()) + name = "issue78-reviewer-claude" + ws = mam_sandbox + ctx = DiscoveryContext(workspace=str(ws), agent_name="claude", home_dir=str(mam_sandbox)) + transcript = Path(get_adapter("claude").artifact_path(conv, ctx)) + transcript.parent.mkdir(parents=True, exist_ok=True) + transcript.write_text(json.dumps({"sessionId": conv}) + "\n", encoding="utf-8") + + lib = mam_sandbox / ".agents" / "skills" / "lib.sh" + yaml_path = mam_sandbox / ".mam" / "agent-sessions.yaml" + mutation = ( + f"d['herdr_sessions'] = [{{'name': {name!r}, 'status': 'running', 'agent': 'claude', " + f"'role': 'reviewer', 'claude_session_id_own': {conv!r}, " + f"'pane': {{'cwd': {os.path.realpath(str(ws))!r}}}}}]\n" + ) + res_init = subprocess.run( + ["bash", "-c", f'source "{lib}" && atomic_dump_yaml "{yaml_path}"'], + input=mutation, + capture_output=True, + text=True, + env=_clean_env({ + "HOME": str(mam_sandbox), + "HOME_DIR": str(mam_sandbox), + "WORKSPACE_ROOT": str(mam_sandbox), + "AGENT_SESSIONS_YAML": str(yaml_path), + }), + ) + assert res_init.returncode == 0, res_init.stdout + res_init.stderr + + holder = subprocess.Popen(["sleep", "300"]) + try: + bindir = mam_sandbox / ".local" / "bin" + _write_fake_lsof(bindir, {conv: [holder.pid]}) + stop_sh = mam_sandbox / ".agents" / "skills" / "multi-agent-mux-stop" / "scripts" / "stop_session.sh" + env = _clean_env({ + "HOME": str(mam_sandbox), + "HOME_DIR": str(mam_sandbox), + "CLAUDE_PROJECT_DIR": str(mam_sandbox / ".claude" / "projects"), + "WORKSPACE_ROOT": str(mam_sandbox), + "AGENT_SESSIONS_YAML": str(yaml_path), + "PATH": f"{bindir}:{os.environ.get('PATH', '')}", + }) + res = subprocess.run( + ["bash", str(stop_sh), "--session", name, "--agent", "claude", "--purge-conversation", "--yes"], + capture_output=True, + text=True, + cwd=str(mam_sandbox), + env=env, + timeout=60, + ) + assert res.returncode == 4, res.stdout + res.stderr + assert f"held open by pid {holder.pid}:" in res.stderr + assert "sleep 300" in res.stderr + assert transcript.exists() + assert name in yaml_path.read_text(encoding="utf-8") + finally: + holder.kill() + holder.wait()