fix(id_order): keep an orphaned marker blocking while its herdr session is alive (Issue #63)
A create/resume killed after spawning its pane but before registering its row left a marker whose owner pid was dead; the next same-cwd waiter pruned it on the pid alone and could start a second session that discovers the same workspace conversation id (PR #69 review P1). - Markers record the herdr server. A dead owner's marker is pruned only once its row is registered for the attempt (running, newer epoch: the row decides) or `_herdr has-session` reports the session gone; an unanswered probe counts as alive. On timeout the kill-session command is printed. - A live marker of another process for the waiter's own session name is kept and blocks instead of being deleted; the waiter's own pid's markers go. - Tests: orphan / registered / gone / same-session / missing pane pid cases, and a create-level reproduction against mock herdr (fails before the fix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a907cc5294
commit
ede9c62be3
3 files changed
+216
-14
No files matched your search
@@ -25,7 +25,15 @@ UB = "b6300002-0000-4000-8000-000000000002"
|
||||
UC = "c6300003-0000-4000-8000-000000000003"
|
||||
UX = "d6300004-0000-4000-8000-000000000004"
|
||||
|
||||
pytestmark = pytest.mark.usefixtures("own_workspace_root")
|
||||
pytestmark = pytest.mark.usefixtures("own_workspace_root", "no_herdr_probe")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_herdr_probe(monkeypatch):
|
||||
"""In-process id_order.wait never asks the real herdr about a dead-owner
|
||||
marker's session: it reads as gone unless a test passes pane_alive."""
|
||||
from lib_py.agents import id_order
|
||||
monkeypatch.setattr(id_order, "herdr_session_alive", lambda server, session: False)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- helpers
|
||||
@@ -1034,3 +1042,134 @@ def test_id_order_resolved_but_unrecorded_resume_uuid_reserves_startup_until_rec
|
||||
proc_b.kill()
|
||||
proc_a.wait()
|
||||
proc_b.wait()
|
||||
|
||||
|
||||
def test_id_order_dead_owner_marker_blocks_while_its_unregistered_herdr_session_is_alive(
|
||||
tmp_path, monkeypatch, capsys
|
||||
):
|
||||
"""PR #69 review: a create/resume killed after spawning its pane but before
|
||||
registering its row leaves a dead-owner marker. It keeps blocking while the
|
||||
marker's herdr session is alive, goes once the row is registered (the row
|
||||
decides) or the session is gone; a live attempt on the same session name is
|
||||
not pruned; a running row without a pane pid blocks (fail-closed)."""
|
||||
from lib_py.agents import id_order
|
||||
|
||||
home = tmp_path / "home"
|
||||
ws = tmp_path / "ws"
|
||||
ws.mkdir(parents=True)
|
||||
yaml_path = tmp_path / "agent-sessions.yaml"
|
||||
yaml_path.write_text(yaml.safe_dump({"herdr_sessions": []}))
|
||||
monkeypatch.setenv("HOME", str(home))
|
||||
monkeypatch.setenv("AGENT_SESSIONS_YAML", str(yaml_path))
|
||||
dpath = id_order.order_dir(str(ws), "agy", home=str(home))
|
||||
marker_a = Path(dpath) / "orphan-a-agy.json"
|
||||
|
||||
alive_sessions = {"orphan-a-agy"}
|
||||
probes = []
|
||||
|
||||
def pane_alive(server, session):
|
||||
probes.append((server, session))
|
||||
return session in alive_sessions
|
||||
|
||||
def wait_b(**kw):
|
||||
return id_order.wait(str(ws), "agy", "new-b-agy", os.getpid(), yaml_path=str(yaml_path),
|
||||
home=str(home), wait_s_val=0, pane_alive=pane_alive, **kw)
|
||||
|
||||
dead = subprocess.Popen(["true"])
|
||||
dead.wait()
|
||||
live = subprocess.Popen(["sleep", "120"])
|
||||
try:
|
||||
# Orphan: owner dead, no row, herdr session alive -> blocks, marker kept, kill hint printed
|
||||
id_order.write_marker(dpath, "orphan-a-agy", dead.pid, str(ws), "agy", herdr_server="srv-x")
|
||||
capsys.readouterr()
|
||||
assert wait_b() == 1
|
||||
assert marker_a.exists()
|
||||
assert ("srv-x", "orphan-a-agy") in probes
|
||||
err = capsys.readouterr().err
|
||||
assert "HERDR_SESSION_NAME=srv-x herdr kill-session -t orphan-a-agy" in err
|
||||
|
||||
# Row registered for the attempt -> marker pruned without a probe; the row decides
|
||||
probes.clear()
|
||||
epoch = int(time.time())
|
||||
yaml_path.write_text(yaml.safe_dump({"herdr_sessions": [
|
||||
_row("orphan-a-agy", "agy", ws, epoch, pid=live.pid)]}))
|
||||
assert wait_b() == 1
|
||||
assert not marker_a.exists() and probes == []
|
||||
yaml_path.write_text(yaml.safe_dump({"herdr_sessions": [
|
||||
_row("orphan-a-agy", "agy", ws, epoch, pid=live.pid, agy_conversation_id_own=UA)]}))
|
||||
assert wait_b() == 0
|
||||
|
||||
# Session gone -> pruned, B admitted
|
||||
yaml_path.write_text(yaml.safe_dump({"herdr_sessions": []}))
|
||||
id_order.write_marker(dpath, "orphan-a-agy", dead.pid, str(ws), "agy", herdr_server="srv-x")
|
||||
alive_sessions.clear()
|
||||
assert wait_b() == 0
|
||||
assert not marker_a.exists()
|
||||
|
||||
# A live attempt on the same session name is kept and blocks; a dead one is taken over
|
||||
same = Path(dpath) / "same-agy.json"
|
||||
id_order.write_marker(dpath, "same-agy", live.pid, str(ws), "agy")
|
||||
assert id_order.wait(str(ws), "agy", "same-agy", os.getpid(), yaml_path=str(yaml_path),
|
||||
home=str(home), wait_s_val=0, pane_alive=pane_alive) == 1
|
||||
assert json.loads(same.read_text())["owner_pid"] == live.pid
|
||||
live.kill()
|
||||
live.wait()
|
||||
assert id_order.wait(str(ws), "agy", "same-agy", os.getpid(), yaml_path=str(yaml_path),
|
||||
home=str(home), wait_s_val=0, pane_alive=pane_alive) == 0
|
||||
assert json.loads(same.read_text())["owner_pid"] == os.getpid()
|
||||
same.unlink()
|
||||
|
||||
# Non-blocking review note: a running same-cwd row without a pane pid blocks
|
||||
no_pid = _row("no-pid-agy", "agy", ws, epoch)
|
||||
no_pid["pane"].pop("pid")
|
||||
yaml_path.write_text(yaml.safe_dump({"herdr_sessions": [no_pid]}))
|
||||
assert wait_b() == 1
|
||||
finally:
|
||||
live.kill()
|
||||
live.wait()
|
||||
|
||||
|
||||
def test_id_order_create_waits_for_orphan_herdr_session_of_a_killed_create(
|
||||
mam_sandbox, mock_herdr, mock_agents
|
||||
):
|
||||
"""PR #69 review reproduction: A's create spawned its pane and was killed
|
||||
before appending its row (dead-owner marker, live herdr agent, no row).
|
||||
create B refuses while A's herdr session is alive and proceeds once it is gone."""
|
||||
from lib_py.agents import id_order
|
||||
|
||||
ws = mam_sandbox / "orphan-ws"
|
||||
ws.mkdir(parents=True, exist_ok=True)
|
||||
name_a, name_b = "orphan-ws-creator-agy", "orphan-ws-reviewer-agy"
|
||||
create_sh = mam_sandbox / ".agents" / "skills" / "multi-agent-mux-create" / "scripts" / "create_session.sh"
|
||||
|
||||
dead = subprocess.Popen(["true"])
|
||||
dead.wait()
|
||||
holder = subprocess.Popen(["sleep", "120"])
|
||||
try:
|
||||
dpath = id_order.order_dir(str(ws), "agy", home=str(mam_sandbox))
|
||||
id_order.write_marker(dpath, name_a, dead.pid, str(ws), "agy", herdr_server="orphan-ws")
|
||||
_mock_herdr_agent(mock_herdr, name_a, ws, holder.pid, int(time.time()), pane_id="w1:p_a")
|
||||
|
||||
def create_b():
|
||||
return subprocess.run(
|
||||
["bash", str(create_sh), "--workspace", str(ws), "--agent", "agy",
|
||||
"--role", "reviewer", "--session", name_b, "--no-onboard"],
|
||||
capture_output=True, text=True, cwd=str(mam_sandbox),
|
||||
env=dict(os.environ, MAM_ID_ORDER_WAIT_S="0"), timeout=120,
|
||||
)
|
||||
|
||||
res = create_b()
|
||||
assert res.returncode == 1, res.stdout + res.stderr
|
||||
assert name_a in res.stderr and "kill-session -t " + name_a in res.stderr
|
||||
assert name_b not in _read_rows(mam_sandbox)
|
||||
assert name_b not in json.loads(Path(mock_herdr).read_text()).get("agents", {})
|
||||
|
||||
state = json.loads(Path(mock_herdr).read_text())
|
||||
state["agents"].pop(name_a)
|
||||
Path(mock_herdr).write_text(json.dumps(state))
|
||||
res = create_b()
|
||||
assert res.returncode == 0, res.stdout + res.stderr
|
||||
assert _read_rows(mam_sandbox)[name_b]["status"] == "running"
|
||||
finally:
|
||||
holder.kill()
|
||||
holder.wait()
|
||||
Reference in new issue
Block a user