fix(opencode): a purge takes its child sessions' 1.x rows, the background service alone holds no session, XDG_DATA_HOME is followed (Issue #60, review)
- Review 614b6263 (codex, P2): `opencode session delete` removes the child
sessions with their parent, but their 1.x rows stayed and the parent's
residual was empty. The child sessions (session_v2 and a 1.x session
table, any depth) are taken before the delete; the 1.x tables (message,
part, todo, session_share, session) are cleaned for all of them, and the
residual also reports a child still on record (a failed delete).
- The in-use check counted any process holding the shared opencode.db; the
per-user 2.x background service (`serve --service`) holds it for every
session, so every opencode purge was refused while it ran (it does on
this Mac). It no longer counts; any other holder and a command line
naming the session still do.
- The data dir follows the caller's XDG_DATA_HOME when ctx is the caller's
own home (else <home>/.local/share), for debug paths, the fallback and the
residual scan (review 614b6263 / f4cedbae N4).
- resident_children = ('opencode',): the --standalone server stays the TUI's
idle child; what runs under it still counts (review 1abe0a90).
- Tests: child sessions on a migrated database, a failed delete, the service
holder, XDG, resident children, drift C pinning the parent over a newer
child session (review f4cedbae N2), and the real-binary test no longer
depends on PATH (N1).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
1 parent
6b4b96fa18
commit
cb5f439cce
4 files changed
+245
-41
No files matched your search
@@ -896,10 +896,10 @@ def _db(path: Path, ddl: list, rows: list):
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_opencode_residual_reports_event_rows(home, tmp_path, monkeypatch):
|
||||
def test_opencode_purge_leaves_no_rows(home, tmp_path, monkeypatch):
|
||||
# A database migrated from 1.x (Issue #60): `opencode session delete`
|
||||
# (the fake, as measured on 2.0.32) removes the 2.x rows, the purge
|
||||
# deletes the 1.x session/message/part rows, and todo is reported.
|
||||
# (the fake, as measured on 2.0.32) removes the 2.x rows and the purge
|
||||
# deletes the rows of the 1.x tables 2.x no longer writes.
|
||||
u, o = f"ses_{uuidlib.uuid4().hex}", f"ses_{uuidlib.uuid4().hex}"
|
||||
db = home / ".local" / "share" / "opencode" / "opencode.db"
|
||||
ddl = ["CREATE TABLE session (id TEXT PRIMARY KEY)",
|
||||
@@ -931,7 +931,7 @@ def test_opencode_residual_reports_event_rows(home, tmp_path, monkeypatch):
|
||||
assert adapter.purge_artifacts(u, ctx) == [f"opencode session (with its child sessions): {u}",
|
||||
f"sqlite rows for session: {u}"]
|
||||
|
||||
assert adapter.residual_artifacts(u, ctx) == [f"{db} (todo.session_id: 1 rows)"]
|
||||
assert adapter.residual_artifacts(u, ctx) == []
|
||||
assert len(adapter.residual_artifacts(o, ctx)) == 8 # the other session is untouched: one entry per table
|
||||
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import shutil
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -145,6 +146,122 @@ def test_a_failed_delete_is_warned_and_reported(oc, monkeypatch, capsys, broken)
|
||||
assert f"{adapter._resolve_db(ctx)} (session_v2.id: 1 rows)" in adapter.residual_artifacts("ses_main", ctx)
|
||||
|
||||
|
||||
def test_purge_takes_the_1x_rows_of_the_child_sessions_too(oc):
|
||||
"""A migrated database: the child sessions opencode deletes with their
|
||||
parent keep 1.x rows that only the purge removes (review 614b6263)."""
|
||||
adapter, ctx, db, ws, calls = oc
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.execute("CREATE TABLE session (id TEXT PRIMARY KEY, parent_id TEXT)")
|
||||
conn.execute("CREATE TABLE message (id TEXT PRIMARY KEY, session_id TEXT)")
|
||||
conn.execute("CREATE TABLE todo (id TEXT, session_id TEXT)")
|
||||
conn.commit()
|
||||
conn.close()
|
||||
tree = [("ses_main", None), ("ses_child", "ses_main"), ("ses_grand", "ses_child"), ("ses_other", None)]
|
||||
for sid, parent in tree:
|
||||
_add(db, sid, ws, 2_000_000, parent=parent)
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.execute("INSERT INTO session VALUES (?, ?)", (sid, parent))
|
||||
conn.execute("INSERT INTO message VALUES (?, ?)", (f"m-{sid}", sid))
|
||||
conn.execute("INSERT INTO todo VALUES ('t', ?)", (sid,))
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
assert any(r.endswith("[child session ses_grand]") for r in adapter.residual_artifacts("ses_main", ctx))
|
||||
purged = adapter.purge_artifacts("ses_main", ctx)
|
||||
assert purged == ["opencode session (with its child sessions): ses_main",
|
||||
"sqlite rows for session: ses_main and 2 child session(s)"]
|
||||
for sid in ("ses_main", "ses_child", "ses_grand"):
|
||||
assert adapter.residual_artifacts(sid, ctx) == [], sid
|
||||
# the unrelated session is untouched: session_v2, session_message, event_sequence, session, message, todo
|
||||
assert len(adapter.residual_artifacts("ses_other", ctx)) == 6
|
||||
|
||||
|
||||
def test_a_failed_delete_reports_the_child_sessions_left(oc, monkeypatch):
|
||||
adapter, ctx, db, ws, calls = oc
|
||||
_add(db, "ses_main", ws, 2_000_000)
|
||||
_add(db, "ses_child", ws, 2_000_000, parent="ses_main")
|
||||
monkeypatch.setattr(adapter, "_bin", lambda ctx: None)
|
||||
adapter.purge_artifacts("ses_main", ctx)
|
||||
residual = adapter.residual_artifacts("ses_main", ctx)
|
||||
assert f"{db} (session_v2.id: 1 rows) [child session ses_child]" in residual
|
||||
assert f"{db} (session_v2.id: 1 rows)" in residual
|
||||
|
||||
|
||||
def _holder(tmp_path, db, *argv):
|
||||
"""A live process holding `db` open, with `argv` on its command line."""
|
||||
ready = tmp_path / f"ready-{len(argv)}"
|
||||
code = ("import sys, time; f = open(sys.argv[1]); open(sys.argv[2], 'w').close(); time.sleep(60)")
|
||||
p = subprocess.Popen([sys.executable, "-c", code, str(db), str(ready), *argv])
|
||||
for _ in range(100):
|
||||
if ready.exists():
|
||||
break
|
||||
time.sleep(0.05)
|
||||
return p
|
||||
|
||||
|
||||
@pytest.mark.skipif(shutil.which("lsof") is None, reason="no lsof")
|
||||
def test_the_background_service_alone_does_not_hold_a_session(oc, tmp_path):
|
||||
"""The per-user service keeps the shared database open for every session
|
||||
(2.x); MAM runs none of its sessions there."""
|
||||
adapter, ctx, db, ws, calls = oc
|
||||
_add(db, "ses_main", ws, 2_000_000)
|
||||
assert adapter.conversation_in_use("ses_main", ctx) is False
|
||||
service = _holder(tmp_path, db, "serve", "--service")
|
||||
try:
|
||||
assert adapter.conversation_in_use("ses_main", ctx) is False
|
||||
other = _holder(tmp_path, db, "--auto", "--standalone") # another session's TUI or its server
|
||||
try:
|
||||
assert adapter.conversation_in_use("ses_main", ctx) is True
|
||||
finally:
|
||||
other.kill()
|
||||
other.wait()
|
||||
finally:
|
||||
service.kill()
|
||||
service.wait()
|
||||
named = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(60)", "--session", "ses_main"])
|
||||
try:
|
||||
time.sleep(0.2)
|
||||
assert adapter.conversation_in_use("ses_main", ctx) is True
|
||||
finally:
|
||||
named.kill()
|
||||
named.wait()
|
||||
|
||||
|
||||
def test_the_data_dir_follows_the_callers_xdg_data_home_only_for_its_own_home(tmp_path, monkeypatch):
|
||||
adapter = get_adapter("opencode")
|
||||
monkeypatch.setattr(adapter, "_bin", lambda ctx: None)
|
||||
own, other, xdg = tmp_path / "own", tmp_path / "other", tmp_path / "xdg"
|
||||
monkeypatch.setenv("HOME", str(own))
|
||||
monkeypatch.setenv("XDG_DATA_HOME", str(xdg))
|
||||
ws = str(tmp_path)
|
||||
assert adapter._resolve_db(DiscoveryContext(workspace=ws, agent_name="opencode", home_dir=str(own))) \
|
||||
== f"{xdg}/opencode/opencode.db"
|
||||
assert adapter._resolve_db(DiscoveryContext(workspace=ws, agent_name="opencode", home_dir=str(other))) \
|
||||
== f"{other}/.local/share/opencode/opencode.db"
|
||||
monkeypatch.delenv("XDG_DATA_HOME")
|
||||
assert adapter._resolve_db(DiscoveryContext(workspace=ws, agent_name="opencode", home_dir=str(own))) \
|
||||
== f"{own}/.local/share/opencode/opencode.db"
|
||||
|
||||
|
||||
def test_the_standalone_server_is_a_resident_child_but_its_tools_are_work():
|
||||
"""--standalone keeps `opencode serve --stdio` under the TUI while idle;
|
||||
the stall watcher skips it but still sees what runs under it."""
|
||||
from lib_py.stall_watch import background_children
|
||||
resident = get_adapter("opencode").resident_children
|
||||
tui = subprocess.Popen(["bash", "-c", "(exec -a opencode sleep 300) & exec sleep 300"], start_new_session=True)
|
||||
busy = subprocess.Popen(["bash", "-c", "(exec -a opencode bash -c 'sleep 300; :') & exec sleep 300"],
|
||||
start_new_session=True)
|
||||
try:
|
||||
time.sleep(0.5)
|
||||
assert background_children(tui.pid, ()) != []
|
||||
assert background_children(tui.pid, resident) == []
|
||||
assert background_children(busy.pid, resident) != []
|
||||
finally:
|
||||
for p in (tui, busy):
|
||||
os.killpg(p.pid, 9) # the whole group: children and grandchildren
|
||||
p.wait()
|
||||
|
||||
|
||||
def test_spawn_and_resume_never_pass_agent_and_run_standalone():
|
||||
"""The shell copies of the opencode command line match the adapter."""
|
||||
adapter = get_adapter("opencode")
|
||||
@@ -214,6 +331,9 @@ def test_real_opencode_2x_contract(tmp_path, monkeypatch):
|
||||
json.dumps({"title": "mam child", "parentID": parent})).stdout)["data"]["id"]
|
||||
|
||||
adapter = get_adapter("opencode")
|
||||
# The binary was found with the real HOME; under the test's HOME only a
|
||||
# PATH entry would find it (review f4cedbae N1).
|
||||
monkeypatch.setattr(adapter, "_bin", lambda ctx: REAL)
|
||||
ctx = DiscoveryContext(workspace=ws_real, agent_name="opencode", home_dir=str(home))
|
||||
db = adapter._resolve_db(ctx)
|
||||
assert db == str(home / ".local" / "share" / "opencode" / "opencode.db")
|
||||
|
||||
@@ -1249,6 +1249,17 @@ def test_comp_opencode_create_and_reconcile_and_stop(mam_sandbox, mock_herdr, mo
|
||||
assert s["opencode_session_id_own"] is None
|
||||
assert s["session_id_source"] == "pending-discovery"
|
||||
|
||||
# A subagent's session in the same directory, newer than the TUI's own
|
||||
# (opencode 2.x, Issue #60): drift C must pin the top-level session.
|
||||
import sqlite3
|
||||
odb = os.path.join(os.environ["HOME"], ".local", "share", "opencode", "opencode.db")
|
||||
conn = sqlite3.connect(odb)
|
||||
(parent, directory, created), = conn.execute("SELECT id, directory, time_created FROM session_v2").fetchall()
|
||||
conn.execute("INSERT INTO session_v2 (id, parent_id, directory, time_created) VALUES (?, ?, ?, ?)",
|
||||
(f"{parent}-child", parent, directory, created + 1000))
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
# Run reconcile.sh to trigger drift-C materialization
|
||||
reconcile_script = mam_sandbox / ".agents" / "skills" / "multi-agent-mux-monitor" / "scripts" / "reconcile.sh"
|
||||
res_rec = subprocess.run(["bash", str(reconcile_script), "--once", "--emit-diff"], capture_output=True, text=True, cwd=str(mam_sandbox), env={**os.environ, "WORKSPACE_ROOT": str(mam_sandbox)})
|
||||
@@ -1259,7 +1270,7 @@ def test_comp_opencode_create_and_reconcile_and_stop(mam_sandbox, mock_herdr, mo
|
||||
with open(yaml_path) as f:
|
||||
data_after = yaml.safe_load(f)
|
||||
s_after = [x for x in data_after.get("herdr_sessions", []) if x.get("name") == session_name][0]
|
||||
assert s_after.get("opencode_session_id_own") is not None
|
||||
assert s_after.get("opencode_session_id_own") == parent
|
||||
assert s_after.get("last_visible_status") in ("pinned", "resume_verified")
|
||||
|
||||
# Stop session
|
||||
|
||||
Reference in new issue
Block a user