From c82ea75a15ab4251109bb769de622f714e48aa55 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 08:18:56 +0900 Subject: [PATCH] fix(issue-10,11,13,15): complete agy purge, orchestrator purge, workspace-anchored registry Issue #10 - agy purge_artifacts left resumable pieces behind: it now also removes the SQLite sidecars (-wal/-shm/-journal), the title annotation and the presence lock (the lock only once no live process references the conversation), and residual_artifacts() reports anything under ~/.gemini/antigravity-cli still carrying the uuid. Issue #13 - the orchestrator has no herdr_sessions row, so its conversation could not be purged by any skill. `lib_py.agents purge ` resolves the adapter from what is on disk (orchestrator_uuids stores bare uuids), refuses while a live process names the uuid (conversation_in_use: process list minus the caller's own lineage, plus lsof), and prints what it purged. `orc_onboard.sh --remove --purge-conversation --yes [--agent]` runs it before the registry update so a refusal changes nothing (exit 4 without --yes, 5 while in use). Issue #11 - "Successfully removeed": conjugate add/remove explicitly. Issue #15 - the job registry and audit log followed the caller's cwd, so the pytest suite (create_session.sh with its default onboarding job, run from the repo root) had left 593 test records in the real .mam/jobs. Defaults now resolve MAM_JOBS_DIR / DELEGATE_JOB_LOGS_DIR, then $WORKSPACE_ROOT/.mam/..., then the cwd (mqtt_common.default_registry_dir / get_logs_dir; registry.py, publish_event.py, service.py, create_session.sh, stop_session.sh); the sandbox fixture pins both. The tier suites that used to leak now leave the real registry untouched (881 records before and after). Tests: test_issue10_13_purge.py (13), test_issue15_registry_isolation.py (7); T-O13 pins the corrected message; the R1 helper test and B-9 logs-dir test now assert the workspace-anchored contract; publish_event snapshot updated. Refs #10 #11 #13 #15 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib.sh | 5 +- .agents/skills/lib_py/agents/__main__.py | 66 ++++- .agents/skills/lib_py/agents/adapters/agy.py | 47 +++- .agents/skills/lib_py/agents/base.py | 54 +++- .../skills/lib_py/delegation/mqtt_common.py | 35 ++- .../skills/lib_py/delegation/publish_event.py | 3 +- .agents/skills/lib_py/delegation/registry.py | 3 +- .agents/skills/lib_py/delegation/service.py | 2 +- .../scripts/create_session.sh | 2 +- .../multi-agent-mux-delegate-job/SKILL.md | 9 +- .../multi-agent-mux-orc-onboard/SKILL.md | 21 +- .../scripts/orc_onboard.sh | 95 ++++++- .agents/skills/multi-agent-mux-stop/SKILL.md | 2 +- .../scripts/stop_session.sh | 2 +- tests/conftest.py | 6 + tests/test_issue10_13_purge.py | 264 ++++++++++++++++++ tests/test_issue15_registry_isolation.py | 120 ++++++++ tests/test_issue5_phase1_delegation.py | 15 +- tests/test_orc_onboard.py | 2 + tests/test_tier1_unit.py | 3 + 20 files changed, 723 insertions(+), 33 deletions(-) create mode 100644 tests/test_issue10_13_purge.py create mode 100644 tests/test_issue15_registry_isolation.py diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index e189b32..dd2cb2e 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2036,8 +2036,9 @@ _start_watchdog_legacy() { # NOT `python -m`: for -m, sys.path[0] is the current directory, which therefore # beats PYTHONPATH. A frozen loop whose cwd happened to contain a lib_py package # would silently execute live code, losing B-13. An explicit insert of SKILL_DIR -# cannot be shadowed that way. cwd is deliberately left alone, because the -# registry and audit-log paths are relative to it. +# cannot be shadowed that way. cwd is deliberately left alone; the registry and +# audit-log paths resolve to MAM_JOBS_DIR / DELEGATE_JOB_LOGS_DIR, then to +# $WORKSPACE_ROOT/.mam/..., and only then relative to it (Issue #15). _delegation_py() { PYTHONPATH="$SKILL_DIR:${PYTHONPATH:-}" "$(_delegate_py_bin)" -c ' import sys diff --git a/.agents/skills/lib_py/agents/__main__.py b/.agents/skills/lib_py/agents/__main__.py index ce07ae4..b244a1c 100644 --- a/.agents/skills/lib_py/agents/__main__.py +++ b/.agents/skills/lib_py/agents/__main__.py @@ -1,11 +1,48 @@ # __main__.py — CLI bridge for lib_py.agents facts and resolution (N4) -import sys, json, shlex +import os, sys, json, shlex from lib_py.agents.registry import get_adapter, own_key, agent_of_row +# `purge auto` probes adapters in this order; opencode last because its +# artifact path is one shared DB that exists for every uuid. +PURGE_PROBE_ORDER = ('agy', 'claude', 'hermes', 'grok', 'codex', 'opencode') + + +def _resolve_purge_target(agent_hint: str, uuid: str, workspace: str): + """(agent, adapter, ctx) whose on-disk artifact carries `uuid`, else None. + + An explicit agent is trusted as long as its artifact exists. `auto` tries + each adapter's own verification first (revalidate mode: an orchestrator + conversation is not in any workspace cache), then a plain path check for + adapters whose artifact path embeds the uuid. + """ + from lib_py.agents.base import DiscoveryContext + names = [agent_hint] if agent_hint and agent_hint != 'auto' else list(PURGE_PROBE_ORDER) + probes = [] + for name in names: + adapter = get_adapter(name) + if not adapter: + continue + probes.append((name, adapter, DiscoveryContext(workspace, name, mode="revalidate"))) + for name, adapter, ctx in probes: + try: + if adapter.verify_artifact(uuid, ctx): + return name, adapter, ctx + except Exception: + pass + for name, adapter, ctx in probes: + try: + path = adapter.artifact_path(uuid, ctx) + except Exception: + continue + if uuid in path and os.path.exists(path): + return name, adapter, ctx + return None + + def main(): if len(sys.argv) < 2: - print("Usage: python -m lib_py.agents [args...]", file=sys.stderr) + print("Usage: python -m lib_py.agents [args...]", file=sys.stderr) sys.exit(1) cmd = sys.argv[1] @@ -83,6 +120,31 @@ def main(): print('/exit') sys.exit(0) + elif cmd == 'purge': + # purge [workspace] (Issue #13) + # Deletes a conversation's on-disk artifacts through its adapter and + # prints one JSON object: {"agent", "purged", "residual", "in_use"}. + # Exit 0 purged; 4 refused, conversation still in use (nothing + # removed); 5 no adapter has an artifact for this uuid. + agent_hint = sys.argv[2] if len(sys.argv) > 2 else 'auto' + uuid = sys.argv[3] if len(sys.argv) > 3 else '' + workspace = sys.argv[4] if len(sys.argv) > 4 and sys.argv[4] else os.getcwd() + if not uuid: + print("ERROR: purge requires ", file=sys.stderr) + sys.exit(1) + target = _resolve_purge_target(agent_hint, uuid, workspace) + if not target: + print(json.dumps({"agent": None, "purged": [], "residual": [], "in_use": False})) + sys.exit(5) + name, adapter, ctx = target + if adapter.conversation_in_use(uuid, ctx): + print(json.dumps({"agent": name, "purged": [], "residual": [], "in_use": True})) + sys.exit(4) + purged = adapter.purge_artifacts(uuid, ctx) + residual = adapter.residual_artifacts(uuid, ctx) + print(json.dumps({"agent": name, "purged": purged, "residual": residual, "in_use": False})) + sys.exit(0) + else: print(f"ERROR: Unknown CLI command {cmd!r}", file=sys.stderr) sys.exit(1) diff --git a/.agents/skills/lib_py/agents/adapters/agy.py b/.agents/skills/lib_py/agents/adapters/agy.py index be06cce..c91b8da 100644 --- a/.agents/skills/lib_py/agents/adapters/agy.py +++ b/.agents/skills/lib_py/agents/adapters/agy.py @@ -1,4 +1,4 @@ -import os, json, sqlite3, shutil +import os, sys, json, sqlite3, shutil from typing import Optional, Any from lib_py.agents.base import BaseAgentAdapter, DiscoveryContext @@ -71,18 +71,55 @@ class AgyAgentAdapter(BaseAgentAdapter): return False return True + def _cli_root(self, ctx: DiscoveryContext) -> str: + return f"{ctx.home_dir}/.gemini/antigravity-cli" + def purge_artifacts(self, uuid: str, ctx: DiscoveryContext) -> list: + # Issue #10: a purge must leave nothing resumable behind. Besides the + # conversation DB and the brain dir, agy keeps SQLite sidecars (-wal / + # -shm / -journal, created whenever the DB is opened - the stop path + # itself does so while capturing the id, and an unflushed WAL can still + # hold conversation content), the title annotation and a presence lock. purged = [] db = self.artifact_path(uuid, ctx) - if os.path.exists(db): - os.remove(db) - purged.append(db) - brain = f"{ctx.home_dir}/.gemini/antigravity-cli/brain/{uuid}" + for path in [db] + [f"{db}{suffix}" for suffix in ("-wal", "-shm", "-journal")]: + if os.path.exists(path): + os.remove(path) + purged.append(path) + root = self._cli_root(ctx) + brain = f"{root}/brain/{uuid}" if os.path.isdir(brain): shutil.rmtree(brain, ignore_errors=True) purged.append(brain) + annotation = f"{root}/annotations/{uuid}.pbtxt" + if os.path.exists(annotation): + os.remove(annotation) + purged.append(annotation) + # The presence lock marks a conversation some process has open; only + # remove it once nothing references the conversation any more. + lock = f"{root}/presence/{uuid}.lock" + if os.path.exists(lock): + if self.conversation_in_use(uuid, ctx): + print(f"WARN: presence lock kept, conversation {uuid} is still in use: {lock}", file=sys.stderr) + else: + os.remove(lock) + purged.append(lock) return purged + def residual_artifacts(self, uuid: str, ctx: DiscoveryContext) -> list: + """Anything under the agy CLI tree still carrying this conversation id.""" + root = self._cli_root(ctx) + if not os.path.isdir(root): + return [] + found = [] + for dirpath, dirnames, filenames in os.walk(root): + for name in dirnames + filenames: + if uuid in name: + found.append(os.path.join(dirpath, name)) + # Do not descend into a matched directory; it is reported as a whole. + dirnames[:] = [d for d in dirnames if uuid not in d] + return sorted(found) + def spawn_spec(self, binary: str, session_uuid: str = "", use_wrapper: bool = False) -> str: return f"{binary} --dangerously-skip-permissions" diff --git a/.agents/skills/lib_py/agents/base.py b/.agents/skills/lib_py/agents/base.py index ca2a306..b75f47f 100644 --- a/.agents/skills/lib_py/agents/base.py +++ b/.agents/skills/lib_py/agents/base.py @@ -1,10 +1,53 @@ # base.py — BaseAgentAdapter abstract base class, SpawnSpec, and DiscoveryContext -import os, json, sqlite3 +import os, json, sqlite3, shutil, subprocess from typing import Optional, Dict, Any, List from lib_py.paths import resolve_home from lib_py.verify_session import verify_session_uuid, workspace_key + +def conversation_in_use(uuid: str, paths=()) -> bool: + """A live process still references the conversation (Issues #10, #13). + + Either its id is on a command line other than this process's own lineage + (`claude -r `, `agy --conversation `, ... - the purge command + itself names the uuid, so its ancestors are excluded), or one of `paths` + is held open (lsof, where available). A purge must not touch a + conversation that is in use. + """ + try: + out = subprocess.run(["ps", "-axo", "pid=,ppid=,command="], + capture_output=True, text=True, timeout=10).stdout + except Exception: + out = "" + parents: Dict[int, int] = {} + holders: List[int] = [] + for line in out.splitlines(): + parts = line.split(None, 2) + if len(parts) < 2 or not (parts[0].isdigit() and parts[1].isdigit()): + continue + pid, ppid = int(parts[0]), int(parts[1]) + parents[pid] = ppid + if len(parts) > 2 and uuid in parts[2]: + holders.append(pid) + if holders: + lineage = set() + p = os.getpid() + while p and p not in lineage: + lineage.add(p) + p = parents.get(p, 0) + if any(h not in lineage for h in holders): + return True + existing = [p for p in paths if p and os.path.exists(p)] + if existing and shutil.which("lsof"): + try: + r = subprocess.run(["lsof", "-t"] + existing, capture_output=True, text=True, timeout=10) + if r.stdout.strip(): + return True + except Exception: + pass + return False + class SpawnSpec: def __init__(self, agent_name: str, session_name: str, command: str, env: Optional[Dict[str, str]] = None): self.agent_name = agent_name @@ -107,6 +150,15 @@ class BaseAgentAdapter: def purge_artifacts(self, uuid: str, ctx: DiscoveryContext) -> list: raise NotImplementedError + def residual_artifacts(self, uuid: str, ctx: DiscoveryContext) -> list: + """Paths still carrying `uuid` after a purge. Adapters that know their + on-disk tree override this; the default has nothing to scan.""" + return [] + + 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)]) + def spawn_spec(self, binary: str, session_uuid: str = "", use_wrapper: bool = False) -> str: raise NotImplementedError diff --git a/.agents/skills/lib_py/delegation/mqtt_common.py b/.agents/skills/lib_py/delegation/mqtt_common.py index f06b83b..533a1d5 100644 --- a/.agents/skills/lib_py/delegation/mqtt_common.py +++ b/.agents/skills/lib_py/delegation/mqtt_common.py @@ -137,6 +137,31 @@ DEFAULT_REGISTRY_DIR = ".mam/jobs" DEFAULT_TOPIC_ROOT = "python/mqtt/jobs" LOCK_FILENAME = ".lock" + +def _workspace_mam_dir(leaf: str) -> Optional[str]: + """`$WORKSPACE_ROOT/.mam/` when lib.sh has told us which workspace we + serve; None otherwise.""" + root = os.environ.get("WORKSPACE_ROOT") + if root and root.strip() and os.path.isdir(root): + return os.path.join(root, ".mam", leaf) + return None + + +def default_registry_dir() -> str: + """Registry root, resolved at call time (Issue #15). + + ``MAM_JOBS_DIR`` wins; otherwise the workspace lib.sh exported as + ``WORKSPACE_ROOT``; otherwise the historical cwd-relative ``.mam/jobs``. + Anchoring on the workspace is what keeps a test sandbox (or any caller + whose cwd is not the workspace) from registering jobs into whichever + ``.mam/jobs`` happens to sit under the current directory - the pytest + suite used to leave hundreds of onboarding records in the real registry. + """ + env = os.environ.get("MAM_JOBS_DIR") + if env and env.strip(): + return env + return _workspace_mam_dir("jobs") or DEFAULT_REGISTRY_DIR + # Persistent audit-log layout: .mam/delegate_job_logs//{meta,events,status}. # This is a *separate* artifact from the registry: the registry is the live job # record (mutated in place), the audit log is an append-only history that @@ -150,14 +175,16 @@ def get_logs_dir() -> str: """Audit-log root, resolved at call time (B-9). Overridable with ``DELEGATE_JOB_LOGS_DIR``; otherwise - ``/.mam/delegate_job_logs``. Resolved per call rather than at import - so a chdir after import cannot strand the audit trail in the old tree — - the same reason ``DEFAULT_REGISTRY_DIR`` stays a relative string. + ``$WORKSPACE_ROOT/.mam/delegate_job_logs`` (Issue #15, see + ``default_registry_dir``); otherwise ``/.mam/delegate_job_logs``. + Resolved per call rather than at import so a chdir after import cannot + strand the audit trail in the old tree — the same reason + ``DEFAULT_REGISTRY_DIR`` stays a relative string. """ env = os.environ.get("DELEGATE_JOB_LOGS_DIR") if env and env.strip(): return env - return os.path.join(os.getcwd(), ".mam", "delegate_job_logs") + return _workspace_mam_dir("delegate_job_logs") or os.path.join(os.getcwd(), ".mam", "delegate_job_logs") def __getattr__(name: str): # PEP 562 (3.7+) diff --git a/.agents/skills/lib_py/delegation/publish_event.py b/.agents/skills/lib_py/delegation/publish_event.py index 98b3683..2a4a807 100755 --- a/.agents/skills/lib_py/delegation/publish_event.py +++ b/.agents/skills/lib_py/delegation/publish_event.py @@ -35,6 +35,7 @@ from lib_py.delegation.mqtt_common import ( DEFAULT_REGISTRY_DIR, SCHEMA_VERSION, broker_config_from_job, + default_registry_dir, events_topic_for, load_job, next_seq, @@ -242,7 +243,7 @@ def main(argv=None) -> int: parser.add_argument("--data", default=None, help="optional JSON object string") parser.add_argument("--retained", action="store_true", help="force retain=True (auto for completed/error)") - parser.add_argument("--registry-dir", default=DEFAULT_REGISTRY_DIR) + parser.add_argument("--registry-dir", default=default_registry_dir()) parser.add_argument("--attempts", type=int, default=3) parser.add_argument("--finalize-report", default=None, metavar="PATH", help="atomically finalize this report and declare its " diff --git a/.agents/skills/lib_py/delegation/registry.py b/.agents/skills/lib_py/delegation/registry.py index 3174fd5..d73bd15 100755 --- a/.agents/skills/lib_py/delegation/registry.py +++ b/.agents/skills/lib_py/delegation/registry.py @@ -28,6 +28,7 @@ from lib_py.delegation.mqtt_common import ( _atomic_write_record, _utcnow, broker_config_from_env, + default_registry_dir, load_job, registry_lock, topic_prefix_for, @@ -266,7 +267,7 @@ def get_feedback(job_id: str, registry_dir: str = DEFAULT_REGISTRY_DIR) -> str: # -------------------------------------------------------------------------- def _build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser(description="multi-agent-mux-delegate-job registry CLI") - parser.add_argument("--registry-dir", default=DEFAULT_REGISTRY_DIR) + parser.add_argument("--registry-dir", default=default_registry_dir()) sub = parser.add_subparsers(dest="command", required=True) p_reg = sub.add_parser("register", help="create a pending job; prints the job id") diff --git a/.agents/skills/lib_py/delegation/service.py b/.agents/skills/lib_py/delegation/service.py index a281b63..86c72e6 100644 --- a/.agents/skills/lib_py/delegation/service.py +++ b/.agents/skills/lib_py/delegation/service.py @@ -18,7 +18,7 @@ logger = logging.getLogger("delegate_job.service") class DelegationService(object): def __init__(self, registry_dir=None, persistent=False): - self.registry_dir = registry_dir or mqtt_common.DEFAULT_REGISTRY_DIR + self.registry_dir = registry_dir or mqtt_common.default_registry_dir() self.persistent = persistent self._publisher = None diff --git a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh index e9a69e7..9df4a15 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -350,7 +350,7 @@ if [ -n "$SUBMIT_JOB_PROMPT" ]; then local_info=$(python3 -c " import sys, json, os jid = '$DELEGATE_JOB_ID' -reg_dir = os.environ.get('MAM_JOBS_DIR', '.mam/jobs') +reg_dir = os.environ.get('MAM_JOBS_DIR') or (os.path.join(os.environ['WORKSPACE_ROOT'], '.mam', 'jobs') if os.environ.get('WORKSPACE_ROOT') else '.mam/jobs') p = os.path.join(reg_dir, f'{jid}.json') if os.path.exists(p): try: diff --git a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md index 1fb22e8..878781e 100644 --- a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md +++ b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md @@ -109,8 +109,13 @@ Which tree wins, by execution context: `SKILL_DIR` at the front of the import path. It deliberately does **not** use `python -m`: for `-m` the current directory precedes `PYTHONPATH`, so a frozen loop whose working directory happened to contain a `lib_py` package would have -silently executed live code. The working directory is left untouched, because -the registry and audit-log paths are relative to it. +silently executed live code. The working directory is left untouched; the +registry and audit-log paths resolve to `MAM_JOBS_DIR` / `DELEGATE_JOB_LOGS_DIR` +when set, otherwise to `$WORKSPACE_ROOT/.mam/jobs` and +`$WORKSPACE_ROOT/.mam/delegate_job_logs` (the workspace `lib.sh` exports), and +only without either to the cwd-relative `.mam/...` (Issue #15 — this is what +stops a caller whose cwd is not the workspace, such as the pytest suite, from +registering jobs into a foreign registry). Setting `MAM_DELEGATION_PY=0` switches the shell helpers back to their preserved legacy bodies with no redeploy. diff --git a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md index 76ed504..dae291a 100644 --- a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md +++ b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md @@ -26,10 +26,27 @@ Registers an orchestrator session UUID into the `orchestrator_uuids` list of `.m ### Options - `--uuid `: Explicitly register the specified orchestrator session UUID. - `--remove `: Remove the specified UUID from the `orchestrator_uuids` list. +- `--purge-conversation`: With `--remove`, also delete the conversation's on-disk artifacts through its agent adapter (Issue #13). Irreversible — requires `--yes`; refused (exit `5`, registration untouched) while a live process still references the conversation, e.g. when run from inside that very session. +- `--yes`: Skip the `--purge-conversation` confirmation (without it the script prints the danger notice and exits `4`). +- `--agent `: Adapter to purge with (`claude|agy|hermes|grok|opencode|codex`). Default `auto`: each adapter is asked whether it owns an artifact for the UUID (`agy` → `~/.gemini/antigravity-cli/conversations/.db` etc.). Needed because `orchestrator_uuids` stores bare UUIDs, not the agent kind. - `--list`: Display all currently registered orchestrator UUIDs. - `--no-autodetect`: Disable process ancestry auto-detection when `--uuid` is not provided. - `--dry-run`: Output proposed changes without mutating the state registry. +### Purging the orchestrator conversation + +`multi-agent-mux-stop --purge-conversation` only reaches sessions that have a `herdr_sessions` row; the orchestrator never has one. To wipe an orchestrator conversation for good: + +```bash +.agents/skills/multi-agent-mux-orc-onboard/scripts/orc_onboard.sh --remove --purge-conversation --yes +# purged: ~/.gemini/antigravity-cli/conversations/.db +# purged: ~/.gemini/antigravity-cli/brain/ +# ... +# residual: none (agent: agy) +``` + +The purge runs before the registry update, so a refused purge (conversation in use, exit `5`) changes nothing. If no adapter finds an artifact for the UUID the registration is still removed with a warning. For agy the adapter also removes the SQLite sidecars (`-wal`/`-shm`/`-journal`), the title annotation and the presence lock (Issue #10) and reports anything under `~/.gemini/antigravity-cli` that still carries the UUID as `WARN residual:`. + ## Auto-Detection Hierarchy (Rev.2) When `--uuid` is omitted, `orc_onboard.sh` inspects the process ancestry tree of the nearest agent ancestor (`claude`, `agy`, `hermes`, `grok`, `opencode`, `codex`) in the following order: @@ -51,5 +68,7 @@ When `--uuid` is omitted, `orc_onboard.sh` inspects the process ancestry tree of ## Exit Statuses - `0`: Success (registered, removed, listed, or already present). - `1`: Conflict — the ID is currently owned by a `running` sub-agent session row. -- `2`: Invalid arguments or malformed ID format. +- `2`: Invalid arguments or malformed ID format (including `--purge-conversation` without `--remove`). - `3`: Detection failed — ID could not be determined. +- `4`: `--purge-conversation` needs `--yes` — nothing changed. +- `5`: Purge refused — the conversation is still in use by a live process; nothing changed. diff --git a/.agents/skills/multi-agent-mux-orc-onboard/scripts/orc_onboard.sh b/.agents/skills/multi-agent-mux-orc-onboard/scripts/orc_onboard.sh index 3348bc3..9235ea1 100755 --- a/.agents/skills/multi-agent-mux-orc-onboard/scripts/orc_onboard.sh +++ b/.agents/skills/multi-agent-mux-orc-onboard/scripts/orc_onboard.sh @@ -13,17 +13,26 @@ MODE="add" TARGET_UUID="" AUTODETECT=true DRY_RUN=false +PURGE=false +YES=false +AGENT_HINT="auto" usage() { cat <<'EOF' Usage: orc_onboard.sh [OPTIONS] Options: - --uuid Register specified orchestrator session UUID - --remove Remove specified UUID from orchestrator_uuids list - --list List all registered orchestrator UUIDs - --no-autodetect Disable process ancestry auto-detection when --uuid is omitted - --dry-run Preview changes without updating state registry + --uuid Register specified orchestrator session UUID + --remove Remove specified UUID from orchestrator_uuids list + --purge-conversation With --remove: also delete the conversation's on-disk + artifacts through its agent adapter (irreversible; + refused while a live process still references it) + --yes Skip the --purge-conversation confirmation + --agent Adapter for --purge-conversation (claude|agy|hermes| + grok|opencode|codex); default: auto-detect from disk + --list List all registered orchestrator UUIDs + --no-autodetect Disable process ancestry auto-detection when --uuid is omitted + --dry-run Preview changes without updating state registry EOF exit 2 } @@ -46,6 +55,19 @@ while [ $# -gt 0 ]; do MODE="list" shift ;; + --purge-conversation) + PURGE=true + shift + ;; + --yes) + YES=true + shift + ;; + --agent) + if [ $# -lt 2 ] || [ -z "$2" ]; then usage; fi + AGENT_HINT="$2" + shift 2 + ;; --no-autodetect) AUTODETECT=false shift @@ -197,6 +219,11 @@ if ! is_valid_id "$TARGET_UUID"; then exit 2 fi +if [ "$PURGE" = true ] && [ "$MODE" != "remove" ]; then + echo "ERROR: --purge-conversation only applies together with --remove ." >&2 + exit 2 +fi + if [ "$MODE" = "add" ]; then # Check if running sub-agent session row currently owns this UUID check_owner_rc=0 @@ -222,10 +249,45 @@ sys.exit(0) fi if [ "$DRY_RUN" = true ]; then - echo "[dry-run] would $MODE orchestrator uuid: $TARGET_UUID" + if [ "$PURGE" = true ]; then + echo "[dry-run] would remove orchestrator uuid: $TARGET_UUID and purge its conversation artifacts (agent: $AGENT_HINT)" + else + echo "[dry-run] would $MODE orchestrator uuid: $TARGET_UUID" + fi exit 0 fi +# Issue #13: the orchestrator has no herdr_sessions row, so stop_session.sh +# --purge-conversation can never reach its conversation. Purge FIRST, through +# the adapter (`lib_py.agents purge`): a refusal because the conversation is +# still in use leaves the registration untouched. +PURGE_JSON="" +if [ "$PURGE" = true ]; then + if [ "$YES" != true ]; then + echo "DANGER: --purge-conversation will DELETE orchestrator conversation $TARGET_UUID from disk." >&2 + echo " This cannot be undone: no future resume of that conversation." >&2 + echo " Re-run with --yes to confirm." >&2 + exit 4 + fi + purge_rc=0 + PURGE_JSON="$("$(_delegate_py_bin)" -m lib_py.agents purge "$AGENT_HINT" "$TARGET_UUID" "$WORKSPACE_ROOT")" || purge_rc=$? + case "$purge_rc" in + 0) ;; + 4) + echo "ERROR: conversation $TARGET_UUID is still in use (a live process references it); nothing purged, registration unchanged." >&2 + exit 5 + ;; + 5) + echo "WARN: no on-disk conversation found for $TARGET_UUID (agent: $AGENT_HINT); nothing to purge." >&2 + PURGE_JSON="" + ;; + *) + echo "ERROR: purge failed (rc=$purge_rc): $PURGE_JSON" >&2 + exit 1 + ;; + esac +fi + MAM_TARGET_UUID="$TARGET_UUID" MAM_OP_MODE="$MODE" atomic_dump_yaml "$AGENT_SESSIONS_YAML" <<'PYEOF' import os, sys, json @@ -243,4 +305,23 @@ elif op == "remove": d["orchestrator_uuids"] = uuids PYEOF -echo "Successfully ${MODE}ed orchestrator UUID: $TARGET_UUID" +case "$MODE" in + add) verb="added" ;; + remove) verb="removed" ;; + *) verb="${MODE}ed" ;; +esac +echo "Successfully $verb orchestrator UUID: $TARGET_UUID" + +if [ -n "$PURGE_JSON" ]; then + printf '%s' "$PURGE_JSON" | python3 -c ' +import json, sys +d = json.loads(sys.stdin.read() or "{}") +for p in d.get("purged", []): + print("purged: %s" % p) +residual = d.get("residual", []) +for p in residual: + print("WARN residual: %s" % p) +if not residual: + print("residual: none (agent: %s)" % d.get("agent")) +' +fi diff --git a/.agents/skills/multi-agent-mux-stop/SKILL.md b/.agents/skills/multi-agent-mux-stop/SKILL.md index 49044bd..d513000 100644 --- a/.agents/skills/multi-agent-mux-stop/SKILL.md +++ b/.agents/skills/multi-agent-mux-stop/SKILL.md @@ -105,7 +105,7 @@ The script: 2. If `delegate_job_id` is set, automatically publishes a `progress --detail "terminating"` event to the multi-agent-mux-delegate-job registry 3. Captures the `last_visible_status` from `herdr capture-pane` (so we have a final TUI snapshot for audit) 4. Attempts graceful exit keys → SIGTERM kill-session → SIGKILL fallback -5. For `purge-conversation`: deletes `~/.claude/projects/.../jsonl` (claude) or `~/.gemini/antigravity-cli/conversations/...db` + `brain/...` (agy) +5. For `purge-conversation`: deletes `~/.claude/projects/.../jsonl` (claude) or, for agy, `~/.gemini/antigravity-cli/conversations/.db` with its `-wal`/`-shm`/`-journal` sidecars, `brain//`, `annotations/.pbtxt` and `presence/.lock` (the lock only once no live process references the conversation — Issue #10) 6. Updates the YAML entry and SQLite database atomically 7. If `delegate_job_id` is set, publishes a `completed` event to the multi-agent-mux-delegate-job registry diff --git a/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh b/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh index 9b4514c..b006783 100755 --- a/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh +++ b/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh @@ -148,7 +148,7 @@ if [ -n "$DELEGATE_JOB_ID" ]; then local_info=$(python3 -c " import sys, json, os jid = '$DELEGATE_JOB_ID' -reg_dir = os.environ.get('MAM_JOBS_DIR', '.mam/jobs') +reg_dir = os.environ.get('MAM_JOBS_DIR') or (os.path.join(os.environ['WORKSPACE_ROOT'], '.mam', 'jobs') if os.environ.get('WORKSPACE_ROOT') else '.mam/jobs') p = os.path.join(reg_dir, f'{jid}.json') if os.path.exists(p): try: diff --git a/tests/conftest.py b/tests/conftest.py index ceda407..5164733 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -42,6 +42,12 @@ def mam_sandbox(tmp_path, monkeypatch): monkeypatch.setenv("LOCAL_BIN", str(tmp_path / ".local" / "bin")) monkeypatch.setenv("AGENT_SESSIONS_YAML", str(yaml_path)) monkeypatch.setenv("WORKSPACE_ROOT", str(tmp_path)) + # Issue #15: the job registry and its audit log must land in the sandbox + # too, whatever the cwd of the script under test - not in the repo's own + # .mam/jobs. Explicit here even though the defaults now follow + # WORKSPACE_ROOT, so a test that unsets WORKSPACE_ROOT still stays inside. + monkeypatch.setenv("MAM_JOBS_DIR", str(mam_dir / "jobs")) + monkeypatch.setenv("DELEGATE_JOB_LOGS_DIR", str(mam_dir / "delegate_job_logs")) # D6 (issue #5 phase 1): point the delegation env loader at a file that does not # exist, so importing or running delegation code inside a sandbox never injects the # real .mam.env into this process's os.environ. diff --git a/tests/test_issue10_13_purge.py b/tests/test_issue10_13_purge.py new file mode 100644 index 0000000..a76443c --- /dev/null +++ b/tests/test_issue10_13_purge.py @@ -0,0 +1,264 @@ +#!/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 --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. +""" +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" + + +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 ` or `claude -r ` 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 _run_onboard(args, sandbox: Path, extra_env=None) -> subprocess.CompletedProcess: + env = dict(os.environ) + for k in ("CLAUDE_CODE_SESSION_ID", "ANTIGRAVITY_CONVERSATION_ID", "HERMES_SESSION_ID", "MAM_ORCHESTRATOR_UUIDS"): + env.pop(k, None) + env.update({ + "PYTHONPATH": f"{SKILLS}:{env.get('PYTHONPATH', '')}", + "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 + + +# --------------------------------------------------------------------------- +# #13 — lib_py.agents purge CLI +# --------------------------------------------------------------------------- +def _purge_cli(agent: str, uuid: str, home: Path) -> subprocess.CompletedProcess: + env = dict(os.environ, HOME_DIR=str(home), PYTHONPATH=f"{SKILLS}:{os.environ.get('PYTHONPATH', '')}") + return subprocess.run([sys.executable, "-m", "lib_py.agents", "purge", agent, uuid, str(home)], + 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_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_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) diff --git a/tests/test_issue15_registry_isolation.py b/tests/test_issue15_registry_isolation.py new file mode 100644 index 0000000..a1f232f --- /dev/null +++ b/tests/test_issue15_registry_isolation.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Issue #15 — the job registry and its audit log follow the workspace, not +the caller's cwd. + +`DEFAULT_REGISTRY_DIR` (".mam/jobs") and the audit-log root were resolved +relative to the current directory, so `create_session.sh`'s default onboarding +job — registered through `lib_py.delegation.cli submit` without a +`--registry-dir` — landed in whatever `.mam/jobs` sat under the cwd. Run from +the repo root, the pytest suite left hundreds of `running` test records in the +real registry. The defaults now honour `MAM_JOBS_DIR` / `DELEGATE_JOB_LOGS_DIR`, +then `$WORKSPACE_ROOT/.mam/...`, and only then the cwd. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS = REPO_ROOT / ".agents" / "skills" +REAL_JOBS = REPO_ROOT / ".mam" / "jobs" +REAL_LOGS = REPO_ROOT / ".mam" / "delegate_job_logs" + +sys.path.insert(0, str(SKILLS)) +from lib_py.delegation import mqtt_common # noqa: E402 + + +def _listing(d: Path) -> set: + return set(os.listdir(d)) if d.is_dir() else set() + + +# --------------------------------------------------------------------------- +# resolution order +# --------------------------------------------------------------------------- +def test_registry_dir_env_wins(monkeypatch, tmp_path): + monkeypatch.setenv("MAM_JOBS_DIR", str(tmp_path / "explicit")) + monkeypatch.setenv("WORKSPACE_ROOT", str(tmp_path)) + assert mqtt_common.default_registry_dir() == str(tmp_path / "explicit") + + +def test_registry_dir_follows_workspace_root(monkeypatch, tmp_path): + monkeypatch.delenv("MAM_JOBS_DIR", raising=False) + monkeypatch.setenv("WORKSPACE_ROOT", str(tmp_path)) + assert mqtt_common.default_registry_dir() == str(tmp_path / ".mam" / "jobs") + + +def test_registry_dir_falls_back_to_cwd_relative(monkeypatch, tmp_path): + monkeypatch.delenv("MAM_JOBS_DIR", raising=False) + monkeypatch.delenv("WORKSPACE_ROOT", raising=False) + assert mqtt_common.default_registry_dir() == ".mam/jobs" + # A WORKSPACE_ROOT that is not a directory is ignored, not trusted. + monkeypatch.setenv("WORKSPACE_ROOT", str(tmp_path / "does-not-exist")) + assert mqtt_common.default_registry_dir() == ".mam/jobs" + + +def test_logs_dir_follows_the_same_order(monkeypatch, tmp_path): + monkeypatch.setenv("DELEGATE_JOB_LOGS_DIR", str(tmp_path / "logs")) + monkeypatch.setenv("WORKSPACE_ROOT", str(tmp_path)) + assert mqtt_common.get_logs_dir() == str(tmp_path / "logs") + monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR") + assert mqtt_common.get_logs_dir() == str(tmp_path / ".mam" / "delegate_job_logs") + monkeypatch.delenv("WORKSPACE_ROOT") + assert mqtt_common.get_logs_dir() == os.path.join(os.getcwd(), ".mam", "delegate_job_logs") + + +def test_cli_defaults_are_resolved_per_process(monkeypatch, tmp_path): + """registry.py / publish_event.py build their argparse defaults at call + time, so a process started with WORKSPACE_ROOT set registers there.""" + monkeypatch.delenv("MAM_JOBS_DIR", raising=False) + env = dict(os.environ, WORKSPACE_ROOT=str(tmp_path), PYTHONPATH=f"{SKILLS}:{os.environ.get('PYTHONPATH', '')}") + env.pop("MAM_JOBS_DIR", None) + res = subprocess.run( + [sys.executable, "-m", "lib_py.delegation.cli", "submit", "--prompt", "issue 15", "--agent", "claude-code", + "--agent-session", "herdr:issue15-probe"], + capture_output=True, text=True, env=env, cwd=str(REPO_ROOT), timeout=60, + ) + assert res.returncode == 0, res.stderr + records = list((tmp_path / ".mam" / "jobs").glob("*.json")) + assert len(records) == 1, records + assert json.loads(records[0].read_text())["agent_session"] == "herdr:issue15-probe" + + +# --------------------------------------------------------------------------- +# the leak itself: create_session.sh's onboarding job from the repo root cwd +# --------------------------------------------------------------------------- +def _create_session_from_repo_root(sandbox: Path, session: str) -> subprocess.CompletedProcess: + script = sandbox / ".agents" / "skills" / "multi-agent-mux-create" / "scripts" / "create_session.sh" + # cwd deliberately the repo root — the exact shape of the leaking tests. + return subprocess.run( + ["bash", str(script), "--workspace", str(sandbox), "--agent", "claude", "--role", "Creator", + "--session", session], + capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=180, + ) + + +def _assert_registered_in_sandbox_only(sandbox: Path, session: str, before_jobs: set, before_logs: set, res): + assert res.returncode == 0, res.stderr + res.stdout + assert _listing(REAL_JOBS) == before_jobs, "onboarding job leaked into the repo's .mam/jobs" + assert _listing(REAL_LOGS) == before_logs, "audit log leaked into the repo's .mam/delegate_job_logs" + records = [json.loads(p.read_text()) for p in (sandbox / ".mam" / "jobs").glob("*.json")] + assert any(r.get("agent_session") == f"herdr:{session}" for r in records), records + + +def test_onboarding_job_stays_in_sandbox_with_explicit_env(mam_sandbox, mock_herdr, mock_agents): + before_jobs, before_logs = _listing(REAL_JOBS), _listing(REAL_LOGS) + res = _create_session_from_repo_root(mam_sandbox, "issue15-explicit-creator-claude") + _assert_registered_in_sandbox_only(mam_sandbox, "issue15-explicit-creator-claude", before_jobs, before_logs, res) + + +def test_onboarding_job_follows_workspace_root_alone(mam_sandbox, mock_herdr, mock_agents, monkeypatch): + """Without the explicit MAM_JOBS_DIR / DELEGATE_JOB_LOGS_DIR overrides the + WORKSPACE_ROOT that lib.sh exports must be enough to keep the record out + of the repo's registry.""" + monkeypatch.delenv("MAM_JOBS_DIR", raising=False) + monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR", raising=False) + before_jobs, before_logs = _listing(REAL_JOBS), _listing(REAL_LOGS) + res = _create_session_from_repo_root(mam_sandbox, "issue15-wsroot-creator-claude") + _assert_registered_in_sandbox_only(mam_sandbox, "issue15-wsroot-creator-claude", before_jobs, before_logs, res) diff --git a/tests/test_issue5_phase1_delegation.py b/tests/test_issue5_phase1_delegation.py index 70e777e..32c851a 100644 --- a/tests/test_issue5_phase1_delegation.py +++ b/tests/test_issue5_phase1_delegation.py @@ -314,7 +314,9 @@ def test_publish_event_cli_contract_snapshot(mam_sandbox): # Defaults that shell and wrapper callers depend on. src = Path(publish_event.__file__).read_text(encoding="utf-8") assert '"--attempts", type=int, default=3' in src - assert '"--registry-dir", default=DEFAULT_REGISTRY_DIR' in src + # Issue #15: resolved per process (MAM_JOBS_DIR, then WORKSPACE_ROOT, then + # the cwd-relative DEFAULT_REGISTRY_DIR) instead of the bare constant. + assert '"--registry-dir", default=default_registry_dir()' in src assert '"--event", default="progress", choices=VALID_EVENTS' in src @@ -1063,8 +1065,15 @@ def test_r1_shell_helper_ignores_a_lib_py_in_the_working_directory(mam_sandbox, assert res.returncode == 0, res.stderr job_id = res.stdout.strip() assert re.fullmatch(r"[0-9a-f]+", job_id), repr(res.stdout) - assert (decoy / ".mam" / "jobs" / (job_id + ".json")).exists(), ( - "the real cli should have registered the job in the cwd registry" + # Issue #15: the registry follows MAM_JOBS_DIR / WORKSPACE_ROOT (both point + # at the sandbox here), no longer the cwd — the decoy workspace's own + # .mam/jobs must stay empty. + registry = Path(env["MAM_JOBS_DIR"]) + assert (registry / (job_id + ".json")).exists(), ( + "the real cli should have registered the job in the sandbox registry" + ) + assert not (decoy / ".mam" / "jobs" / (job_id + ".json")).exists(), ( + "the job must not be registered relative to the cwd" ) diff --git a/tests/test_orc_onboard.py b/tests/test_orc_onboard.py index 979c054..ada9d33 100644 --- a/tests/test_orc_onboard.py +++ b/tests/test_orc_onboard.py @@ -242,6 +242,8 @@ def test_o13_onboard_remove_uuid(mam_sandbox): res = run_onboard(["--remove", orc_uuid], env=env, cwd=str(mam_sandbox)) assert res.returncode == 0 + # Issue #11: "removeed" was the naive ${MODE}ed conjugation. + assert "Successfully removed orchestrator UUID" in res.stdout, res.stdout res_list = run_onboard(["--list"], env=env, cwd=str(mam_sandbox)) assert orc_uuid not in res_list.stdout diff --git a/tests/test_tier1_unit.py b/tests/test_tier1_unit.py index f9b50a2..091cc47 100644 --- a/tests/test_tier1_unit.py +++ b/tests/test_tier1_unit.py @@ -493,6 +493,9 @@ def test_b9_logs_dir_follows_cwd_changes(mam_sandbox, tmp_path, monkeypatch): """B-9: the audit-log root must be resolved per call, not frozen at import.""" mq = get_mqtt_common(mam_sandbox) monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR", raising=False) + # Issue #15: with WORKSPACE_ROOT set the root is anchored on the workspace + # instead; the cwd only decides when neither override is present. + monkeypatch.delenv("WORKSPACE_ROOT", raising=False) a = tmp_path / "a"; b = tmp_path / "b" a.mkdir(); b.mkdir()