diff --git a/.agents/skills/lib_py/stall_watch.py b/.agents/skills/lib_py/stall_watch.py index a311f84..a8255d9 100644 --- a/.agents/skills/lib_py/stall_watch.py +++ b/.agents/skills/lib_py/stall_watch.py @@ -38,6 +38,7 @@ from typing import List, Optional from lib_py.agents.input_region import extract_input_region from lib_py.agents.registry import agent_of_row, get_adapter +from lib_py.agents.sanitize import sanitize_herdr_agent_name from lib_py.delegation.engine import TERMINAL_STATUSES from lib_py.loop import watcher @@ -156,6 +157,9 @@ class HerdrSource(object): def __init__(self, target, agent, session=None): self.target, self.session = target, session or None + # herdr knows the agent only by its sanitized name (#66); the kind is + # read off the original name, whose suffix a long name's hash replaces. + self.name = sanitize_herdr_agent_name(target) self.agent = resolve_agent(agent, target) adapter = get_adapter(self.agent) if self.agent else None self.resident = tuple(adapter.resident_children) if adapter else () @@ -176,7 +180,7 @@ class HerdrSource(object): return proc.stdout def agent_get(self): - payload = json.loads(self._herdr("agent", "get", self.target) or "{}") + payload = json.loads(self._herdr("agent", "get", self.name) or "{}") return (payload.get("result") or {}).get("agent") or {} def agent_pid(self, pane_id): @@ -199,12 +203,12 @@ class HerdrSource(object): if not self.agent: return None try: - return extract_input_region(self._herdr("agent", "read", self.target), self.agent) == "" + return extract_input_region(self._herdr("agent", "read", self.name), self.agent) == "" except ValueError: return None def prompt(self, text): - self._herdr("agent", "prompt", self.target, text) + self._herdr("agent", "prompt", self.name, text) def collect(self, sig: Signals) -> Signals: """Fill the process and input signals of an idle agent.""" @@ -222,6 +226,7 @@ class StallWatch(object): def __init__(self, args, source=None): self.args = args self.src = source or HerdrSource(args.target, args.agent, args.herdr_session) + self.herdr_name = sanitize_herdr_agent_name(args.target) # for herdr only; logs keep args.target name = resolve_agent(args.agent, args.target) adapter = get_adapter(name) if name else None reliable = bool(adapter.agent_status_reliable) if adapter else True @@ -323,7 +328,7 @@ class StallWatch(object): until, tmo = ("idle", "done", "working"), 60000 else: until, tmo = ("idle", "done", "blocked"), 60000 - out = watcher.wait_once(self.args.target, until=until, timeout_ms=tmo, session=self.args.herdr_session) + out = watcher.wait_once(self.herdr_name, until=until, timeout_ms=tmo, session=self.args.herdr_session) if self._wrapper_gone(): # round-2 review D2: before any notice return RC_TERMINAL if out.kind in (watcher.TRANSPORT_ERROR, watcher.AGENT_NOT_FOUND): @@ -425,7 +430,7 @@ def main(argv=None) -> int: p = argparse.ArgumentParser(description="notify-only stall watcher for one local herdr job (#30)") p.add_argument("--job", required=True) p.add_argument("--registry-dir", required=True) - p.add_argument("--target", required=True, help="herdr agent name of the job's session") + p.add_argument("--target", required=True, help="session name of the job (herdr is called with its sanitized form)") p.add_argument("--agent", required=True, help="agent CLI (adapter name)") p.add_argument("--herdr-session", default="") p.add_argument("--expect-dispatch-token", default=None) diff --git a/tests/conftest.py b/tests/conftest.py index 8aba27d..788dfe6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -470,6 +470,9 @@ def _sanitize_agent_name(name): def _match_agent(k, target): if not k or not target: return False + # Issue #66: like the real herdr, answer only the registered (sanitized) name. + if state.get("strict_agent_names"): + return k == target return k == target or _sanitize_agent_name(k) == target or _sanitize_agent_name(target) == k or _sanitize_agent_name(k) == _sanitize_agent_name(target) args = sys.argv[1:] @@ -1052,6 +1055,10 @@ elif cmd1 == "agent": })) sys.exit(0) else: + if state.get("strict_agent_names"): + sys.stderr.write(json.dumps({"error": {"code": "agent_not_found", + "message": "agent " + name + " not found"}}) + "\\n") + sys.exit(1) sys.stderr.write("Agent " + name + " not found\\n") sys.exit(1) elif cmd2 == "wait": @@ -1143,6 +1150,10 @@ elif cmd1 == "agent": print(buffer_content) sys.exit(0) else: + if state.get("strict_agent_names"): + sys.stderr.write(json.dumps({"error": {"code": "agent_not_found", + "message": "agent " + name + " not found"}}) + "\\n") + sys.exit(1) sys.stderr.write("Agent " + name + " not found\\n") sys.exit(1) elif cmd2 == "prompt": diff --git a/tests/test_issue66_stall_watch_sanitized_name.py b/tests/test_issue66_stall_watch_sanitized_name.py new file mode 100644 index 0000000..65e24ea --- /dev/null +++ b/tests/test_issue66_stall_watch_sanitized_name.py @@ -0,0 +1,129 @@ +"""Issue #66: stall_watch calls herdr with the sanitized agent name. + +herdr registers an agent under `sanitize_herdr_agent_name(session)` (lower +case, at most 32 characters). The watcher gets the original session name from +the wrapper, so it must convert it for every herdr call while its logs keep +the original name and the agent kind is still read off the original suffix. + +The harness is the one of test_issue30_stall_watch (a real watcher process, a +real job, the conftest mock herdr), with the mock switched to +`strict_agent_names`: like the real herdr it answers an agent only under its +registered (sanitized) name, so the original name gets `agent_not_found`. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import test_issue30_stall_watch as t30 # noqa: E402 + +from lib_py.agents.sanitize import sanitize_herdr_agent_name # noqa: E402 + +LONG = "creator-claude-fix-issue-99-long-branch" # 39 characters +UPPER = "Creator-Claude-T" +PLAIN = "creator-claude-t" # needs no conversion +LONG_CODEX = "mam-wt-issue-66-very-long-creator-codex" # kind only in the original suffix + +DIALOG = "\n Dangerous rm operation detected. Do you want to proceed?\n 1. Yes\n" + + +@pytest.fixture +def strict_box(mam_sandbox, mock_herdr): + state = json.loads(mock_herdr.read_text()) + state["strict_agent_names"] = True + mock_herdr.write_text(json.dumps(state, indent=2)) + reg = mam_sandbox / ".mam" / "jobs" + return mock_herdr, reg, t30._env(mam_sandbox, MAM_STALL_IDLE_S=2) + + +def _agent_calls(state_file: Path) -> list: + return [c for c in json.loads(state_file.read_text())["calls"] + if c[:1] == ["agent"] and c[1:2] in (["get"], ["read"], ["prompt"], ["wait"])] + + +def test_the_names_change_and_the_strict_mock_answers_only_the_sanitized_one(strict_box): + state, _, env = strict_box + assert len(LONG) > 32 and sanitize_herdr_agent_name(LONG) != LONG + assert sanitize_herdr_agent_name(UPPER) == PLAIN + assert sanitize_herdr_agent_name(PLAIN) == PLAIN + for name in (LONG, UPPER): + herdr_name = sanitize_herdr_agent_name(name) + t30._agent(state, herdr_name, "claude", 4242, "idle", "Ready") + for cmd in (["get"], ["read"], ["wait", "--until", "idle"], ["prompt"]): + argv = ["herdr", "agent", cmd[0], name] + cmd[1:] + (["hi"] if cmd == ["prompt"] else []) + res = subprocess.run(argv, capture_output=True, text=True, env=env) + assert res.returncode == 1, (argv, res.stdout) + assert json.loads(res.stderr)["error"]["code"] == "agent_not_found", argv + argv[3] = herdr_name + assert subprocess.run(argv, capture_output=True, text=True, env=env).returncode == 0, argv + + +@pytest.mark.parametrize("target", [LONG, UPPER, PLAIN]) +def test_an_idle_agent_is_nudged_through_the_sanitized_name(strict_box, target): + state, reg, env = strict_box + herdr_name = sanitize_herdr_agent_name(target) + jid = t30._job(env, reg) + agent = t30._proc("exec sleep 300") + try: + t30._agent(state, herdr_name, "claude", agent.pid, "idle", t30._pane("claude")) + w = t30._watch(env, reg, jid, target, "claude") + assert t30._until(lambda: t30._prompts(state)) + time.sleep(2) + assert t30._details(reg, jid) == ["stalled:idle"] + calls = _agent_calls(state) + assert {c[1] for c in calls} == {"get", "read", "prompt", "wait"}, calls + assert all(c[2] == herdr_name for c in calls), calls + assert len(t30._prompts(state)) == 1 + out = t30._finish(env, reg, jid, w) + assert f"agent {target} idle for" in out + finally: + t30._kill(agent) + + +@pytest.mark.parametrize("target", [LONG, UPPER]) +def test_a_blocked_agent_is_reported_with_the_original_name(strict_box, target): + state, reg, env = strict_box + herdr_name = sanitize_herdr_agent_name(target) + jid = t30._job(env, reg) + agent = t30._proc("exec sleep 300") + try: + t30._agent(state, herdr_name, "claude", agent.pid, "blocked", t30._pane("claude"), detection=DIALOG) + w = t30._watch(env, reg, jid, target, "claude") + assert t30._until(lambda: t30._details(reg, jid)) + time.sleep(2) + assert t30._details(reg, jid) == ["stalled:blocked Dangerous rm operation detected. Do you want to proceed?"] + calls = _agent_calls(state) + assert calls and all(c[2] == herdr_name for c in calls), calls + assert not t30._prompts(state) + out = t30._finish(env, reg, jid, w) + assert f"agent {target} is blocked" in out + finally: + t30._kill(agent) + + +def test_the_kind_of_a_long_name_comes_from_its_original_suffix(strict_box): + """No usable --agent: the kind comes from the session name. Read off the + sanitized name (a hash at the end) it would be unknown, and the nudge would + be withheld as `input_check_unavailable`.""" + state, reg, env = strict_box + herdr_name = sanitize_herdr_agent_name(LONG_CODEX) + assert not herdr_name.endswith("-codex") + jid = t30._job(env, reg) + agent = t30._proc("exec sleep 300") + try: + t30._agent(state, herdr_name, "codex", agent.pid, "idle", t30._pane("codex")) + w = t30._watch(env, reg, jid, LONG_CODEX, "") + assert t30._until(lambda: t30._prompts(state)) + time.sleep(2) + assert t30._details(reg, jid) == ["stalled:idle"] + assert all(c[2] == herdr_name for c in _agent_calls(state)) + t30._finish(env, reg, jid, w) + finally: + t30._kill(agent)