test: review round 1 of Issue #71 (names, row-less stop server, migration retry)
- create with a non-ASCII name: the attach target is the name herdr has;
bash/Python parity for non-ASCII and option-like names; no python3 is
an error for a non-ASCII name.
- row-less stop without --herdr-session uses HERDR_SESSION_NAME; an
unreadable row is not taken for no row.
- a retry (entry point, or any write) repairs a YAML a failed migration
left in format 1.
- tier3: the "cannot infer agent" case registers its row, as tier1 does.
- S11: base re-pinned to 5cd72d2, which changes the shim's sanitize body.
Refs #71
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9410913da9
commit
a6fb39ba9f
3 files changed
+97
-6
No files matched your search
@@ -303,7 +303,7 @@ def test_s11_this_branch_does_not_change_the_generated_shim(mam_sandbox):
|
|||||||
too (one sanitize definition written in with `declare -f`, agent attach
|
too (one sanitize definition written in with `declare -f`, agent attach
|
||||||
resolved, no silent wrong name), so the base is that commit."""
|
resolved, no silent wrong name), so the base is that commit."""
|
||||||
repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||||
base = os.environ.get("MAM_BASE_SHA", "4243da5")
|
base = os.environ.get("MAM_BASE_SHA", "5cd72d2")
|
||||||
show = subprocess.run(["git", "show", "%s:.agents/skills/lib.sh" % base],
|
show = subprocess.run(["git", "show", "%s:.agents/skills/lib.sh" % base],
|
||||||
capture_output=True, text=True, cwd=repo)
|
capture_output=True, text=True, cwd=repo)
|
||||||
if show.returncode != 0:
|
if show.returncode != 0:
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ from lib_py.agents.sanitize import sanitize_herdr_agent_name # noqa: E402
|
|||||||
LONG = "lab-issue71-a-very-long-branch-name-creator-claude" # > 32 characters
|
LONG = "lab-issue71-a-very-long-branch-name-creator-claude" # > 32 characters
|
||||||
UPPER = "Lab-Upper-creator-claude"
|
UPPER = "Lab-Upper-creator-claude"
|
||||||
PLAIN = "lab-plain-creator-claude" # needs no conversion
|
PLAIN = "lab-plain-creator-claude" # needs no conversion
|
||||||
|
UNI = "\u0130stanbul-creator-claude" # str.lower() differs from tr here
|
||||||
STOP = "bash .agents/skills/multi-agent-mux-stop/scripts/stop_session.sh"
|
STOP = "bash .agents/skills/multi-agent-mux-stop/scripts/stop_session.sh"
|
||||||
|
|
||||||
|
|
||||||
@@ -77,7 +78,7 @@ def _assert_new_form(row, server):
|
|||||||
assert "kill_command" not in row and "start_command" not in row
|
assert "kill_command" not in row and "start_command" not in row
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("name", [LONG, UPPER, PLAIN])
|
@pytest.mark.parametrize("name", [LONG, UPPER, PLAIN, UNI])
|
||||||
@pytest.mark.parametrize("server", ["default", "mam-iso71"])
|
@pytest.mark.parametrize("server", ["default", "mam-iso71"])
|
||||||
def test_create_writes_the_attach_command(mam_sandbox, mock_herdr, mock_agents, name, server):
|
def test_create_writes_the_attach_command(mam_sandbox, mock_herdr, mock_agents, name, server):
|
||||||
_set_mock(mock_herdr, strict_agent_names=True)
|
_set_mock(mock_herdr, strict_agent_names=True)
|
||||||
@@ -160,7 +161,9 @@ def test_wrapper_attach_calls_herdr_with_the_herdr_name(mam_sandbox, mock_herdr,
|
|||||||
# --- one sanitize rule ----------------------------------------------------------
|
# --- one sanitize rule ----------------------------------------------------------
|
||||||
|
|
||||||
NAMES = ["", "plain", "Upper-Case", "9lead", "-dash", "has space/and.dot", "x" * 32, "y" * 33,
|
NAMES = ["", "plain", "Upper-Case", "9lead", "-dash", "has space/and.dot", "x" * 32, "y" * 33,
|
||||||
LONG, LONG.upper(), "creator-claude-fix-issue-99-long-branch"]
|
LONG, LONG.upper(), "creator-claude-fix-issue-99-long-branch", "-n", "-e", "a\\nb"]
|
||||||
|
UNI_NAMES = [UNI, "caf\u00e9-creator-claude", "\u00fc" * 40, "kelvin-\u212a", "\u65e5\u672c-creator-claude",
|
||||||
|
"\u0130" * 20 + "-creator-claude"]
|
||||||
|
|
||||||
|
|
||||||
def test_lib_sh_has_one_definition():
|
def test_lib_sh_has_one_definition():
|
||||||
@@ -216,6 +219,26 @@ def test_each_hasher_alone_matches_python(tmp_path, hashers):
|
|||||||
assert res.stdout == sanitize_herdr_agent_name(name) + "\n", name
|
assert res.stdout == sanitize_herdr_agent_name(name) + "\n", name
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("hashers", [["python3"], ["shasum", "python3"]])
|
||||||
|
def test_a_name_that_is_not_ascii_matches_python(tmp_path, hashers):
|
||||||
|
if not shutil.which(hashers[0]):
|
||||||
|
pytest.skip(hashers[0] + " not installed")
|
||||||
|
defn = _definition(tmp_path)
|
||||||
|
d = _tools_dir(tmp_path, ["tr", "sed", "awk", "cut"] + hashers)
|
||||||
|
for name in UNI_NAMES:
|
||||||
|
res = _sanitize_with_path(defn, d, name)
|
||||||
|
assert res.returncode == 0, (name, res.stderr)
|
||||||
|
assert res.stdout == sanitize_herdr_agent_name(name) + "\n", name
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_name_that_is_not_ascii_needs_python3(tmp_path):
|
||||||
|
defn = _definition(tmp_path)
|
||||||
|
d = _tools_dir(tmp_path, ["tr", "sed", "awk", "cut"])
|
||||||
|
for name in UNI_NAMES:
|
||||||
|
res = _sanitize_with_path(defn, d, name)
|
||||||
|
assert res.returncode != 0 and res.stdout == "" and "not ASCII" in res.stderr, (name, res.stderr)
|
||||||
|
|
||||||
|
|
||||||
def test_no_hasher_at_all_is_an_error_not_a_name(tmp_path):
|
def test_no_hasher_at_all_is_an_error_not_a_name(tmp_path):
|
||||||
defn = _definition(tmp_path)
|
defn = _definition(tmp_path)
|
||||||
d = _tools_dir(tmp_path, ["tr", "sed", "awk", "cut"])
|
d = _tools_dir(tmp_path, ["tr", "sed", "awk", "cut"])
|
||||||
@@ -351,6 +374,41 @@ def test_any_write_migrates_and_rewrites_the_yaml(own_workspace_root, tmp_path):
|
|||||||
_assert_new_form(s, s["herdr_session"])
|
_assert_new_form(s, s["herdr_session"])
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("retry", ["entry-point", "any-write"])
|
||||||
|
def test_a_retry_repairs_the_yaml_a_failed_migration_left_behind(own_workspace_root, tmp_path, monkeypatch, retry):
|
||||||
|
"""The DB is committed before the YAML is replaced; if that fails, the next run rewrites the YAML."""
|
||||||
|
from lib_py import registry_format
|
||||||
|
ws = tmp_path / "ws"
|
||||||
|
yaml_path = _write_old_registry(ws, with_db=True)
|
||||||
|
real_replace, failed = os.replace, []
|
||||||
|
|
||||||
|
def replace_once(src, dst, *a, **kw):
|
||||||
|
if os.path.abspath(str(dst)) == str(yaml_path) and not failed:
|
||||||
|
failed.append(dst)
|
||||||
|
raise OSError("injected: YAML not replaced")
|
||||||
|
return real_replace(src, dst, *a, **kw)
|
||||||
|
|
||||||
|
monkeypatch.setenv("YAML_PATH", "")
|
||||||
|
monkeypatch.setenv("AGENT_SESSIONS_MUTATION", "")
|
||||||
|
monkeypatch.setattr(os, "replace", replace_once)
|
||||||
|
assert registry_format.main(["--yaml", str(yaml_path)]) == 1 and failed
|
||||||
|
monkeypatch.setattr(os, "replace", real_replace)
|
||||||
|
assert _state(ws)["registry_format"] == 2
|
||||||
|
assert registry_format.stored_version(yaml.safe_load(yaml_path.read_text())) == 1
|
||||||
|
|
||||||
|
if retry == "entry-point":
|
||||||
|
res = _entry(yaml_path)
|
||||||
|
else:
|
||||||
|
res = subprocess.run(["bash", "-c", 'source "$1" && atomic_dump_yaml "$2"', "_", str(SKILLS / "lib.sh"), str(yaml_path)],
|
||||||
|
input="pass\n", capture_output=True, text=True, timeout=60)
|
||||||
|
assert res.returncode == 0, res.stderr
|
||||||
|
y = yaml.safe_load(yaml_path.read_text())
|
||||||
|
assert y["registry_format"] == 2
|
||||||
|
assert {s["name"]: s for s in y["herdr_sessions"]} == _db_rows(ws), "YAML and DB agree"
|
||||||
|
for s in y["herdr_sessions"]:
|
||||||
|
_assert_new_form(s, s["herdr_session"])
|
||||||
|
|
||||||
|
|
||||||
def test_a_new_registry_is_written_current(own_workspace_root, tmp_path):
|
def test_a_new_registry_is_written_current(own_workspace_root, tmp_path):
|
||||||
yaml_path = tmp_path / "fresh" / ".mam" / "agent-sessions.yaml"
|
yaml_path = tmp_path / "fresh" / ".mam" / "agent-sessions.yaml"
|
||||||
yaml_path.parent.mkdir(parents=True)
|
yaml_path.parent.mkdir(parents=True)
|
||||||
@@ -375,10 +433,12 @@ def _registry_bytes(sandbox):
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
def _stop(sandbox, name, *extra):
|
def _stop(sandbox, name, *extra, server="default", env=None):
|
||||||
script = sandbox / ".agents" / "skills" / "multi-agent-mux-stop" / "scripts" / "stop_session.sh"
|
script = sandbox / ".agents" / "skills" / "multi-agent-mux-stop" / "scripts" / "stop_session.sh"
|
||||||
return subprocess.run(["bash", str(script), "--session", name, "--herdr-session", "default"] + list(extra),
|
flag = ["--herdr-session", server] if server else []
|
||||||
capture_output=True, text=True, cwd=str(sandbox), timeout=120)
|
return subprocess.run(["bash", str(script), "--session", name] + flag + list(extra),
|
||||||
|
capture_output=True, text=True, cwd=str(sandbox), timeout=120,
|
||||||
|
env=dict(os.environ, **(env or {})))
|
||||||
|
|
||||||
|
|
||||||
def _agent(cwd, pane):
|
def _agent(cwd, pane):
|
||||||
@@ -422,6 +482,33 @@ def test_stop_reports_a_rowless_session_that_is_not_there(mam_sandbox, mock_herd
|
|||||||
assert _registry_bytes(mam_sandbox) == registry_with_a_row
|
assert _registry_bytes(mam_sandbox) == registry_with_a_row
|
||||||
|
|
||||||
|
|
||||||
|
def test_stop_without_a_row_uses_the_environments_server(mam_sandbox, mock_herdr, registry_with_a_row):
|
||||||
|
"""No --herdr-session: the server is HERDR_SESSION_NAME, as create chose it, not one derived from the workspace."""
|
||||||
|
_set_mock(mock_herdr, agents={"orphan73": _agent(mam_sandbox, "w1:p74")}, calls=[])
|
||||||
|
res = _stop(mam_sandbox, "orphan73", server=None, env={"HERDR_SESSION_NAME": "wanted-env-server"})
|
||||||
|
assert res.returncode == 0, res.stdout + res.stderr
|
||||||
|
assert "wanted-env-server" in res.stdout
|
||||||
|
calls = json.loads(Path(mock_herdr).read_text())["calls"]
|
||||||
|
# every call on an agent or pane names that server (`session list` and `config check` name none)
|
||||||
|
targeted = [c for c in calls if "agent" in c[:3] or "pane" in c[:3]]
|
||||||
|
assert targeted and all(c[:2] == ["--session", "wanted-env-server"] for c in targeted), calls
|
||||||
|
assert ["--session", "wanted-env-server", "agent", "get", "orphan73"] in calls
|
||||||
|
assert _registry_bytes(mam_sandbox) == registry_with_a_row
|
||||||
|
|
||||||
|
|
||||||
|
def test_stop_does_not_take_an_unreadable_row_for_no_row(mam_sandbox, mock_herdr):
|
||||||
|
"""Only "no such row" leads to the row-less path; a lookup that fails otherwise stops."""
|
||||||
|
db = mam_sandbox / ".mam" / "agent-sessions.db"
|
||||||
|
if db.exists():
|
||||||
|
db.unlink()
|
||||||
|
(mam_sandbox / ".mam" / "agent-sessions.yaml").write_text("herdr_sessions:\n- 42\n")
|
||||||
|
_set_mock(mock_herdr, agents={"orphan74": _agent(mam_sandbox, "w1:p75")})
|
||||||
|
res = _stop(mam_sandbox, "orphan74")
|
||||||
|
assert res.returncode == 1, res.stdout + res.stderr
|
||||||
|
assert "cannot read the row" in res.stderr
|
||||||
|
assert "orphan74" in json.loads(Path(mock_herdr).read_text())["agents"]
|
||||||
|
|
||||||
|
|
||||||
def test_stop_without_a_row_purges_nothing(mam_sandbox, mock_herdr, registry_with_a_row):
|
def test_stop_without_a_row_purges_nothing(mam_sandbox, mock_herdr, registry_with_a_row):
|
||||||
_set_mock(mock_herdr, agents={"orphan72": _agent(mam_sandbox, "w1:p73")})
|
_set_mock(mock_herdr, agents={"orphan72": _agent(mam_sandbox, "w1:p73")})
|
||||||
res = _stop(mam_sandbox, "orphan72", "--purge-conversation", "--yes")
|
res = _stop(mam_sandbox, "orphan72", "--purge-conversation", "--yes")
|
||||||
|
|||||||
@@ -393,6 +393,10 @@ def test_integration_invalid_arguments_exit_statuses(mam_sandbox, mock_herdr, mo
|
|||||||
assert "ERROR: --session required" in res.stderr
|
assert "ERROR: --session required" in res.stderr
|
||||||
|
|
||||||
# 3. stop_session.sh cannot infer agent from weird session name
|
# 3. stop_session.sh cannot infer agent from weird session name
|
||||||
|
# (Issue #71: a session with no row needs no agent kind, so the row is
|
||||||
|
# registered here, with no agent, suffix or pane.cmd)
|
||||||
|
(tmp_path / ".mam" / "agent-sessions.yaml").write_text(
|
||||||
|
"herdr_sessions:\n- name: bad-name\n status: running\n pane: {}\n")
|
||||||
res = subprocess.run(["bash", str(stop_script), "--session", "bad-name"], capture_output=True, text=True, cwd=str(tmp_path))
|
res = subprocess.run(["bash", str(stop_script), "--session", "bad-name"], capture_output=True, text=True, cwd=str(tmp_path))
|
||||||
assert res.returncode == 2
|
assert res.returncode == 2
|
||||||
assert "ERROR: cannot infer agent" in res.stderr
|
assert "ERROR: cannot infer agent" in res.stderr
|
||||||
|
|||||||
Reference in new issue
Block a user