GodopuandClaude Opus 5
7610258950
Review of0282312(planner-reviewer-claude-01, N1): conversation_in_use excluded the purge's whole ancestor lineage, and that lineage includes the agent CLI that launched the skill. An orchestrator purging its own UUID from its own session was therefore never refused - the only process naming the UUID was excluded, and claude keeps no transcript open for lsof - so its live conversation was deleted, contrary to what orc-onboard SKILL.md promises. - base.conversation_in_use: in use when one of the caller's session-id variables (CLAUDE_CODE_SESSION_ID, ANTIGRAVITY_CONVERSATION_ID, HERMES_SESSION_ID, GROK_SESSION_ID, OPENCODE_SESSION_ID) equals the uuid; the command-line check now excludes only the purge's own helper chain and stops at the nearest agent CLI ancestor (basename claude/agy/hermes/grok/ opencode/codex, read like detect_nearest_agent), which counts as a holder when it names the uuid; the lsof check is unchanged - tests: the non-agent helper chain is excluded but an agent ancestor is counted; the purge CLI is refused under an agent CLI naming the uuid and with each of the five session variables set to it; orc_onboard --remove --purge-conversation --yes is refused from the orchestrator's own session (exit 5, registration kept). The eight new cases fail on0282312- job_subscriber --registry-dir now defaults to default_registry_dir() like the other entry points (review R-d) - orc-onboard SKILL.md spells out how the calling session is recognised; VERSIONS.md v4.4.3 item 2 and the new-test count (24) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
355 lines
15 KiB
Python
355 lines
15 KiB
Python
#!/usr/bin/env python3
|
|
"""Issues #10 and #13 — purging a conversation must leave nothing resumable
|
|
behind, and an orchestrator conversation (no herdr_sessions row) must be
|
|
purgeable through orc_onboard.sh.
|
|
|
|
#10: the agy adapter deleted only the conversation DB and the brain dir; the
|
|
SQLite sidecars (-wal/-shm/-journal), the title annotation and the presence
|
|
lock survived. The lock is removed only once nothing references the
|
|
conversation any more.
|
|
#13: `orc_onboard.sh --remove <uuid> --purge-conversation --yes` resolves the
|
|
adapter from what is on disk (orchestrator_uuids stores bare UUIDs), purges
|
|
through it, and refuses while a live process still names the UUID -
|
|
including the orchestrator's own session running the command.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import sqlite3
|
|
import subprocess
|
|
import sys
|
|
import time
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
SKILLS = REPO_ROOT / ".agents" / "skills"
|
|
ONBOARD_SH = SKILLS / "multi-agent-mux-orc-onboard" / "scripts" / "orc_onboard.sh"
|
|
|
|
sys.path.insert(0, str(SKILLS))
|
|
from lib_py.agents.registry import get_adapter # noqa: E402
|
|
from lib_py.agents.base import DiscoveryContext, conversation_in_use # noqa: E402
|
|
|
|
UUID = "7d0b6f12-3a4c-4e5f-8a9b-0c1d2e3f4a5b"
|
|
OTHER = "11111111-2222-4333-8444-555555555555"
|
|
|
|
# The session-id variables an agent CLI exports to its tool shells. Scrubbed
|
|
# from every subprocess env below so the caller's own session never leaks in.
|
|
SESSION_ENV = ("CLAUDE_CODE_SESSION_ID", "ANTIGRAVITY_CONVERSATION_ID",
|
|
"HERMES_SESSION_ID", "GROK_SESSION_ID", "OPENCODE_SESSION_ID")
|
|
|
|
|
|
def _clean_env(extra=None) -> dict:
|
|
env = dict(os.environ)
|
|
for k in SESSION_ENV + ("MAM_ORCHESTRATOR_UUIDS",):
|
|
env.pop(k, None)
|
|
env["PYTHONPATH"] = f"{SKILLS}:{env.get('PYTHONPATH', '')}"
|
|
if extra:
|
|
env.update(extra)
|
|
return env
|
|
|
|
|
|
def _agy_tree(home: Path, uuid: str) -> dict:
|
|
root = home / ".gemini" / "antigravity-cli"
|
|
paths = {
|
|
"db": root / "conversations" / f"{uuid}.db",
|
|
"wal": root / "conversations" / f"{uuid}.db-wal",
|
|
"shm": root / "conversations" / f"{uuid}.db-shm",
|
|
"brain": root / "brain" / uuid,
|
|
"annotation": root / "annotations" / f"{uuid}.pbtxt",
|
|
"lock": root / "presence" / f"{uuid}.lock",
|
|
}
|
|
for key, p in paths.items():
|
|
if key == "brain":
|
|
p.mkdir(parents=True, exist_ok=True)
|
|
(p / "note.txt").write_text("brain")
|
|
elif key == "db":
|
|
# A real conversation DB: the adapter's verify_artifact opens it
|
|
# with sqlite3, and sqlite discards a garbage -wal next to a
|
|
# garbage main file, which would hide the sidecar from the purge.
|
|
p.parent.mkdir(parents=True, exist_ok=True)
|
|
conn = sqlite3.connect(p)
|
|
conn.execute("CREATE TABLE steps (id INTEGER)")
|
|
conn.execute("INSERT INTO steps VALUES (1)")
|
|
conn.commit()
|
|
conn.close()
|
|
else:
|
|
p.parent.mkdir(parents=True, exist_ok=True)
|
|
p.write_text("x" if key != "lock" else "")
|
|
return paths
|
|
|
|
|
|
def _holder(uuid: str) -> subprocess.Popen:
|
|
"""A live process whose command line names the conversation, the way a
|
|
resumed `agy --conversation <uuid>` or `claude -r <uuid>` does. (Not
|
|
`sh -c 'sleep …'`: a shell execs its last simple command and the uuid
|
|
would vanish from the process list.)"""
|
|
proc = subprocess.Popen([sys.executable, "-c", "import sys, time; time.sleep(60)", "--conversation", uuid])
|
|
time.sleep(0.3)
|
|
return proc
|
|
|
|
|
|
def _stop(proc: subprocess.Popen) -> None:
|
|
proc.kill()
|
|
proc.wait(timeout=10)
|
|
|
|
|
|
def _fake_agent_cli(tmp_path: Path, name: str) -> Path:
|
|
"""An executable named like an agent CLI. `<name> <flag> <uuid> -- cmd…`
|
|
runs cmd as its child (not exec: this process stays in the tree with the
|
|
uuid on its command line) - the agent session a skill runs under."""
|
|
bindir = tmp_path / "agent-bin"
|
|
bindir.mkdir(exist_ok=True)
|
|
cli = bindir / name
|
|
cli.write_text('#!/bin/sh\nshift 3\n"$@"\nexit $?\n')
|
|
cli.chmod(0o755)
|
|
return cli
|
|
|
|
|
|
def _run_onboard(args, sandbox: Path, extra_env=None) -> subprocess.CompletedProcess:
|
|
env = _clean_env()
|
|
env.update({
|
|
"AGENT_SESSIONS_YAML": str(sandbox / ".mam" / "agent-sessions.yaml"),
|
|
"YAML_PATH": str(sandbox / ".mam" / "agent-sessions.yaml"),
|
|
"HOME_DIR": str(sandbox),
|
|
"AGENT_PYTHON_BIN": sys.executable,
|
|
"DELEGATE_JOB_PYTHON": sys.executable,
|
|
})
|
|
if extra_env:
|
|
env.update(extra_env)
|
|
return subprocess.run(["bash", str(ONBOARD_SH)] + list(args), capture_output=True, text=True,
|
|
env=env, cwd=str(sandbox), timeout=120)
|
|
|
|
|
|
def _registered(sandbox: Path, uuid: str) -> bool:
|
|
return uuid in _run_onboard(["--list"], sandbox).stdout
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# #10 — agy adapter
|
|
# ---------------------------------------------------------------------------
|
|
def test_agy_purge_removes_sidecars_annotation_and_lock(tmp_path):
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
adapter = get_adapter("agy")
|
|
ctx = DiscoveryContext(workspace=str(tmp_path), agent_name="agy", home_dir=str(tmp_path))
|
|
|
|
purged = adapter.purge_artifacts(UUID, ctx)
|
|
|
|
assert len(purged) == 6, purged
|
|
for key, p in paths.items():
|
|
assert not p.exists(), f"{key} survived the purge: {p}"
|
|
assert adapter.residual_artifacts(UUID, ctx) == []
|
|
|
|
|
|
def test_agy_purge_keeps_presence_lock_while_in_use(tmp_path):
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
adapter = get_adapter("agy")
|
|
ctx = DiscoveryContext(workspace=str(tmp_path), agent_name="agy", home_dir=str(tmp_path))
|
|
holder = _holder(UUID)
|
|
try:
|
|
purged = adapter.purge_artifacts(UUID, ctx)
|
|
finally:
|
|
_stop(holder)
|
|
|
|
assert paths["lock"].exists(), "presence lock must survive while a process references the conversation"
|
|
assert str(paths["lock"]) not in purged
|
|
for key in ("db", "wal", "shm", "brain", "annotation"):
|
|
assert not paths[key].exists(), key
|
|
assert adapter.residual_artifacts(UUID, ctx) == [str(paths["lock"])]
|
|
|
|
|
|
def test_agy_purge_reports_only_what_existed(tmp_path):
|
|
"""The historical 2-item contract (db + brain) still holds when no sidecar
|
|
exists (test_a4_adapter_contract pins len == 2)."""
|
|
root = tmp_path / ".gemini" / "antigravity-cli"
|
|
(root / "conversations").mkdir(parents=True)
|
|
(root / "conversations" / f"{UUID}.db").write_text("db")
|
|
(root / "brain" / UUID).mkdir(parents=True)
|
|
adapter = get_adapter("agy")
|
|
ctx = DiscoveryContext(workspace=str(tmp_path), agent_name="agy", home_dir=str(tmp_path))
|
|
assert len(adapter.purge_artifacts(UUID, ctx)) == 2
|
|
|
|
|
|
def test_conversation_in_use_ignores_own_lineage():
|
|
"""The purge command names the uuid on its own command line (and its
|
|
parent shell does); only *other* processes count as holders."""
|
|
assert conversation_in_use(OTHER) is False
|
|
holder = _holder(OTHER)
|
|
try:
|
|
assert conversation_in_use(OTHER) is True
|
|
finally:
|
|
_stop(holder)
|
|
assert conversation_in_use(OTHER) is False
|
|
|
|
|
|
def test_conversation_in_use_excludes_only_the_non_agent_helper_chain(tmp_path):
|
|
"""The purge's own helper chain names the uuid too - the python argv and
|
|
the script that launched it - and must not count. The exclusion stops at
|
|
the nearest agent CLI ancestor, though: that process is the session the
|
|
purge runs in, and when it names the uuid the conversation is live."""
|
|
probe = ("import sys; sys.path.insert(0, %r); "
|
|
"from lib_py.agents.base import conversation_in_use; "
|
|
"print(conversation_in_use(sys.argv[1]))") % str(SKILLS)
|
|
helper = tmp_path / "helper.sh"
|
|
helper.write_text('#!/bin/sh\n"$@"\nexit $?\n')
|
|
helper.chmod(0o755)
|
|
|
|
alone = subprocess.run([str(helper), sys.executable, "-c", probe, OTHER],
|
|
capture_output=True, text=True, env=_clean_env(), timeout=60)
|
|
assert alone.stdout.strip() == "False", alone.stdout + alone.stderr
|
|
|
|
cli = _fake_agent_cli(tmp_path, "agy")
|
|
under_agent = subprocess.run([str(cli), "--conversation", OTHER, "--",
|
|
str(helper), sys.executable, "-c", probe, OTHER],
|
|
capture_output=True, text=True, env=_clean_env(), timeout=60)
|
|
assert under_agent.stdout.strip() == "True", under_agent.stdout + under_agent.stderr
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# #13 — lib_py.agents purge CLI
|
|
# ---------------------------------------------------------------------------
|
|
def _purge_cli(agent: str, uuid: str, home: Path, extra_env=None, prefix=()) -> subprocess.CompletedProcess:
|
|
env = _clean_env(extra_env)
|
|
env["HOME_DIR"] = str(home)
|
|
cmd = list(prefix) + [sys.executable, "-m", "lib_py.agents", "purge", agent, uuid, str(home)]
|
|
return subprocess.run(cmd, capture_output=True, text=True, env=env, cwd=str(home), timeout=60)
|
|
|
|
|
|
def test_purge_cli_auto_resolves_agy_from_disk(tmp_path):
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
res = _purge_cli("auto", UUID, tmp_path)
|
|
assert res.returncode == 0, res.stderr
|
|
out = json.loads(res.stdout)
|
|
assert out["agent"] == "agy"
|
|
assert len(out["purged"]) == 6 and out["residual"] == [] and out["in_use"] is False
|
|
assert not any(p.exists() for p in paths.values())
|
|
|
|
|
|
def test_purge_cli_refuses_while_in_use(tmp_path):
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
holder = _holder(UUID)
|
|
try:
|
|
res = _purge_cli("agy", UUID, tmp_path)
|
|
finally:
|
|
_stop(holder)
|
|
assert res.returncode == 4, res.stdout + res.stderr
|
|
assert json.loads(res.stdout)["in_use"] is True
|
|
assert all(p.exists() for p in paths.values()), "a refused purge must remove nothing"
|
|
|
|
|
|
def test_purge_cli_refuses_inside_the_agent_session_that_owns_the_uuid(tmp_path):
|
|
"""An orchestrator purging its own conversation from its own session: the
|
|
purge runs as a descendant of the agent CLI whose command line names the
|
|
uuid. That ancestor is the conversation's live process, not part of the
|
|
purge's helper chain, so the purge must be refused and delete nothing."""
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
cli = _fake_agent_cli(tmp_path, "claude")
|
|
res = _purge_cli("agy", UUID, tmp_path, prefix=(str(cli), "--session-id", UUID, "--"))
|
|
assert res.returncode == 4, res.stdout + res.stderr
|
|
assert json.loads(res.stdout)["in_use"] is True
|
|
assert all(p.exists() for p in paths.values()), "a refused purge must remove nothing"
|
|
|
|
|
|
@pytest.mark.parametrize("var", SESSION_ENV)
|
|
def test_purge_cli_refuses_from_the_sessions_own_shell(tmp_path, var):
|
|
"""The session a tool shell belongs to is in its environment even when no
|
|
process carries the uuid in argv and nothing holds the artifact open
|
|
(claude keeps no transcript open): a purge run from there is refused."""
|
|
paths = _agy_tree(tmp_path, UUID)
|
|
res = _purge_cli("agy", UUID, tmp_path, extra_env={var: UUID})
|
|
assert res.returncode == 4, res.stdout + res.stderr
|
|
assert json.loads(res.stdout)["in_use"] is True
|
|
assert all(p.exists() for p in paths.values()), "a refused purge must remove nothing"
|
|
|
|
|
|
def test_purge_cli_unresolved_uuid(tmp_path):
|
|
res = _purge_cli("auto", OTHER, tmp_path)
|
|
assert res.returncode == 5, res.stdout + res.stderr
|
|
assert json.loads(res.stdout)["agent"] is None
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# #13 — orc_onboard.sh --remove --purge-conversation
|
|
# ---------------------------------------------------------------------------
|
|
def test_orc_remove_purge_requires_yes(mam_sandbox):
|
|
paths = _agy_tree(mam_sandbox, UUID)
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation"], mam_sandbox)
|
|
|
|
assert res.returncode == 4, res.stdout + res.stderr
|
|
assert "Re-run with --yes" in res.stderr
|
|
assert all(p.exists() for p in paths.values())
|
|
assert _registered(mam_sandbox, UUID), "registration must be untouched without --yes"
|
|
|
|
|
|
def test_orc_remove_purge_deletes_and_unregisters(mam_sandbox):
|
|
paths = _agy_tree(mam_sandbox, UUID)
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation", "--yes"], mam_sandbox)
|
|
|
|
assert res.returncode == 0, res.stdout + res.stderr
|
|
assert "Successfully removed orchestrator UUID" in res.stdout
|
|
assert res.stdout.count("purged: ") == 6, res.stdout
|
|
assert "residual: none (agent: agy)" in res.stdout
|
|
assert not any(p.exists() for p in paths.values())
|
|
assert not _registered(mam_sandbox, UUID)
|
|
|
|
|
|
def test_orc_remove_purge_refused_while_in_use(mam_sandbox):
|
|
paths = _agy_tree(mam_sandbox, UUID)
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
holder = _holder(UUID)
|
|
try:
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation", "--yes"], mam_sandbox)
|
|
finally:
|
|
_stop(holder)
|
|
|
|
assert res.returncode == 5, res.stdout + res.stderr
|
|
assert "still in use" in res.stderr
|
|
assert all(p.exists() for p in paths.values())
|
|
assert _registered(mam_sandbox, UUID), "a refused purge must leave the registration in place"
|
|
|
|
|
|
def test_orc_remove_purge_refused_from_the_orchestrators_own_session(mam_sandbox):
|
|
"""The case the skill documents: the orchestrator removes and purges its
|
|
own UUID from inside its own session (its tool shell carries the session
|
|
id). The purge must be refused (exit 5) and the registration kept."""
|
|
paths = _agy_tree(mam_sandbox, UUID)
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation", "--yes"], mam_sandbox,
|
|
extra_env={"CLAUDE_CODE_SESSION_ID": UUID})
|
|
|
|
assert res.returncode == 5, res.stdout + res.stderr
|
|
assert "still in use" in res.stderr
|
|
assert all(p.exists() for p in paths.values())
|
|
assert _registered(mam_sandbox, UUID), "a refused purge must leave the registration in place"
|
|
|
|
|
|
def test_orc_remove_purge_without_artifacts_still_unregisters(mam_sandbox):
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation", "--yes"], mam_sandbox)
|
|
assert res.returncode == 0, res.stdout + res.stderr
|
|
assert "nothing to purge" in res.stderr
|
|
assert not _registered(mam_sandbox, UUID)
|
|
|
|
|
|
def test_orc_purge_flag_needs_remove(mam_sandbox):
|
|
res = _run_onboard(["--uuid", UUID, "--purge-conversation", "--yes"], mam_sandbox)
|
|
assert res.returncode == 2
|
|
assert not _registered(mam_sandbox, UUID)
|
|
|
|
|
|
def test_orc_dry_run_purge_changes_nothing(mam_sandbox):
|
|
paths = _agy_tree(mam_sandbox, UUID)
|
|
assert _run_onboard(["--uuid", UUID], mam_sandbox).returncode == 0
|
|
res = _run_onboard(["--remove", UUID, "--purge-conversation", "--yes", "--dry-run"], mam_sandbox)
|
|
assert res.returncode == 0
|
|
assert "purge its conversation artifacts" in res.stdout
|
|
assert all(p.exists() for p in paths.values())
|
|
assert _registered(mam_sandbox, UUID)
|