From fae70c13d9a7fb9df50e1bfe759f39e35d3e1500 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 10:30:02 +0900 Subject: [PATCH] fix(issue-10,13): check every agy artifact for holders and purge orphaned leftovers Round-3 review of f5a6575 (planner-reviewer-codex-01, R1/R2). R1 - the adapter's in-use check handed only the conversation DB to lsof, so a holder that keeps the presence lock (or a sidecar) open while naming the uuid on no command line and in no session variable was invisible, and the held lock was unlinked from under it. AgyAgentAdapter.conversation_in_use now passes every path of the conversation (DB, -wal/-shm/-journal, brain, annotation, presence lock). R2 - both resolver stages required the DB, so once an interrupted or pre-#10 purge had removed it, the WAL/annotation/lock left behind were "no artifact" (exit 5) forever. BaseAgentAdapter.has_artifacts (default: a uuid-bearing artifact path that exists) replaces the resolver's path fallback, and agy's override accepts any of its pieces. Tests: a flock holder that names the uuid nowhere is refused and nothing is removed; orphaned leftovers resolve to agy and are purged. VERSIONS.md v4.4.3 items 1-2 and orc-onboard SKILL.md updated. Refs #10 #13 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib_py/agents/__main__.py | 10 ++-- .agents/skills/lib_py/agents/adapters/agy.py | 23 +++++++- .agents/skills/lib_py/agents/base.py | 8 +++ .../multi-agent-mux-orc-onboard/SKILL.md | 2 +- VERSIONS.md | 4 +- tests/test_issue10_13_purge.py | 52 +++++++++++++++++++ 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/.agents/skills/lib_py/agents/__main__.py b/.agents/skills/lib_py/agents/__main__.py index b244a1c..d437b75 100644 --- a/.agents/skills/lib_py/agents/__main__.py +++ b/.agents/skills/lib_py/agents/__main__.py @@ -13,8 +13,9 @@ def _resolve_purge_target(agent_hint: str, uuid: str, workspace: str): 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. + conversation is not in any workspace cache), then the adapter's own + `has_artifacts` - which, for agy, also accepts orphaned sidecars, the + annotation or the presence lock left behind after the DB is gone. """ from lib_py.agents.base import DiscoveryContext names = [agent_hint] if agent_hint and agent_hint != 'auto' else list(PURGE_PROBE_ORDER) @@ -32,11 +33,10 @@ def _resolve_purge_target(agent_hint: str, uuid: str, workspace: str): pass for name, adapter, ctx in probes: try: - path = adapter.artifact_path(uuid, ctx) + if adapter.has_artifacts(uuid, ctx): + return name, adapter, ctx except Exception: continue - if uuid in path and os.path.exists(path): - return name, adapter, ctx return None diff --git a/.agents/skills/lib_py/agents/adapters/agy.py b/.agents/skills/lib_py/agents/adapters/agy.py index c91b8da..ba20214 100644 --- a/.agents/skills/lib_py/agents/adapters/agy.py +++ b/.agents/skills/lib_py/agents/adapters/agy.py @@ -1,6 +1,6 @@ import os, sys, json, sqlite3, shutil from typing import Optional, Any -from lib_py.agents.base import BaseAgentAdapter, DiscoveryContext +from lib_py.agents.base import BaseAgentAdapter, DiscoveryContext, conversation_in_use class AgyAgentAdapter(BaseAgentAdapter): @property @@ -74,6 +74,27 @@ class AgyAgentAdapter(BaseAgentAdapter): def _cli_root(self, ctx: DiscoveryContext) -> str: return f"{ctx.home_dir}/.gemini/antigravity-cli" + def _artifact_paths(self, uuid: str, ctx: DiscoveryContext) -> list: + """Every on-disk piece of one conversation, DB first: the SQLite + sidecars, the brain dir, the title annotation and the presence lock.""" + db = self.artifact_path(uuid, ctx) + root = self._cli_root(ctx) + return ([db] + [f"{db}{suffix}" for suffix in ("-wal", "-shm", "-journal")] + + [f"{root}/brain/{uuid}", f"{root}/annotations/{uuid}.pbtxt", + f"{root}/presence/{uuid}.lock"]) + + def has_artifacts(self, uuid: str, ctx: DiscoveryContext) -> bool: + # Any piece counts, not only the DB: a purge that was interrupted (or + # the pre-#10 purge) can leave a WAL, annotation or lock behind with + # 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 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)) + 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 / diff --git a/.agents/skills/lib_py/agents/base.py b/.agents/skills/lib_py/agents/base.py index 681c496..41faa3a 100644 --- a/.agents/skills/lib_py/agents/base.py +++ b/.agents/skills/lib_py/agents/base.py @@ -191,6 +191,14 @@ class BaseAgentAdapter: on-disk tree override this; the default has nothing to scan.""" return [] + def has_artifacts(self, uuid: str, ctx: DiscoveryContext) -> bool: + """Anything of this conversation is on disk (a purge target exists + even when `verify_artifact` would not vouch for it). The default only + trusts an artifact path that embeds the uuid - a shared store (one DB + for every conversation) exists for any uuid and proves nothing.""" + path = self.artifact_path(uuid, ctx) + return uuid in path and os.path.exists(path) + 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)]) diff --git a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md index 9d309fe..5cf72d0 100644 --- a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md +++ b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md @@ -45,7 +45,7 @@ Registers an orchestrator session UUID into the `orchestrator_uuids` list of `.m # 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:`. +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:`. Orphaned leftovers — a WAL, annotation or lock whose DB is already gone — still resolve to agy and are purged, and the in-use check asks `lsof` about every one of those paths, so a holder that keeps only the presence lock open (naming the UUID nowhere) is refused too. ## Auto-Detection Hierarchy (Rev.2) diff --git a/VERSIONS.md b/VERSIONS.md index 5b48da6..898eca4 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -42,8 +42,8 @@ > **패치 마일스톤 (PATCH Release)**: agy 대화 purge 가 resume 가능한 부속 파일을 남기던 문제(Issue #10), herdr 세션 행이 없는 오케스트레이터 대화를 어떤 스킬로도 지울 수 없던 문제(Issue #13), `orc_onboard.sh --remove` 의 "removeed" 오타(Issue #11), 호출자 cwd 를 따라가던 잡 레지스트리·감사 로그 때문에 pytest 스위트가 실제 `.mam/jobs` 에 테스트 레코드를 남기던 문제(Issue #15)를 해결했다. 새 서브커맨드·옵션은 모두 추가 기능이다. 레지스트리·감사 로그의 **기본** 경로는 `WORKSPACE_ROOT` 가 있으면 cwd 대신 그 워크스페이스를 따르며, 명시적 `--registry-dir` 와 `MAM_JOBS_DIR`/`DELEGATE_JOB_LOGS_DIR` 는 그대로 우선한다. #### 🌟 주요 개선 사항 -1. **agy purge 부속 파일 완결 (Issue #10, `lib_py/agents/adapters/agy.py`)**: `purge_artifacts` 가 대화 DB·`brain/` 에 더해 SQLite 부속 파일(`-wal`/`-shm`/`-journal` — DB 를 열 때마다 생기고 stop 경로의 ID 캡처도 이를 만들며, 체크포인트되지 않은 WAL 에는 대화 내용이 남을 수 있다), 제목 annotation(`annotations/.pbtxt`), presence lock(`presence/.lock`)까지 지운다. presence lock 은 `conversation_in_use` 가 거짓일 때만 지우고, 사용 중이면 남긴 채 경고한다. 새 `residual_artifacts()` 는 `~/.gemini/antigravity-cli` 아래에 그 uuid 를 이름에 가진 경로가 남아 있으면 보고한다. db+brain 만 있는 경우 2건을 돌려주는 어댑터 계약은 유지. -2. **오케스트레이터 대화 purge CLI (Issue #13, `python -m lib_py.agents purge [workspace]`)**: `orchestrator_uuids` 는 uuid 만 저장하므로 디스크에서 어댑터를 판별한다. `auto` 는 `agy → claude → hermes → grok → codex → opencode` 순으로(opencode 는 모든 uuid 에 대해 존재하는 공유 DB 라 마지막) 각 어댑터의 `verify_artifact`(revalidate 모드 — 오케스트레이터 대화는 워크스페이스 캐시에 없음)를 먼저, 그다음 uuid 가 들어간 artifact 경로의 존재를 확인한다. 살아 있는 프로세스가 그 uuid 를 참조하면 거부한다(`base.conversation_in_use`: ① 호출 환경의 세션 id — `CLAUDE_CODE_SESSION_ID`·`ANTIGRAVITY_CONVERSATION_ID`·`HERMES_SESSION_ID`·`GROK_SESSION_ID`·`OPENCODE_SESSION_ID` — 가 그 uuid 이면 사용 중, 즉 오케스트레이터가 자기 세션 안에서 자기 대화를 purge 하는 경우를 막는다(claude 는 transcript 를 열어 두지 않아 아래 두 검사로는 보이지 않는다). ② 프로세스 명령줄 검사에서 purge 자신의 헬퍼 사슬(python·셸·스크립트)만 빼되 **가장 가까운 에이전트 CLI 조상에서 멈추고**, 그 조상의 명령줄에 uuid 가 있으면 보유자로 센다. ③ `lsof` 로 artifact 점유 확인). 결과는 JSON 한 줄(`agent`·`purged`·`residual`·`in_use`)이며 종료 코드는 0 purge 완료 / 4 사용 중이라 거부(아무것도 삭제하지 않음) / 5 해당 uuid 의 artifact 없음. +1. **agy purge 부속 파일 완결 (Issue #10, `lib_py/agents/adapters/agy.py`)**: `purge_artifacts` 가 대화 DB·`brain/` 에 더해 SQLite 부속 파일(`-wal`/`-shm`/`-journal` — DB 를 열 때마다 생기고 stop 경로의 ID 캡처도 이를 만들며, 체크포인트되지 않은 WAL 에는 대화 내용이 남을 수 있다), 제목 annotation(`annotations/.pbtxt`), presence lock(`presence/.lock`)까지 지운다. presence lock 은 `conversation_in_use` 가 거짓일 때만 지우고, 사용 중이면 남긴 채 경고한다. agy 의 `conversation_in_use` 는 DB 만이 아니라 부속 파일·annotation·presence lock 전부를 `lsof` 점유 검사에 넘기므로, uuid 를 어디에도 적지 않은 채 lock 만 열어 둔 보유자도 잡는다. DB 가 이미 사라지고 WAL·annotation·lock 만 남은 고아 상태도 `has_artifacts` 가 인식해 purge 대상이 된다. 새 `residual_artifacts()` 는 `~/.gemini/antigravity-cli` 아래에 그 uuid 를 이름에 가진 경로가 남아 있으면 보고한다. db+brain 만 있는 경우 2건을 돌려주는 어댑터 계약은 유지. +2. **오케스트레이터 대화 purge CLI (Issue #13, `python -m lib_py.agents purge [workspace]`)**: `orchestrator_uuids` 는 uuid 만 저장하므로 디스크에서 어댑터를 판별한다. `auto` 는 `agy → claude → hermes → grok → codex → opencode` 순으로(opencode 는 모든 uuid 에 대해 존재하는 공유 DB 라 마지막) 각 어댑터의 `verify_artifact`(revalidate 모드 — 오케스트레이터 대화는 워크스페이스 캐시에 없음)를 먼저, 그다음 uuid 가 들어간 artifact 경로의 존재를 확인한다. 살아 있는 프로세스가 그 uuid 를 참조하면 거부한다(`base.conversation_in_use`: ① 호출 환경의 세션 id — `CLAUDE_CODE_SESSION_ID`·`ANTIGRAVITY_CONVERSATION_ID`·`HERMES_SESSION_ID`·`GROK_SESSION_ID`·`OPENCODE_SESSION_ID` — 가 그 uuid 이면 사용 중, 즉 오케스트레이터가 자기 세션 안에서 자기 대화를 purge 하는 경우를 막는다(claude 는 transcript 를 열어 두지 않아 아래 두 검사로는 보이지 않는다). ② 프로세스 명령줄 검사에서 purge 자신의 헬퍼 사슬(python·셸·스크립트)만 빼되 **가장 가까운 에이전트 CLI 조상에서 멈추고**, 그 조상의 명령줄에 uuid 가 있으면 보유자로 센다. ③ `lsof` 로 artifact 점유 확인 — 어댑터가 넘긴 모든 경로(agy 는 DB·부속 파일·annotation·presence lock)). 결과는 JSON 한 줄(`agent`·`purged`·`residual`·`in_use`)이며 종료 코드는 0 purge 완료 / 4 사용 중이라 거부(아무것도 삭제하지 않음) / 5 해당 uuid 의 artifact 없음. 3. **`orc_onboard.sh --remove --purge-conversation --yes [--agent ]` (Issue #13)**: 위 purge 를 **레지스트리 갱신 전에** 실행하므로 거부되면 등록이 바뀌지 않는다. `--yes` 가 없으면 경고 후 exit 4, 대화가 사용 중이면 exit 5(등록 유지), 디스크에 대화가 없으면 경고 후 등록만 해제, `--dry-run` 은 예정 작업만 출력, `--purge-conversation` 은 `--remove` 와 함께일 때만 허용(exit 2). 성공 시 purge 된 경로와 잔여(residual) 검사 결과를 출력한다. 4. **`orc_onboard.sh` 성공 메시지 과거형 (Issue #11)**: `add`/`remove` 를 `added`/`removed` 로 명시 활용해 "Successfully removeed" 를 없앴다. 5. **잡 레지스트리·감사 로그의 워크스페이스 고정 (Issue #15, `mqtt_common.default_registry_dir`·`get_logs_dir`)**: 기본 경로가 `MAM_JOBS_DIR`/`DELEGATE_JOB_LOGS_DIR` → `$WORKSPACE_ROOT/.mam/jobs`·`$WORKSPACE_ROOT/.mam/delegate_job_logs` → cwd 기준 `.mam/…` 순으로 정해진다. `registry.py`·`publish_event.py`·`service.py` 와 `create_session.sh`·`stop_session.sh` 가 같은 순서를 쓴다. 저장소 루트에서 돈 pytest 스위트(기본 온보딩 잡을 등록하는 `create_session.sh`)가 호출자 cwd 를 따라 실제 `.mam/jobs` 에 테스트 레코드 593건을 남기던 문제를 해소했고, 샌드박스 픽스처(`tests/conftest.py`)는 두 환경변수를 모두 고정한다. diff --git a/tests/test_issue10_13_purge.py b/tests/test_issue10_13_purge.py index 5ab31e4..990606e 100644 --- a/tests/test_issue10_13_purge.py +++ b/tests/test_issue10_13_purge.py @@ -297,6 +297,58 @@ def test_purge_cli_unresolved_uuid(tmp_path, other): assert json.loads(res.stdout)["agent"] is None +def _lock_holder(lock: Path) -> subprocess.Popen: + """A process that holds the presence lock open (flock) but names the + conversation nowhere - not on its command line (the path travels through + the environment) and not in a session variable.""" + code = ("import fcntl, os, time; f = open(os.environ['MAM_TEST_LOCK'], 'a'); " + "fcntl.flock(f, fcntl.LOCK_EX); time.sleep(60)") + proc = subprocess.Popen([sys.executable, "-c", code], env=dict(os.environ, MAM_TEST_LOCK=str(lock))) + time.sleep(0.5) + return proc + + +def test_purge_cli_refuses_when_only_the_presence_lock_is_held_open(tmp_path, conv): + """codex R1: the in-use check must also ask lsof about the presence lock + and the sidecars, not just the DB - a holder can keep the lock open with + the uuid on no command line and in no session variable.""" + paths = _agy_tree(tmp_path, conv) + adapter = get_adapter("agy") + ctx = DiscoveryContext(workspace=str(tmp_path), agent_name="agy", home_dir=str(tmp_path)) + holder = _lock_holder(paths["lock"]) + try: + assert adapter.conversation_in_use(conv, ctx) is True + res = _purge_cli("agy", conv, 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" + assert adapter.conversation_in_use(conv, ctx) is False + + +def test_purge_cli_resolves_agy_from_orphaned_leftovers(tmp_path, conv): + """codex R2: after the DB is gone (an interrupted or pre-#10 purge) the + WAL, annotation and lock must still resolve to agy and be purged, instead + of 'no artifact' leaving them behind forever.""" + root = tmp_path / ".gemini" / "antigravity-cli" + leftovers = [root / "conversations" / f"{conv}.db-wal", + root / "annotations" / f"{conv}.pbtxt", + root / "presence" / f"{conv}.lock"] + for p in leftovers: + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("x") + + res = _purge_cli("auto", conv, tmp_path) + + assert res.returncode == 0, res.stdout + res.stderr + out = json.loads(res.stdout) + assert out["agent"] == "agy" + assert sorted(out["purged"]) == sorted(str(p) for p in leftovers) + assert out["residual"] == [] + assert not any(p.exists() for p in leftovers) + + # --------------------------------------------------------------------------- # #13 — orc_onboard.sh --remove --purge-conversation # ---------------------------------------------------------------------------