From a640b0f072c038c2103407abe8089f6170b33d85 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:32:35 +0000 Subject: [PATCH 01/14] feat(issue-45): herdr config for MAM servers with restore-on-restart off lib_py/herdr_config.py builds the user's herdr config (HERDR_CONFIG_PATH, else XDG_CONFIG_HOME, else ~/.config, as herdr reads it) with only [session] resume_agents_on_restore forced to false, written next to the source so relative paths resolve the same. The text merge is re-parsed; a merge that does not leave the rest equal falls back to the key alone, since herdr drops a config that does not parse and would restore again. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/herdr_config.py | 196 ++++++++++++++++++++++++++ tests/test_issue45_herdr_config.py | 192 +++++++++++++++++++++++++ 2 files changed, 388 insertions(+) create mode 100644 .agents/skills/lib_py/herdr_config.py create mode 100644 tests/test_issue45_herdr_config.py diff --git a/.agents/skills/lib_py/herdr_config.py b/.agents/skills/lib_py/herdr_config.py new file mode 100644 index 0000000..69f1500 --- /dev/null +++ b/.agents/skills/lib_py/herdr_config.py @@ -0,0 +1,196 @@ +"""Issue #45: the config file MAM's isolated herdr servers start with. + +herdr restores a named session's saved layout when its server starts and, by +default, relaunches each saved agent with herdr's own resume command - without +the permission flags MAM adds, and even for agents MAM deleted while the server +was down. `[session] resume_agents_on_restore = false` turns that off. The +shim's headless bootstrap (lib.sh) hands the server `HERDR_CONFIG_PATH` pointing +at the file built here: the user's own herdr config with only that key forced +to false. The user's file is never modified. + + * Source: the file herdr itself would read - HERDR_CONFIG_PATH, else + $XDG_CONFIG_HOME/herdr/config.toml, else ~/.config/herdr/config.toml. A + HERDR_CONFIG_PATH naming a file generated here (every MAM pane inherits it + from the server) is followed back to the source recorded in that file. + * Output: `mam-server-config.toml` next to the source, so relative paths in + it (herdr resolves them against the config file's directory) keep their + meaning; `<.mam>/herdr-server-config.toml` only when that directory is not + writable. + * The merge edits text (stdlib has no TOML writer) and is then re-parsed: the + key must read false and everything else must equal the source. A merge that + fails the check falls back to a file holding only the key - herdr would + drop a broken file whole and restore again, silently. + +CLI: `python3 -m lib_py.herdr_config server-config [--fallback-dir DIR]` writes +the file and prints its path; warnings go to stderr; exit 1 when no file could +be written. +""" +from __future__ import annotations + +import argparse +import os +import re +import sys +import tempfile +import tomllib +from typing import List, Mapping, Optional, Tuple + +MARKER = "# generated by multi-agent-mux (Issue #45) - do not edit" +SOURCE_PREFIX = "# source: " +OUTPUT_NAME = "mam-server-config.toml" +FALLBACK_NAME = "herdr-server-config.toml" +KEY = "resume_agents_on_restore" +MINIMAL = f"[session]\n{KEY} = false\n" + +_SESSION_HEADER = re.compile(r"^\s*\[\s*session\s*\]\s*(#.*)?$") +_ANY_HEADER = re.compile(r"^\s*\[") +_KEY_LINE = re.compile(rf"^\s*{KEY}\s*=") + + +def _generated_source(path: str) -> Optional[str]: + """The source recorded in a file generated here; '' when it is one without + a source line; None when it is not a generated file.""" + try: + with open(path, encoding="utf-8") as f: + if f.readline().rstrip("\n") != MARKER: + return None + line = f.readline().rstrip("\n") + except OSError: + return None + return line[len(SOURCE_PREFIX):] if line.startswith(SOURCE_PREFIX) else "" + + +def user_config_path(env: Mapping[str, str]) -> str: + """The config file herdr reads under `env`, seen through generated files.""" + explicit = env.get("HERDR_CONFIG_PATH", "") + if explicit: + source = _generated_source(explicit) + if source is None: + return explicit + if source: + return source + xdg = env.get("XDG_CONFIG_HOME", "") + if xdg: + # herdr takes a relative XDG_CONFIG_HOME against its cwd, too. + return os.path.join(os.path.abspath(xdg), "herdr", "config.toml") + return os.path.join(env.get("HOME", os.path.expanduser("~")), ".config", "herdr", "config.toml") + + +def _without_key(d: dict) -> dict: + d = dict(d) + session = dict(d.get("session") or {}) + session.pop(KEY, None) + if session: + d["session"] = session + else: + d.pop("session", None) + return d + + +def _merge_text(user_text: str) -> str: + lines = user_text.splitlines() + header = next((i for i, l in enumerate(lines) if _SESSION_HEADER.match(l)), None) + if header is None: + sep = "" if user_text.endswith("\n") else "\n" + return f"{user_text}{sep}\n{MINIMAL}" + end = next((i for i in range(header + 1, len(lines)) if _ANY_HEADER.match(lines[i])), len(lines)) + key_line = next((i for i in range(header + 1, end) if _KEY_LINE.match(lines[i])), None) + if key_line is None: + lines.insert(header + 1, f"{KEY} = false") + else: + lines[key_line] = f"{KEY} = false" + return "\n".join(lines) + "\n" + + +def server_config_text(user_text: Optional[str]) -> Tuple[str, List[str]]: + """(the TOML body for the server, warnings). The body always parses and + always has session.resume_agents_on_restore = false.""" + if not user_text or not user_text.strip(): + return MINIMAL, [] + try: + user = tomllib.loads(user_text) + except tomllib.TOMLDecodeError as e: + return MINIMAL, [f"herdr config does not parse ({e}); the MAM server uses only {KEY} = false"] + if (user.get("session") or {}).get(KEY) is False: + return user_text, [] + merged = _merge_text(user_text) + try: + result = tomllib.loads(merged) + ok = result["session"][KEY] is False and _without_key(result) == _without_key(user) + except (tomllib.TOMLDecodeError, KeyError, TypeError): + ok = False + if not ok: + return MINIMAL, [f"could not set [session] {KEY} in the herdr config (dotted key or inline table?); " + f"the MAM server uses only {KEY} = false, without the rest of your settings"] + return merged, [] + + +def _write_atomic(path: str, text: str, mode: int) -> None: + d = os.path.dirname(path) + os.makedirs(d, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=d, prefix=".mam-server-config.", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(text) + os.chmod(tmp, mode) + os.replace(tmp, path) + except BaseException: + try: + os.unlink(tmp) + except OSError: + pass + raise + + +def write_server_config(env: Mapping[str, str], fallback_dir: Optional[str] = None) -> Tuple[Optional[str], List[str]]: + """Write the server config; (its path or None, warnings).""" + source = user_config_path(env) + warnings: List[str] = [] + user_text: Optional[str] = None + mode = 0o644 + try: + with open(source, encoding="utf-8") as f: + user_text = f.read() + mode = os.stat(source).st_mode & 0o777 + except FileNotFoundError: + pass + except (OSError, UnicodeDecodeError) as e: + # herdr cannot use it either, so it runs on defaults: same as no file. + warnings.append(f"cannot read herdr config {source}: {e}") + body, merge_warnings = server_config_text(user_text) + warnings += merge_warnings + text = f"{MARKER}\n{SOURCE_PREFIX}{source}\n{body}" + target = os.path.join(os.path.dirname(source), OUTPUT_NAME) + try: + _write_atomic(target, text, mode) + return target, warnings + except OSError as e: + warnings.append(f"cannot write {target}: {e}") + if fallback_dir: + target = os.path.join(fallback_dir, FALLBACK_NAME) + try: + _write_atomic(target, text, mode) + warnings.append(f"wrote {target} instead; relative paths in the herdr config may not resolve") + return target, warnings + except OSError as e: + warnings.append(f"cannot write {target}: {e}") + return None, warnings + + +def main(argv=None) -> int: + p = argparse.ArgumentParser(description="herdr config for MAM's isolated servers (Issue #45)") + sub = p.add_subparsers(dest="cmd", required=True) + sc = sub.add_parser("server-config", help="write the server config and print its path") + sc.add_argument("--fallback-dir", default=None, help="where to write when the config dir is not writable") + args = p.parse_args(argv) + path, warnings = write_server_config(os.environ, args.fallback_dir) + for w in warnings: + print(f"WARNING: {w}", file=sys.stderr) + if not path: + return 1 + print(path) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py new file mode 100644 index 0000000..add63e4 --- /dev/null +++ b/tests/test_issue45_herdr_config.py @@ -0,0 +1,192 @@ +"""Issue #45 (A): the herdr config MAM's isolated servers start with. + +lib_py/herdr_config.py builds the user's herdr config with only +[session] resume_agents_on_restore forced to false. herdr drops a config that +does not parse and runs on defaults - restore on - so every result here is +re-parsed, and fed to the real `herdr config check` when herdr is installed. +That restore really is off on a live server is test_issue45_restore.py. +""" +from __future__ import annotations + +import os +import shutil +import stat +import subprocess +import sys +import tomllib +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / ".agents" / "skills")) + +from lib_py import herdr_config as hc # noqa: E402 + +HERDR = shutil.which("herdr") + +# (name, user config text or None, expect the user's other settings kept, expect a warning) +CASES = [ + ("missing", None, True, False), + ("empty", "", True, False), + ("broken", "[session]\n[session]\n", False, True), + ("no_session", "[ui]\nconfirm_close = false\n", True, False), + ("no_session_no_newline", "[ui]\nconfirm_close = false", True, False), + ("session_without_key", "[session]\n# a comment\n\n[ui]\nconfirm_close = false\n", True, False), + ("key_true", "[ui]\nconfirm_close = false\n[session]\nresume_agents_on_restore = true # mine\n", True, False), + ("key_false", "[session]\nresume_agents_on_restore=false\n", True, False), + ("sub_table_only", "[session.extra]\na = 1\n", True, False), + ("dotted_key", "session.resume_agents_on_restore = true\n", False, True), + ("inline_table", "session = { resume_agents_on_restore = true }\n", False, True), + ("header_inside_string", 'banner = """\n[session]\n"""\n', False, True), +] + + +def _without_key(d): + return hc._without_key(d) + + +@pytest.mark.parametrize("name,user,kept,warned", CASES, ids=[c[0] for c in CASES]) +def test_merge_always_parses_with_restore_off(name, user, kept, warned): + text, warnings = hc.server_config_text(user) + parsed = tomllib.loads(text) + assert parsed["session"]["resume_agents_on_restore"] is False + assert bool(warnings) == warned, warnings + if kept and user: + assert _without_key(parsed) == _without_key(tomllib.loads(user)) + if not kept: + assert text == hc.MINIMAL + + +def test_existing_false_is_left_byte_for_byte(): + user = "[session]\nresume_agents_on_restore=false # keep\n" + assert hc.server_config_text(user) == (user, []) + + +def test_a_bad_value_type_falls_back_to_minimal(): + # herdr rejects a non-bool here as a parse error of the whole file. + text, warnings = hc.server_config_text('[session]\nresume_agents_on_restore = "no"\n') + assert tomllib.loads(text)["session"]["resume_agents_on_restore"] is False + # tomllib reads the string fine, so the merge replaces the line instead. + assert warnings == [] + + +def _env(tmp_path, **kw): + env = {"HOME": str(tmp_path / "home")} + env.update(kw) + return env + + +def test_source_is_home_config_by_default(tmp_path): + assert hc.user_config_path(_env(tmp_path)) == str(tmp_path / "home" / ".config" / "herdr" / "config.toml") + + +def test_source_follows_xdg_config_home(tmp_path, monkeypatch): + assert hc.user_config_path(_env(tmp_path, XDG_CONFIG_HOME=str(tmp_path / "x"))) == \ + str(tmp_path / "x" / "herdr" / "config.toml") + monkeypatch.chdir(tmp_path) # herdr takes a relative one against its cwd + assert hc.user_config_path(_env(tmp_path, XDG_CONFIG_HOME="rel")) == \ + str(tmp_path / "rel" / "herdr" / "config.toml") + + +def test_source_is_herdr_config_path_unless_generated(tmp_path): + user = tmp_path / "custom.toml" + user.write_text("[ui]\nconfirm_close = false\n") + env = _env(tmp_path, HERDR_CONFIG_PATH=str(user), XDG_CONFIG_HOME=str(tmp_path / "x")) + assert hc.user_config_path(env) == str(user) + + # A MAM pane inherits HERDR_CONFIG_PATH=; a server bootstrapped + # from there must still build from the user's file, not from the copy. + generated, _ = hc.write_server_config(env) + assert generated == str(tmp_path / hc.OUTPUT_NAME) + assert hc.user_config_path(dict(env, HERDR_CONFIG_PATH=generated)) == str(user) + + # A generated file without a source line falls through to XDG / HOME. + bare = tmp_path / "bare.toml" + bare.write_text(hc.MARKER + "\n" + hc.MINIMAL) + assert hc.user_config_path(dict(env, HERDR_CONFIG_PATH=str(bare))) == \ + str(tmp_path / "x" / "herdr" / "config.toml") + + +def test_written_next_to_source_with_its_mode(tmp_path): + cfg_dir = tmp_path / "home" / ".config" / "herdr" + cfg_dir.mkdir(parents=True) + user = cfg_dir / "config.toml" + user.write_text('[ui.sound]\npath = "sounds/done.wav"\n') + user.chmod(0o600) + path, warnings = hc.write_server_config(_env(tmp_path)) + assert path == str(cfg_dir / hc.OUTPUT_NAME) and warnings == [] + text = Path(path).read_text() + assert text.splitlines()[:2] == [hc.MARKER, hc.SOURCE_PREFIX + str(user)] + parsed = tomllib.loads(text) + # same directory, so the relative sound path resolves to the same file + assert parsed["ui"]["sound"]["path"] == "sounds/done.wav" + assert parsed["session"]["resume_agents_on_restore"] is False + assert stat.S_IMODE(os.stat(path).st_mode) == 0o600 + assert user.read_text() == '[ui.sound]\npath = "sounds/done.wav"\n' # untouched + assert [p.name for p in cfg_dir.iterdir() if p.name.endswith(".tmp")] == [] + + +def test_written_without_any_user_config(tmp_path): + path, warnings = hc.write_server_config(_env(tmp_path)) + assert path == str(tmp_path / "home" / ".config" / "herdr" / hc.OUTPUT_NAME) and warnings == [] + assert stat.S_IMODE(os.stat(path).st_mode) == 0o644 + assert tomllib.loads(Path(path).read_text()) == {"session": {"resume_agents_on_restore": False}} + + +def test_rewritten_on_every_call(tmp_path): + cfg_dir = tmp_path / "home" / ".config" / "herdr" + cfg_dir.mkdir(parents=True) + (cfg_dir / "config.toml").write_text("[ui]\nconfirm_close = true\n") + path, _ = hc.write_server_config(_env(tmp_path)) + (cfg_dir / "config.toml").write_text("[ui]\nconfirm_close = false\n") + hc.write_server_config(_env(tmp_path)) + assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") +def test_unwritable_config_dir_falls_back_to_mam_dir(tmp_path): + cfg_dir = tmp_path / "home" / ".config" / "herdr" + cfg_dir.mkdir(parents=True) + (cfg_dir / "config.toml").write_text("[ui]\nconfirm_close = false\n") + cfg_dir.chmod(0o555) + try: + mam = tmp_path / ".mam" + mam.mkdir() + path, warnings = hc.write_server_config(_env(tmp_path), str(mam)) + assert path == str(mam / hc.FALLBACK_NAME) + assert any("relative paths" in w for w in warnings), warnings + assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + assert hc.write_server_config(_env(tmp_path))[0] is None # no fallback given + finally: + cfg_dir.chmod(0o755) + + +def test_cli_prints_the_path(tmp_path): + env = dict(os.environ, HOME=str(tmp_path), PYTHONPATH=str(REPO_ROOT / ".agents" / "skills")) + env.pop("HERDR_CONFIG_PATH", None) + env.pop("XDG_CONFIG_HOME", None) + cfg_dir = tmp_path / ".config" / "herdr" + cfg_dir.mkdir(parents=True) + (cfg_dir / "config.toml").write_text("[session]\n[session]\n") + res = subprocess.run([sys.executable, "-m", "lib_py.herdr_config", "server-config"], + capture_output=True, text=True, env=env) + assert res.returncode == 0, res.stderr + assert res.stdout == str(cfg_dir / hc.OUTPUT_NAME) + "\n" + assert "WARNING: herdr config does not parse" in res.stderr + + +@pytest.mark.skipif(not HERDR, reason="herdr is not installed") +@pytest.mark.parametrize("name,user,kept,warned", CASES, ids=[c[0] for c in CASES]) +def test_real_herdr_accepts_every_result(tmp_path, name, user, kept, warned): + text, _ = hc.server_config_text(user) + cfg = tmp_path / "server.toml" + cfg.write_text(hc.MARKER + "\n" + text) + env = {k: v for k, v in os.environ.items() if not k.startswith("HERDR_")} + env.update(HOME=str(tmp_path), HERDR_CONFIG_PATH=str(cfg)) + res = subprocess.run([HERDR, "config", "check"], capture_output=True, text=True, env=env) + out = res.stdout + res.stderr + assert "parse error" not in out, out + # the fixtures use only keys herdr knows, apart from the made-up ones below + issues = [l for l in out.splitlines()[1:] if l.strip()] + assert all(l.startswith("unknown config key") and ("extra" in l or "banner" in l) for l in issues), out -- 2.54.0 From ed73f67a4f884aa22979e6d04bb94342f4a6aa50 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:32:35 +0000 Subject: [PATCH 02/14] fix(issue-45): the shim bootstrap starts MAM servers with restore-on-restart off The headless bootstrap regenerates the server config on every start and hands it to the server as HERDR_CONFIG_PATH. Without a file the server still starts, with a warning. mam_sandbox unsets HERDR_CONFIG_PATH/XDG_CONFIG_HOME so the file lands in the sandbox HOME. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib.sh | 16 +++++++++++++++- tests/conftest.py | 5 +++++ tests/test_issue9_server_env.py | 21 +++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 51a2fa8..4cd9a2c 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -230,6 +230,18 @@ except Exception: # identity to its sub-agents. Drop only per-session runtime markers - user # configuration such as CLAUDE_PROJECT_DIR or CLAUDE_CONFIG_DIR still # inherits. The family ids mirror what orc_onboard.sh auto-detects. + # + # ISSUE-45: the server restores its saved layout on start and would + # relaunch every saved agent with herdr's own resume command (no permission + # flags, deleted agents included). It gets HERDR_CONFIG_PATH = the user's + # herdr config with [session] resume_agents_on_restore = false, rebuilt on + # every start (lib_py/herdr_config.py). Without that file the server still + # starts - with restore on, as before. + MAM_SERVER_CFG=$(python3 -m lib_py.herdr_config server-config --fallback-dir "$_SHIM_DIR/.." || true) + export MAM_SERVER_CFG + if [ -z "$MAM_SERVER_CFG" ]; then + echo "WARNING: restore-on-restart stays enabled for herdr session $_MAM_SESSION (no server config)" >&2 + fi _mam_server_pid=$(python3 -c ' import os, subprocess, sys deny = ("CLAUDECODE", "CLAUDE_CODE_CHILD_SESSION", "CLAUDE_CODE_SESSION_ID", @@ -237,8 +249,10 @@ deny = ("CLAUDECODE", "CLAUDE_CODE_CHILD_SESSION", "CLAUDE_CODE_SESSION_ID", "CLAUDE_CODE_MESSAGING_TOKEN", "CLAUDE_CODE_MESSAGING_SOCKET", "CLAUDE_CODE_BRIDGE_SESSION_ID", "ANTIGRAVITY_CONVERSATION_ID", "HERMES_SESSION_ID", "GROK_SESSION_ID", - "OPENCODE_SESSION_ID") + "OPENCODE_SESSION_ID", "MAM_SERVER_CFG") env = {k: v for k, v in os.environ.items() if k not in deny} +if os.environ.get("MAM_SERVER_CFG"): + env["HERDR_CONFIG_PATH"] = os.environ["MAM_SERVER_CFG"] p = subprocess.Popen(sys.argv[1:], start_new_session=True, env=env, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL) diff --git a/tests/conftest.py b/tests/conftest.py index ab7560b..c4f47e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -236,6 +236,11 @@ def mam_sandbox(tmp_path, monkeypatch, request): monkeypatch.delenv("HERDR_SESSION_NAME", raising=False) monkeypatch.delenv("HERDR_SERVER_NAME", raising=False) monkeypatch.delenv("HERDR_WORKSPACE_ID", raising=False) + # Issue #45: the shim bootstrap writes a herdr server config next to the + # user's; with these unset that is $HOME/.config/herdr, inside the sandbox. + # Every pane of a MAM server inherits HERDR_CONFIG_PATH. + monkeypatch.delenv("HERDR_CONFIG_PATH", raising=False) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) import sys monkeypatch.setenv("AGENT_PYTHON_BIN", sys.executable) diff --git a/tests/test_issue9_server_env.py b/tests/test_issue9_server_env.py index 500dfb2..9edca48 100644 --- a/tests/test_issue9_server_env.py +++ b/tests/test_issue9_server_env.py @@ -150,6 +150,27 @@ def test_bootstrap_strips_agent_runtime_markers(mam_sandbox): assert key in server_env +def test_bootstrap_hands_the_server_a_no_restore_config(mam_sandbox): + """Issue #45: the server starts with HERDR_CONFIG_PATH = the user's herdr + config plus [session] resume_agents_on_restore = false (that herdr obeys it + is test_issue45_restore.py, on a real server).""" + import tomllib + env = _install_fake_herdr(mam_sandbox) + user_cfg = mam_sandbox / ".config" / "herdr" / "config.toml" + user_cfg.parent.mkdir(parents=True) + user_cfg.write_text("[ui]\nconfirm_close = false\n") + + res = _run_through_shim(mam_sandbox, env, "issue45-probe") + assert res.returncode == 0, res.stderr + assert "restore-on-restart stays enabled" not in res.stderr, res.stderr + + server_env = json.loads(Path(env["FAKE_HERDR_ENV_DUMP"]).read_text()) + assert server_env.get("HERDR_CONFIG_PATH") == str(user_cfg.parent / "mam-server-config.toml") + assert "MAM_SERVER_CFG" not in server_env + cfg = tomllib.loads(Path(server_env["HERDR_CONFIG_PATH"]).read_text()) + assert cfg == {"ui": {"confirm_close": False}, "session": {"resume_agents_on_restore": False}} + + def test_no_autostart_skips_bootstrap_but_still_routes_the_call(mam_sandbox): env = _install_fake_herdr(mam_sandbox) env["MAM_NO_AUTOSTART"] = "1" -- 2.54.0 From 933af344c2e3c0932dc525b66ee297ebe420571d Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:32:49 +0000 Subject: [PATCH 03/14] test(issue-45): S11 compares the shim heredoc with the Issue #45 bootstrap commit S11 keeps the generated .mam/shim/herdr from changing by accident; #45 changes it on purpose, so its base moves to ed73f67. Co-Authored-By: Claude Opus 5.5 --- tests/test_issue23_24_startup_path.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_issue23_24_startup_path.py b/tests/test_issue23_24_startup_path.py index ddcdac6..71addd3 100644 --- a/tests/test_issue23_24_startup_path.py +++ b/tests/test_issue23_24_startup_path.py @@ -296,9 +296,10 @@ def test_s11_this_branch_does_not_change_the_generated_shim(mam_sandbox): lib.sh materialises into $WORKSPACE_ROOT/.mam/shim/herdr. That file is shared by every checkout bound to one control plane, so keeping it byte-identical to base is what makes "the control plane is not modified" - true at runtime.""" + true at runtime. Issue #45 changed the heredoc on purpose (the bootstrap + starts the server with restore-on-restart off), so the base is that commit.""" repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - base = os.environ.get("MAM_BASE_SHA", "1e78dae") + base = os.environ.get("MAM_BASE_SHA", "ed73f67") show = subprocess.run(["git", "show", "%s:.agents/skills/lib.sh" % base], capture_output=True, text=True, cwd=repo) if show.returncode != 0: -- 2.54.0 From fb1380fdc19968d8d5bb2a8e8f8886fb6523ed74 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:32:49 +0000 Subject: [PATCH 04/14] fix(issue-45): resume closes the session's own restored empty shell After a restart with restore off, the session's saved pane comes back as an empty shell (in $HOME when its directory is gone) holding the agent_session it reported. A resume with a CP_PREFIX cannot reuse it, so it stayed next to the new pane. Just before spawning, resume closes panes whose agent_session is the id being resumed, with no live agent and only the shell in the foreground; the registry is not consulted, since a session whose server died stays running. tests/test_issue45_restore.py drives a real herdr server under a throwaway HOME and session with a fake claude: restore stays off through the MAM bootstrap (with a restore-on control), the close conditions, and create -> crash -> resume leaving one pane. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/herdr_restore.py | 88 +++++ .../scripts/resume_session.sh | 8 + tests/test_issue45_restore.py | 342 ++++++++++++++++++ 3 files changed, 438 insertions(+) create mode 100644 .agents/skills/lib_py/herdr_restore.py create mode 100644 tests/test_issue45_restore.py diff --git a/.agents/skills/lib_py/herdr_restore.py b/.agents/skills/lib_py/herdr_restore.py new file mode 100644 index 0000000..07b86e6 --- /dev/null +++ b/.agents/skills/lib_py/herdr_restore.py @@ -0,0 +1,88 @@ +"""Issue #45 (B): close the restored empty shells of the session being resumed. + +With restore-on-restart off (lib_py/herdr_config.py) a restarted MAM server +still brings every saved pane back - as an empty shell in its old cwd, or in +$HOME when that directory is gone. A pane keeps the `agent_session` its agent +reported before the server died. resume_session.sh calls this once, right +before it spawns, with the id it is about to resume; a pane is closed only when +all of these hold: + + 1. the target is an isolated MAM server (HERDR_SESSION_NAME set, not default); + 2. herdr sees no live agent in it; + 3. its agent_session value is that id - so a user's pane, another session's + pane and a freshly split pane (no agent_session yet) are never candidates; + 4. its foreground process is the shell itself (nothing running in it). + +The registry is not consulted: a session whose server died stays `running` +there, so "running" cannot tell a live pane from a restored one. + +CLI: `python3 -m lib_py.herdr_restore close-restored-shells --session-id ID +[--herdr PATH]`. One stderr line per closed pane; exit 1 when herdr cannot be +queried (the caller only warns). +""" +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from typing import List, Optional + +# A pane exports these; they would point the query at the caller's own server +# and workspace instead of the --session named here. +_SCOPE_ENV = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH") + + +def _herdr(herdr: str, session: str, *args: str) -> dict: + env = {k: v for k, v in os.environ.items() if k not in _SCOPE_ENV} + out = subprocess.run([herdr, "--session", session, *args], env=env, + capture_output=True, text=True, timeout=30, check=True).stdout + return json.loads(out).get("result") or {} + + +def _shell_in_foreground(herdr: str, session: str, pane_id: str) -> bool: + info = _herdr(herdr, session, "pane", "process-info", "--pane", pane_id).get("process_info") or {} + shell = info.get("shell_pid") + return shell is not None and info.get("foreground_process_group_id") == shell + + +def close_restored_shells(session_id: str, herdr: str = "herdr", + session: Optional[str] = None) -> List[str]: + """Close the matching panes; return their ids.""" + session = os.environ.get("HERDR_SESSION_NAME", "") if session is None else session + if not session_id or session in ("", "default"): + return [] + closed = [] + for p in _herdr(herdr, session, "pane", "list").get("panes") or []: + pane_id = p.get("pane_id") + if not pane_id or p.get("agent"): + continue + if (p.get("agent_session") or {}).get("value") != session_id: + continue + if not _shell_in_foreground(herdr, session, pane_id): + continue + _herdr(herdr, session, "pane", "close", pane_id) + print(f"closed restored pane {pane_id} (cwd={p.get('cwd', '')}, agent_session={session_id})", + file=sys.stderr) + closed.append(pane_id) + return closed + + +def main(argv=None) -> int: + p = argparse.ArgumentParser(description="close a resumed session's restored empty shells (Issue #45)") + sub = p.add_subparsers(dest="cmd", required=True) + c = sub.add_parser("close-restored-shells") + c.add_argument("--session-id", required=True) + c.add_argument("--herdr", default="", help="the real herdr binary (not the MAM shim)") + args = p.parse_args(argv) + try: + close_restored_shells(args.session_id, args.herdr or "herdr") + except (OSError, subprocess.SubprocessError, ValueError) as e: + print(f"close-restored-shells: {e}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh index 78a58d0..02d9be9 100755 --- a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh +++ b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh @@ -233,6 +233,14 @@ if [ "$AGENT" = "opencode" ]; then fi fi +# Issue #45: a restarted server brings this session's old pane back as an empty +# shell (restore-on-restart is off on MAM servers). The shim cannot reuse it when +# a CP_PREFIX is passed, nor when it fell back to $HOME, so close it first; the +# conditions live in lib_py/herdr_restore.py. Never blocks the resume. +"$(_delegate_py_bin)" -m lib_py.herdr_restore close-restored-shells --session-id "$UUID" \ + --herdr "$(_resolve_real_herdr_path || true)" \ + || echo "WARNING: could not close the restored panes of $SESSION_NAME; an empty shell may remain" >&2 + # 4. Spawn new agent session (delegates to herdr translation shim) _herdr new-session -d -s "$SESSION_NAME" -c "$WORKSPACE" "$CP_PREFIX$CMD_FULL" # Issue #24: every agent, the same call create makes. The signatures and accept diff --git a/tests/test_issue45_restore.py b/tests/test_issue45_restore.py new file mode 100644 index 0000000..66eb254 --- /dev/null +++ b/tests/test_issue45_restore.py @@ -0,0 +1,342 @@ +"""Issue #45 against a real herdr server: a restarted MAM server must not relaunch +saved agents (A), and resume must not leave the session's restored empty shell +behind (B). + +Everything runs on a throwaway herdr session under a short throwaway HOME, so +the server's socket, session.json and config never touch the user's herdr (the +running `multi-agent-mux` server included). The agent is a fake `claude` +script that logs its argv; the user's herdr config sets `[terminal] +default_shell` to a shell that puts the fake first on PATH, so herdr's own +restore command (`claude --resume `) reaches the fake too. That is the +positive control: with restore on, the fake logs `--resume `; through the +MAM bootstrap it must not. No real claude runs and no API is called. + +A crash is `kill -9` of the server after session.json has the pane's +agent_session (herdr writes it a few seconds after a change). +""" +from __future__ import annotations + +import json +import os +import shutil +import signal +import subprocess +import sys +import tempfile +import time +import uuid +from pathlib import Path + +import pytest + +HERDR = shutil.which("herdr") +pytestmark = pytest.mark.skipif(not HERDR, reason="real herdr is not installed") + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / ".agents" / "skills")) +from lib_py import herdr_config # noqa: E402 + +# Inherited from the pane this suite may run in: they would point herdr at that +# pane's server and workspace. +SCOPE_ENV = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH", + "HERDR_SESSION", "HERDR_ENV", "HERDR_CONFIG_PATH", "XDG_CONFIG_HOME") + + +class Rig: + def __init__(self, sandbox: Path, root: Path, session: str): + self.sandbox, self.root, self.session = sandbox, root, session + self.bin = root / "bin" + self.argv_log = root / "argv.log" + self.sdir = root / ".config" / "herdr" / "sessions" / session + self.user_cfg = root / ".config" / "herdr" / "config.toml" + + # -- herdr --------------------------------------------------------------- + def env(self, **extra) -> dict: + env = {k: v for k, v in os.environ.items() if k not in SCOPE_ENV} + env["PATH"] = f"{self.bin}:{env['PATH']}" + env.update(extra) + return env + + def hd(self, *args) -> dict: + out = subprocess.run([HERDR, "--session", self.session, *args], env=self.env(), + capture_output=True, text=True, timeout=30) + assert out.returncode == 0, f"herdr {args}: {out.stdout}{out.stderr}" + return json.loads(out.stdout).get("result") or {} if out.stdout.strip() else {} + + def panes(self) -> list: + return self.hd("pane", "list").get("panes") or [] + + def server_pids(self) -> list: + out = subprocess.run(["pgrep", "-f", "--", f"--session {self.session} server"], + capture_output=True, text=True).stdout + return [int(p) for p in out.split()] + + def _wait_socket(self): + sock = self.sdir / "herdr.sock" + for _ in range(80): + if sock.exists(): + return + time.sleep(0.25) + raise AssertionError("herdr server socket never appeared") + + def restores(self) -> int: + log = self.sdir / "herdr-server.log" + return log.read_text().count("session restore evaluated") if log.exists() else 0 + + def start_raw(self, config: Path | None = None): + """A server as the user would start it: restore on unless `config` says otherwise.""" + env = self.env(**({"HERDR_CONFIG_PATH": str(config)} if config else {})) + subprocess.Popen([HERDR, "--session", self.session, "server"], env=env, start_new_session=True, + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + self._wait_socket() + + def run_lib(self, body: str, **extra) -> subprocess.CompletedProcess: + lib = self.sandbox / ".agents" / "skills" / "lib.sh" + return subprocess.run(["bash", "-c", f'source "{lib}"\n{body}'], cwd=str(self.sandbox), + env=self.env(HERDR_SESSION_NAME=self.session, **extra), + capture_output=True, text=True, timeout=120) + + def start_via_shim(self): + """The only way MAM starts a server: the shim's headless bootstrap.""" + res = self.run_lib("herdr has-session -t nobody-here || true") + assert res.returncode == 0, res.stderr + assert "restore-on-restart stays enabled" not in res.stderr, res.stderr + try: + self._wait_socket() + except AssertionError: + raise AssertionError(f"the shim did not start the server: {res.stderr}") + + def wait_restored(self, before: int): + for _ in range(60): + if self.restores() > before: + time.sleep(3) # let herdr spawn whatever it is going to spawn + return + time.sleep(0.25) + raise AssertionError("the server never evaluated its saved session") + + # -- panes --------------------------------------------------------------- + def wait_shell(self, pane_id: str): + for _ in range(40): + info = self.hd("pane", "process-info", "--pane", pane_id)["process_info"] + if info.get("shell_pid"): + return + time.sleep(0.25) + + def new_pane(self, cwd: Path, label: str) -> str: + cwd.mkdir(parents=True, exist_ok=True) + pane = self.hd("workspace", "create", "--cwd", str(cwd), "--label", label, "--no-focus")["root_pane"]["pane_id"] + self.wait_shell(pane) + return pane + + def run_agent(self, pane_id: str, session_id: str): + """The fake claude, plus the session report herdr's claude integration would send.""" + self.hd("pane", "run", pane_id, f"claude --session-id {session_id}") + self.hd("pane", "report-agent", "--source", "herdr:claude", "--agent", "claude", + "--state", "idle", "--agent-session-id", session_id, pane_id) + + def crash(self, *session_ids: str): + """kill -9 the server once session.json holds every id.""" + saved = self.sdir / "session.json" + for _ in range(80): + text = saved.read_text() if saved.exists() else "" + if all(s in text for s in session_ids): + break + time.sleep(0.25) + else: + raise AssertionError(f"session.json never recorded {session_ids}") + for pid in self.server_pids(): + os.kill(pid, signal.SIGKILL) + self.kill_fakes() + time.sleep(0.5) + (self.sdir / "herdr.sock").unlink(missing_ok=True) + + def kill_fakes(self): + subprocess.run(["pkill", "-9", "-f", f"{self.bin}/claude"], capture_output=True) + + def resumed(self, session_id: str) -> bool: + return self.argv_log.exists() and f"--resume {session_id}" in self.argv_log.read_text() + + +@pytest.fixture +def rig(mam_sandbox, monkeypatch): + # A pytest tmp_path is too long for a unix socket path under it. + root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp")) + session = f"mamt45-{os.getpid()}-{uuid.uuid4().hex[:4]}" + for k in SCOPE_ENV: + monkeypatch.delenv(k, raising=False) + monkeypatch.setenv("HOME", str(root)) + monkeypatch.setenv("HOME_DIR", str(root)) + r = Rig(mam_sandbox, root, session) + r.bin.mkdir() + fake = r.bin / "claude" + fake.write_text(f'#!/usr/bin/env bash\n' + 'if [ "${1:-}" = auth ]; then echo \'{"loggedIn": true}\'; exit 0; fi\n' + f'echo "$*" >> "{r.argv_log}"\n' + 'echo "Welcome to Claude Code (fake)"\nwhile :; do sleep 1; done\n') + fake.chmod(0o755) + shell = root / "shell.sh" + shell.write_text(f'#!/usr/bin/env bash\nexport PATH="{r.bin}:$PATH"\nexec bash --norc -i\n') + shell.chmod(0o755) + r.user_cfg.parent.mkdir(parents=True) + r.user_cfg.write_text(f'[terminal]\ndefault_shell = "{shell}"\n') + try: + yield r + finally: + subprocess.run([HERDR, "session", "stop", session], env=r.env(), capture_output=True, timeout=30) + for pid in r.server_pids(): + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + r.kill_fakes() + subprocess.run(["pkill", "-9", "-f", str(root)], capture_output=True) + subprocess.run([HERDR, "session", "delete", session], env=r.env(), capture_output=True, timeout=30) + shutil.rmtree(root, ignore_errors=True) + + +def _pane_with(rig: Rig, session_id: str) -> dict | None: + return next((p for p in rig.panes() if (p.get("agent_session") or {}).get("value") == session_id), None) + + +# --- A --------------------------------------------------------------------- + +@pytest.mark.parametrize("user_session_table", [ + "", + # an existing [session] table: appending a second one would break the TOML + # and herdr would silently restore again + "[session]\nresume_agents_on_restore = true\n", +], ids=["plain-user-config", "user-config-with-session-table"]) +def test_a_mam_server_does_not_relaunch_saved_agents(rig, user_session_table): + rig.user_cfg.write_text(rig.user_cfg.read_text() + user_session_table) + sid = str(uuid.uuid4()) + + rig.start_raw() + rig.run_agent(rig.new_pane(rig.root / "wsA", "A"), sid) + rig.crash(sid) + + # MAM brings the server back: nothing may be relaunched. + before = rig.restores() + rig.start_via_shim() + rig.wait_restored(before) + assert not rig.resumed(sid), rig.argv_log.read_text() + pane = _pane_with(rig, sid) + assert pane is not None and not pane.get("agent"), rig.panes() + info = rig.hd("pane", "process-info", "--pane", pane["pane_id"])["process_info"] + assert info["foreground_process_group_id"] == info["shell_pid"], info + # and it got there through HERDR_CONFIG_PATH + environ = Path(f"/proc/{rig.server_pids()[0]}/environ").read_bytes().split(b"\0") + assert f"HERDR_CONFIG_PATH={rig.user_cfg.parent / herdr_config.OUTPUT_NAME}".encode() in environ + + # Positive control: the same saved layout under a plain server relaunches + # the agent, so the "not resumed" above is not vacuous. + rig.crash(sid) + before = rig.restores() + rig.start_raw() + rig.wait_restored(before) + assert rig.resumed(sid), rig.argv_log.read_text() if rig.argv_log.exists() else "no argv log" + + +# --- B --------------------------------------------------------------------- + +def _close(rig: Rig, session_id: str) -> subprocess.CompletedProcess: + env = rig.env(HERDR_SESSION_NAME=rig.session, PYTHONPATH=str(rig.sandbox / ".agents" / "skills")) + return subprocess.run([sys.executable, "-m", "lib_py.herdr_restore", "close-restored-shells", + "--session-id", session_id, "--herdr", HERDR], + env=env, capture_output=True, text=True, timeout=60) + + +def test_b_only_the_resumed_sessions_restored_shell_is_closed(rig): + ids = {k: str(uuid.uuid4()) for k in ("X", "Y", "Z", "X2")} + rig.start_raw() + for k in ids: + rig.run_agent(rig.new_pane(rig.root / f"ws{k}", k), ids[k]) + rig.new_pane(rig.root / "wsU", "U") # a user's plain shell: no agent_session + rig.crash(*ids.values()) + shutil.rmtree(rig.root / "wsY") # a removed worktree: restored into $HOME + + before = rig.restores() + rig.start_via_shim() + rig.wait_restored(before) + rig.run_agent(_pane_with(rig, ids["Z"])["pane_id"], ids["Z"]) # running again + rig.hd("pane", "run", _pane_with(rig, ids["X2"])["pane_id"], "sleep 60") # busy shell + time.sleep(1) + assert _pane_with(rig, ids["Y"])["cwd"] == str(rig.root) + n = len(rig.panes()) + + for k in ("Z", "X2"): + res = _close(rig, ids[k]) + assert res.returncode == 0 and "closed" not in res.stderr, res.stderr + res = _close(rig, "") + assert res.returncode == 0 and "closed" not in res.stderr, res.stderr + assert len(rig.panes()) == n + + x = _pane_with(rig, ids["X"])["pane_id"] + res = _close(rig, ids["X"]) + assert res.returncode == 0, res.stderr + assert f"closed restored pane {x}" in res.stderr + assert _pane_with(rig, ids["X"]) is None and len(rig.panes()) == n - 1 + + res = _close(rig, ids["Y"]) + assert res.returncode == 0 and "closed restored pane" in res.stderr, res.stderr + assert _pane_with(rig, ids["Y"]) is None and len(rig.panes()) == n - 2 + # the user's shell, the running agent and the busy shell are all still there + assert any(p.get("cwd") == str(rig.root / "wsU") for p in rig.panes()) + assert _pane_with(rig, ids["Z"]) and _pane_with(rig, ids["X2"]) + + +def test_b_default_server_is_never_touched(rig): + env = rig.env(HERDR_SESSION_NAME="default", PYTHONPATH=str(rig.sandbox / ".agents" / "skills")) + # a herdr that would fail loudly if called at all + res = subprocess.run([sys.executable, "-m", "lib_py.herdr_restore", "close-restored-shells", + "--session-id", "some-id", "--herdr", "/nonexistent/herdr"], + env=env, capture_output=True, text=True, timeout=30) + assert res.returncode == 0 and res.stderr == "", res.stderr + + +def _script(rig: Rig, rel: str, *args: str, timeout: int = 180) -> subprocess.CompletedProcess: + """Output goes through files: a server the script bootstraps outlives it.""" + path = rig.sandbox / ".agents" / "skills" / rel + out, err = rig.root / "script.out", rig.root / "script.err" + with open(out, "w") as o, open(err, "w") as e: + try: + p = subprocess.run(["bash", str(path), *args], cwd=str(rig.sandbox), env=rig.env(), + stdout=o, stderr=e, text=True, timeout=timeout) + except subprocess.TimeoutExpired: + raise AssertionError(f"{rel} timed out:\n{out.read_text()}\n{err.read_text()}") + return subprocess.CompletedProcess(p.args, p.returncode, out.read_text(), err.read_text()) + + +def test_b_resume_leaves_no_restored_shell_behind(rig): + """The real create -> crash -> resume path. The row create writes stays + `running` through the crash, as it does in production; resume runs from a + worktree whose control plane is the sandbox, so it spawns with a CP_PREFIX + and the shim cannot reuse the restored shell (it splits a new pane).""" + ws = rig.root / "wsX" + ws.mkdir() + name = "creator-claude-m45" + res = _script(rig, "multi-agent-mux-create/scripts/create_session.sh", "--workspace", str(ws), + "--agent", "claude", "--role", "creator", "--session", name, + "--herdr-session", rig.session, "--no-onboard", timeout=60) + assert res.returncode == 0, res.stdout + res.stderr + rows = json.loads(rig.run_lib("load_state_json").stdout)["herdr_sessions"] + row = next(r for r in rows if r["name"] == name) + sid = row["claude_session_id_own"] + assert row["status"] == "running" and sid + pane = next(p for p in rig.panes() if p.get("cwd") == str(ws)) + # what herdr's claude integration reports for a real claude (checked on a live server) + rig.hd("pane", "report-agent", "--source", "herdr:claude", "--agent", "claude", + "--state", "idle", "--agent-session-id", sid, pane["pane_id"]) + rig.crash(sid) + + res = _script(rig, "multi-agent-mux-resume/scripts/resume_session.sh", "--workspace", str(ws), + "--agent", "claude", "--session", name) + assert res.returncode == 0, res.stdout + res.stderr + assert not rig.resumed(sid) # herdr did not relaunch it ... + # ... MAM did, with its own command: create's launch plus resume's, both flagged + launches = rig.argv_log.read_text().splitlines() + assert len(launches) == 2 and all("--dangerously-skip-permissions" in l and sid in l for l in launches), launches + in_ws = [p for p in rig.panes() if p.get("cwd") == str(ws)] + assert len(in_ws) == 1, in_ws + info = rig.hd("pane", "process-info", "--pane", in_ws[0]["pane_id"])["process_info"] + assert info["foreground_process_group_id"] != info["shell_pid"], info # the agent, not an empty shell + assert "closed restored pane" in res.stderr, res.stderr -- 2.54.0 From feceddf49fad8526dc7eaf87a1de006779b408bd Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:32:49 +0000 Subject: [PATCH 05/14] docs(issue-45): isolation and resume notes; block inventory line shifts Co-Authored-By: Claude Opus 5.5 --- .../skills/multi-agent-mux-create/SKILL.md | 2 ++ .../skills/multi-agent-mux-resume/SKILL.md | 2 +- tests/data/skill_code_blocks.json | 22 +++++++++---------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index 0efaa3a..d957d36 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -117,6 +117,8 @@ Why use `--herdr-session`? - By default, all skills target `default` herdr session socket — fine for single-workspace use. - By using an isolated session via `HERDR_SESSION_NAME` (or `--herdr-session`), your agent sessions are completely separated from your default user workspace, ensuring 0% interference — this is backed by a genuinely separate `herdr` session/socket, not merely a workspace label. - To deliberately tear down an *entire* isolated group at once (all its workspaces and agents), use `herdr session stop ` followed by `herdr session delete ` — this only affects that named session, never the default one. +- An isolated server MAM starts does not relaunch saved agents when it restarts (#45): it runs with `HERDR_CONFIG_PATH=/mam-server-config.toml`, your herdr config with `[session] resume_agents_on_restore = false`. Its saved panes come back as empty shells; resume closes the one of the session it resumes. +- That file is rebuilt each time MAM starts the server, so a change to your herdr config reaches a MAM server only when it is next started; `herdr server reload-config` rereads the generated file. A server you start yourself (`herdr session attach`) does not get it. --- diff --git a/.agents/skills/multi-agent-mux-resume/SKILL.md b/.agents/skills/multi-agent-mux-resume/SKILL.md index 6786c9d..6342843 100644 --- a/.agents/skills/multi-agent-mux-resume/SKILL.md +++ b/.agents/skills/multi-agent-mux-resume/SKILL.md @@ -60,7 +60,7 @@ If both are empty → for Class A agents (`agy`, `hermes`, `opencode`, `codex`), bash .agents/skills/multi-agent-mux-resume/scripts/resume_session.sh --workspace --agent --session ``` -`resume_session.sh` does the whole flow: original-cwd check, control-plane binding, session-id resolution (`resolve_session_id.sh`), the herdr-alive check, the agent command (adapter `resume-spec`, or `spawn-spec` for a Class A fresh spawn), spawn, start-up dialogs, and the YAML update (`update_yaml_resumed.sh`, which also publishes a `progress "resumed"` event when the row has a `delegate_job_id`). It never attaches; attaching is a separate step that names the server, because the server the script exports does not reach the calling shell and the native `herdr` binary does not route on environment variables: `herdr --session agent attach ` (`` = the `--herdr-session` you passed, or the row's `herdr_session`). +`resume_session.sh` does the whole flow: original-cwd check, control-plane binding, session-id resolution (`resolve_session_id.sh`), the herdr-alive check, the agent command (adapter `resume-spec`, or `spawn-spec` for a Class A fresh spawn), spawn (just before it, the session's own empty shell left by a server restart is closed: same session id, no agent, nothing running; `lib_py/herdr_restore.py`), start-up dialogs, and the YAML update (`update_yaml_resumed.sh`, which also publishes a `progress "resumed"` event when the row has a `delegate_job_id`). It never attaches; attaching is a separate step that names the server, because the server the script exports does not reach the calling shell and the native `herdr` binary does not route on environment variables: `herdr --session agent attach ` (`` = the `--herdr-session` you passed, or the row's `herdr_session`). | Option | Meaning | |---|---| diff --git a/tests/data/skill_code_blocks.json b/tests/data/skill_code_blocks.json index 4c91a05..7081540 100644 --- a/tests/data/skill_code_blocks.json +++ b/tests/data/skill_code_blocks.json @@ -60,8 +60,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 127, - "end": 145, + "start": 129, + "end": 147, "lang": "yaml", "sha": "2dd608d04ae0", "class": "c", @@ -74,8 +74,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 151, - "end": 153, + "start": 153, + "end": 155, "lang": "bash", "sha": "613003708f19", "class": "c", @@ -88,8 +88,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 191, - "end": 195, + "start": 193, + "end": 197, "lang": "bash", "sha": "38836d31bf38", "class": "c", @@ -103,8 +103,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 217, - "end": 239, + "start": 219, + "end": 241, "lang": "bash", "sha": "d90cfd2555fd", "class": "b", @@ -478,9 +478,9 @@ "multi-agent-mux-resume/scripts/resume_session.sh:133-144", "multi-agent-mux-resume/scripts/resume_session.sh:147-158", "multi-agent-mux-resume/scripts/resume_session.sh:172-197", - "multi-agent-mux-resume/scripts/resume_session.sh:237", - "multi-agent-mux-resume/scripts/resume_session.sh:241", - "multi-agent-mux-resume/scripts/resume_session.sh:247-250", + "multi-agent-mux-resume/scripts/resume_session.sh:245", + "multi-agent-mux-resume/scripts/resume_session.sh:249", + "multi-agent-mux-resume/scripts/resume_session.sh:255-258", "lib_py/agents/adapters/claude.py:109" ], "mismatch": [ -- 2.54.0 From eedcebcc1280017451e2be6c1237a9a14e7de676 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:34:27 +0000 Subject: [PATCH 06/14] fix(issue-45): restore stays off when python3 has no tomllib The shim runs a bare python3. Before 3.11 the module failed to import and the server started with restore on; now it gets the key alone, with a warning. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/herdr_config.py | 9 ++++++++- tests/test_issue45_herdr_config.py | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.agents/skills/lib_py/herdr_config.py b/.agents/skills/lib_py/herdr_config.py index 69f1500..1d20f55 100644 --- a/.agents/skills/lib_py/herdr_config.py +++ b/.agents/skills/lib_py/herdr_config.py @@ -32,7 +32,6 @@ import os import re import sys import tempfile -import tomllib from typing import List, Mapping, Optional, Tuple MARKER = "# generated by multi-agent-mux (Issue #45) - do not edit" @@ -42,6 +41,11 @@ FALLBACK_NAME = "herdr-server-config.toml" KEY = "resume_agents_on_restore" MINIMAL = f"[session]\n{KEY} = false\n" +try: + import tomllib +except ImportError: # Python < 3.11: no parser to check a merge with + tomllib = None + _SESSION_HEADER = re.compile(r"^\s*\[\s*session\s*\]\s*(#.*)?$") _ANY_HEADER = re.compile(r"^\s*\[") _KEY_LINE = re.compile(rf"^\s*{KEY}\s*=") @@ -107,6 +111,9 @@ def server_config_text(user_text: Optional[str]) -> Tuple[str, List[str]]: always has session.resume_agents_on_restore = false.""" if not user_text or not user_text.strip(): return MINIMAL, [] + if tomllib is None: + return MINIMAL, [f"this Python has no tomllib (3.11+); the MAM server uses only {KEY} = false, " + "without the rest of your settings"] try: user = tomllib.loads(user_text) except tomllib.TOMLDecodeError as e: diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py index add63e4..3ee1429 100644 --- a/tests/test_issue45_herdr_config.py +++ b/tests/test_issue45_herdr_config.py @@ -71,6 +71,14 @@ def test_a_bad_value_type_falls_back_to_minimal(): assert warnings == [] +def test_without_tomllib_restore_is_still_off(monkeypatch): + # The shim runs a bare `python3`; on one older than 3.11 an import error + # would leave the server with restore on. Off wins over the user's settings. + monkeypatch.setattr(hc, "tomllib", None) + text, warnings = hc.server_config_text("[ui]\nconfirm_close = false\n") + assert text == hc.MINIMAL and "no tomllib" in warnings[0] + + def _env(tmp_path, **kw): env = {"HOME": str(tmp_path / "home")} env.update(kw) -- 2.54.0 From ce91cc11b9be93b4ce654c15d45fd067ba769ba3 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 10:53:14 +0000 Subject: [PATCH 07/14] release(v4.8.5): bump framework and eight skills to 4.8.5 in lockstep Issue #45 A+B: MAM's isolated herdr servers start with restore-on-restart off, and resume closes the session's own restored empty shell. Not reviewed yet. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib.sh | 2 +- .agents/skills/lib_py/loop/eager.py | 2 +- .../skills/multi-agent-mux-create/SKILL.md | 2 +- .../multi-agent-mux-delegate-job/SKILL.md | 2 +- .agents/skills/multi-agent-mux-loop/SKILL.md | 2 +- .../skills/multi-agent-mux-monitor/SKILL.md | 2 +- .../multi-agent-mux-orc-onboard/SKILL.md | 2 +- .../skills/multi-agent-mux-resume/SKILL.md | 2 +- .../skills/multi-agent-mux-status/SKILL.md | 2 +- .agents/skills/multi-agent-mux-stop/SKILL.md | 2 +- BOOTSTRAP.ko.md | 2 +- BOOTSTRAP.md | 2 +- MESSAGING.md | 2 +- README.ko.md | 2 +- README.md | 2 +- VERSIONS.md | 36 +++++++++++++------ 16 files changed, 41 insertions(+), 25 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 4cd9a2c..35959f8 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -29,7 +29,7 @@ export WORKSPACE_ROOT # Framework semantic version. Single runtime source of truth; kept in lockstep # with VERSIONS.md and the 8 SKILL.md frontmatters by tests/test_version_consistency.py. # NOTE: unlike other MAM_* variables this one is intentionally NOT env-overridable. -MAM_VERSION="4.8.4" +MAM_VERSION="4.8.5" export MAM_VERSION AGENT_SESSIONS_YAML="${AGENT_SESSIONS_YAML:-$WORKSPACE_ROOT/.mam/agent-sessions.yaml}" diff --git a/.agents/skills/lib_py/loop/eager.py b/.agents/skills/lib_py/loop/eager.py index 947c72b..0c56c7b 100644 --- a/.agents/skills/lib_py/loop/eager.py +++ b/.agents/skills/lib_py/loop/eager.py @@ -13,7 +13,7 @@ import sys from typing import List, Sequence # Framework version locked with framework constants -FRAMEWORK_VERSION = "4.8.4" +FRAMEWORK_VERSION = "4.8.5" CORE_MODULES: Sequence[str] = ( # Standard library modules diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index d957d36..c97e5ed 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-create description: "Create a new agent session (claude, antigravity/agy) in a dedicated herdr session for context-preserving long-running work. Always creates a herdr session — never backgrounds with nohup/disown. Writes the new session to .mam/agent-sessions.yaml. Use when you want to start a fresh agent (no prior UUID) for a new project workspace." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md index 833e157..a29e3a5 100644 --- a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md +++ b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-delegate-job description: "Delegate a unit of work to any autonomous agent (claude-code, hermes, agy, grok-build, codex, or a human) and observe it asynchronously over an MQTT event channel. Supported roles include orchestrator, worker, and reviewer." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos, windows] diff --git a/.agents/skills/multi-agent-mux-loop/SKILL.md b/.agents/skills/multi-agent-mux-loop/SKILL.md index f802f24..f28c052 100644 --- a/.agents/skills/multi-agent-mux-loop/SKILL.md +++ b/.agents/skills/multi-agent-mux-loop/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-loop description: "Run an autonomous planning-execution-review loop using multiple agents (Planner, Creator, Reviewers) in the workspace. Automatically orchestrates plan discussion, code changes, and peer reviews until a unanimous PASS is achieved or the maximum iteration limit is reached." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index e1f085d..bc2fb5c 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-monitor description: "Run a long-lived reconciler that watches .mam/agent-sessions.yaml against the actual herdr/agent runtime state and reconciles them. Use when you want live visibility into which agent sessions are running, which are dead, which have stale YAML entries, and which have new session ids that haven't been recorded yet. Runs as a persistent loop (`reconcile.sh --subscribe`) that keeps going until it times out, idles out, or is interrupted." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md index e676cc8..8f87ccb 100644 --- a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md +++ b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-orc-onboard description: "Register current or specified orchestrator session UUID into agent-sessions.yaml orchestrator_uuids list to prevent sub-agent discovery capture." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-resume/SKILL.md b/.agents/skills/multi-agent-mux-resume/SKILL.md index 6342843..555e3a5 100644 --- a/.agents/skills/multi-agent-mux-resume/SKILL.md +++ b/.agents/skills/multi-agent-mux-resume/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-resume description: "Resume an existing agent (claude, antigravity/agy) conversation by UUID into a herdr session. Reads .mam/agent-sessions.yaml for the saved session/conversation id, spawns (or reuses) a herdr session of the matching name, and runs `claude -r ` or `agy --conversation ` inside. Use when you want to reattach to a previous session's context, or revive a session whose herdr died but the agent's conversation is still on disk." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-status/SKILL.md b/.agents/skills/multi-agent-mux-status/SKILL.md index 228490f..10cf52d 100644 --- a/.agents/skills/multi-agent-mux-status/SKILL.md +++ b/.agents/skills/multi-agent-mux-status/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-status description: "Read-only instant snapshot of all agent herdr sessions — name, YAML status, herdr alive, pane cmd/cwd, resume UUID on disk, and any drift. No mutation. Reuses reconcile.sh --dry-run for the diff logic. Use when you want to know 'what's running RIGHT NOW' without spinning up the monitor loop." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-stop/SKILL.md b/.agents/skills/multi-agent-mux-stop/SKILL.md index 5fc0b4c..3120fdd 100644 --- a/.agents/skills/multi-agent-mux-stop/SKILL.md +++ b/.agents/skills/multi-agent-mux-stop/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-stop description: "Stop an agent herdr session (claude, antigravity/agy) and update .mam/agent-sessions.yaml. Default stops gracefully and marks status=stopped with conversation preserved for resume. Does NOT delete on-disk conversation artifacts (jsonl/db) — those are preserved unless --purge-conversation is passed. Use when ending a work session, switching to a different one, or cleaning up before a fresh start." -version: 4.8.4 +version: 4.8.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/BOOTSTRAP.ko.md b/BOOTSTRAP.ko.md index f6d7930..2baceb9 100644 --- a/BOOTSTRAP.ko.md +++ b/BOOTSTRAP.ko.md @@ -2,7 +2,7 @@ 본 문서는 `herdr_agent_orchestration` 오케스트레이션 및 메시징 백플레인 워크플로우를 새로운 프로젝트에 도입하여 이식하고, 새로운 개발자/에이전트가 초기 가동을 시작할 때 수행해야 하는 환경 설정 및 구축 절차를 안내합니다. -[![Version: v4.8.4](https://img.shields.io/badge/version-v4.8.4-blue.svg)](VERSIONS.md) +[![Version: v4.8.5](https://img.shields.io/badge/version-v4.8.5-blue.svg)](VERSIONS.md) 새로운 에이전트는 이 안내서에 기술된 절차를 순차적으로 실행하여 초기 환경을 안정적으로 설정할 수 있습니다. diff --git a/BOOTSTRAP.md b/BOOTSTRAP.md index 08a589d..ddb5aed 100644 --- a/BOOTSTRAP.md +++ b/BOOTSTRAP.md @@ -2,7 +2,7 @@ This document guides you through the setup and initialization procedures required to adopt the `herdr_agent_orchestration` orchestration and messaging backplane workflow in a new project, enabling a new developer or agent to get up and running quickly. -[![Version: v4.8.4](https://img.shields.io/badge/version-v4.8.4-blue.svg)](VERSIONS.md) +[![Version: v4.8.5](https://img.shields.io/badge/version-v4.8.5-blue.svg)](VERSIONS.md) A new agent can follow the steps in this guide sequentially to establish a stable and reliable initial environment. diff --git a/MESSAGING.md b/MESSAGING.md index e364621..da2e1f6 100644 --- a/MESSAGING.md +++ b/MESSAGING.md @@ -1,6 +1,6 @@ # Messaging System Technical Analysis & Architecture Report -Applies to: MAM v4.8.4 +Applies to: MAM v4.8.5 This report provides a comprehensive, deep-dive analysis of the messaging system implemented in the `multi-agent-mux-delegate-job` skill. It covers the MQTT broker architecture, event protocols, job lifecycles, codebase internals, cross-system integration, and a list of known limitations along with production recommendations. diff --git a/README.ko.md b/README.ko.md index 1ba2720..aef6295 100644 --- a/README.ko.md +++ b/README.ko.md @@ -2,7 +2,7 @@ Herdr와 MQTT 브로커를 기반으로 구축된 고신뢰성 **다중 에이전트 오케스트레이션 및 메시징 백플레인** 프레임워크입니다. `claude`, `agy`, `hermes`, `grok`, `opencode`, `codex` 등 다중 LLM 백엔드 에이전트 전반에 걸쳐 장시간 수행되는 작업(코드 생성, 리팩토링, 보안 검토 등)을 조정, 격리 및 감사할 수 있도록 설계되었습니다. -[![Version: v4.8.4](https://img.shields.io/badge/version-v4.8.4-blue.svg)](VERSIONS.md) +[![Version: v4.8.5](https://img.shields.io/badge/version-v4.8.5-blue.svg)](VERSIONS.md) --- diff --git a/README.md b/README.md index df04b27..e0fc2c2 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ An advanced, high-reliability **Multi-Agent Orchestration & Messaging Backplane** framework built on Herdr and MQTT. It is designed to coordinate, isolate, and audit long-running agent tasks (such as code generation, refactoring, and security reviews) across multiple LLM backend clients (`claude`, `agy`, `hermes`, `grok`, `opencode`, `codex`). -[![Version: v4.8.4](https://img.shields.io/badge/version-v4.8.4-blue.svg)](VERSIONS.md) +[![Version: v4.8.5](https://img.shields.io/badge/version-v4.8.5-blue.svg)](VERSIONS.md) --- diff --git a/VERSIONS.md b/VERSIONS.md index 87610f8..2390a43 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -6,7 +6,7 @@ ## 📌 현재 버전 개요 (Current Release) -- **프레임워크 버전**: `v4.8.4` +- **프레임워크 버전**: `v4.8.5` - **최신 릴리스 일시**: 2026-10-04 (KST) - **기준 브랜치**: `main` - **핵심 아키텍처**: @@ -24,23 +24,39 @@ ## 🧭 스킬 패키지 버전 매트릭스 (Skills Version Matrix) -모든 8개 스킬은 YAML frontmatter 메타데이터(`author`, `version`, `platforms`, `environments`) 표준화를 통해 `v4.8.4`로 동기화되어 배포됩니다. +모든 8개 스킬은 YAML frontmatter 메타데이터(`author`, `version`, `platforms`, `environments`) 표준화를 통해 `v4.8.5`로 동기화되어 배포됩니다. | 스킬명 | 버전 | 역할 및 주요 책임 | 상태 | | :--- | :---: | :--- | :---: | -| **`multi-agent-mux-create`** | `4.8.4` | 에이전트 세션 신규 생성 및 Herdr 컨테이너 격리 스폰 | ✅ 배포 | -| **`multi-agent-mux-stop`** | `4.8.4` | 대화 UUID 원자적 캡처 및 세션 안전 종료 (Graceful Stop) | ✅ 배포 | -| **`multi-agent-mux-resume`** | `4.8.4` | 온디스크 대화 컨텍스트 기반 Tier-1 초고속 세션 복원 | ✅ 배포 | -| **`multi-agent-mux-status`** | `4.8.4` | 실시간 Herdr 세션 및 레지스트리 드리프트 스냅샷 조회 | ✅ 배포 | -| **`multi-agent-mux-monitor`** | `4.8.4` | YAML ↔ 런타임 상태 간 자율 조정자 (Reconciler Loop) | ✅ 배포 | -| **`multi-agent-mux-delegate-job`** | `4.8.4` | MQTT 이벤트 채널 기반 비동기 단위 작업 위임 | ✅ 배포 | -| **`multi-agent-mux-loop`** | `4.8.4` | Planner-Creator-Reviewer 3자 자율 계획·실행·피어리뷰 루프 | ✅ 배포 | -| **`multi-agent-mux-orc-onboard`** | `4.8.4` | 오케스트레이터 UUID 격리 등록 및 서브 세션 오염 방지 | ✅ 배포 | +| **`multi-agent-mux-create`** | `4.8.5` | 에이전트 세션 신규 생성 및 Herdr 컨테이너 격리 스폰 | ✅ 배포 | +| **`multi-agent-mux-stop`** | `4.8.5` | 대화 UUID 원자적 캡처 및 세션 안전 종료 (Graceful Stop) | ✅ 배포 | +| **`multi-agent-mux-resume`** | `4.8.5` | 온디스크 대화 컨텍스트 기반 Tier-1 초고속 세션 복원 | ✅ 배포 | +| **`multi-agent-mux-status`** | `4.8.5` | 실시간 Herdr 세션 및 레지스트리 드리프트 스냅샷 조회 | ✅ 배포 | +| **`multi-agent-mux-monitor`** | `4.8.5` | YAML ↔ 런타임 상태 간 자율 조정자 (Reconciler Loop) | ✅ 배포 | +| **`multi-agent-mux-delegate-job`** | `4.8.5` | MQTT 이벤트 채널 기반 비동기 단위 작업 위임 | ✅ 배포 | +| **`multi-agent-mux-loop`** | `4.8.5` | Planner-Creator-Reviewer 3자 자율 계획·실행·피어리뷰 루프 | ✅ 배포 | +| **`multi-agent-mux-orc-onboard`** | `4.8.5` | 오케스트레이터 UUID 격리 등록 및 서브 세션 오염 방지 | ✅ 배포 | --- ## 📋 버전별 상세 변경 내역 (Changelog) +### 🚀 `v4.8.5` — Issue #45 A·B: 재기동된 격리 서버가 저장된 에이전트를 다시 띄우지 않고, resume 이 복원된 빈 셸을 닫는다 (2026-10-04) + +> **패치 (PATCH Release)**: 격리 herdr 서버가 에이전트를 정지하지 않은 채 꺼졌다 다시 뜨면, herdr 가 저장 레이아웃을 복원하며 에이전트를 자기 재개 명령으로(권한 플래그 없이, 지운 에이전트까지) 다시 띄웠다. herdr 0.9.1 의 `[session] resume_agents_on_restore = false` 로 MAM 이 띄우는 서버에서 이를 끈다(A). 끈 뒤에도 pane 은 빈 셸로 복원되므로, resume 이 그 세션 자신의 빈 셸을 닫는다(B). 이슈의 제안 2·4 는 범위 밖. + +#### 🌟 주요 개선 사항 +1. **서버 설정 (`lib_py/herdr_config.py`)**: herdr 가 읽는 사용자 설정(`HERDR_CONFIG_PATH` → `XDG_CONFIG_HOME` → `~/.config`)에서 그 키만 false 로 바꾼 `mam-server-config.toml` 을 원본 옆에 만든다(상대 경로가 같게 풀린다). 사용자 파일은 고치지 않는다. 텍스트로 합친 뒤 다시 파싱해, 키가 false 이고 나머지가 원본과 같지 않으면 키 하나만 있는 파일로 떨어진다(깨진 설정이면 herdr 가 조용히 복원을 켜기 때문). MAM pane 이 물려받은 생성 파일은 기록된 원본으로 거슬러 올라간다. +2. **shim bootstrap (`lib.sh` heredoc)**: 서버를 띄울 때마다 설정을 다시 만들어 `HERDR_CONFIG_PATH` 로 넘긴다. 못 만들면 경고하고 예전처럼(복원 켜짐) 띄운다. 기본 `default` 서버와 사용자가 직접 띄운 서버는 바꾸지 않는다. +3. **복원된 빈 셸 (`lib_py/herdr_restore.py`, `resume_session.sh`)**: spawn 직전 한 번, `agent_session` 이 resume 하는 id 와 같고 live 에이전트가 없고 전경이 셸뿐인 pane 을 닫는다. 등록부는 보지 않는다(서버가 죽은 세션도 running 으로 남는다, Creator 이의 job 9f5f1ebe). `CP_PREFIX` 가 붙는 resume 이 재사용하지 못해 남던 빈 셸과 `$HOME` 으로 떨어진 빈 셸이 대상이다. +4. **S11 (`tests/test_issue23_24_startup_path.py`)**: 생성 shim 이 우연히 바뀌지 않게 하는 비교 기준을 heredoc 을 의도적으로 바꾼 ed73f67 로 옮겼다. + +#### ✅ 품질 검증 +- **실서버 테스트 (`tests/test_issue45_restore.py`, herdr 0.9.1)**: 짧은 일회용 HOME·세션과 가짜 claude 로, MAM bootstrap 경로에서 복원이 꺼짐(사용자 설정에 `[session]` 표가 있을 때 포함)과 같은 레이아웃을 일반 서버로 띄우면 `claude --resume` 이 뜨는 양성 대조, 닫기 조건, create → crash → resume 뒤 pane 하나만 남음을 확인한다. 수정을 끄면 A·B 테스트가 각각 실패함을 확인했다. +- **단위 (`tests/test_issue45_herdr_config.py`)**: 합치기 12 가지 경우와 실제 `herdr config check`, 원본 경로 규칙, 쓰기·폴백. +- **관련 스위트**: 새 두 파일 40 passed. issue9·shim contract·issue42·issue24 resume·uuid_target·issue21 prereqs·issue23_24 199 passed / 1 skipped(S11 기준 이동 후). tier1~4·issue21_phase2_loop_a 148 passed. 전체 스위트는 돌리지 않았다. +- **리뷰**: 전. + ### 🚀 `v4.8.4` — `.claude/skills` 링크 검사가 두 로컬 배치를 받고 깨진 링크를 잡는다 (2026-10-04, PR #47) > **패치 (PATCH Release)**: v4.8.3(PR #46)이 넣은 `test_claude_skills_link_resolves_to_agents_skills` 가 병합 후 본 체크아웃에서 실패했다. 테스트는 `.claude/skills` 를 `.agents/skills` 로 가는 링크 하나로 가정했지만, 이 로컬(git-ignored) 디렉터리는 스킬별 링크를 담은 디렉터리일 수도 있다. PR #46 의 리뷰·스위트는 워크트리에서 돌아 이 테스트가 skip 이었다. 테스트만 고친다. -- 2.54.0 From 811b0809669d49567a19f2dc8cc707488c33faee Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 11:17:59 +0000 Subject: [PATCH 08/14] fix(issue-45): review R1-R3 - herdr checks the config, any TOML spelling kept, source never overwritten R1: tomllib accepting a file is not herdr accepting it (a schema error makes herdr drop the whole file, restore included). The shim passes REAL_HERDR and the written file must pass `herdr config check` ("parse error"/"using defaults" reject; unknown keys and sound-file notices do not), else the key-only file is written and checked. A non-table `session` and a source that is not UTF-8 reach that fallback instead of raising. R2: the line edit is replaced by parse -> set -> serialize; the result must parse back to the source plus the key, so dotted keys and inline tables keep the user's other settings. R3: the output is never the source (aliases included) nor a file not generated for the same source; then a per-source name beside it. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib.sh | 7 +- .agents/skills/lib_py/herdr_config.py | 219 ++++++++++++++++++-------- tests/test_issue45_herdr_config.py | 136 +++++++++++++--- 3 files changed, 274 insertions(+), 88 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 35959f8..36efd5b 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -235,9 +235,10 @@ except Exception: # relaunch every saved agent with herdr's own resume command (no permission # flags, deleted agents included). It gets HERDR_CONFIG_PATH = the user's # herdr config with [session] resume_agents_on_restore = false, rebuilt on - # every start (lib_py/herdr_config.py). Without that file the server still - # starts - with restore on, as before. - MAM_SERVER_CFG=$(python3 -m lib_py.herdr_config server-config --fallback-dir "$_SHIM_DIR/.." || true) + # every start and checked by this herdr's `config check` + # (lib_py/herdr_config.py). Without that file the server still starts - + # with restore on, as before. + MAM_SERVER_CFG=$(python3 -m lib_py.herdr_config server-config --fallback-dir "$_SHIM_DIR/.." --herdr "$REAL_HERDR" || true) export MAM_SERVER_CFG if [ -z "$MAM_SERVER_CFG" ]; then echo "WARNING: restore-on-restart stays enabled for herdr session $_MAM_SESSION (no server config)" >&2 diff --git a/.agents/skills/lib_py/herdr_config.py b/.agents/skills/lib_py/herdr_config.py index 1d20f55..dc5469c 100644 --- a/.agents/skills/lib_py/herdr_config.py +++ b/.agents/skills/lib_py/herdr_config.py @@ -14,22 +14,30 @@ to false. The user's file is never modified. from the server) is followed back to the source recorded in that file. * Output: `mam-server-config.toml` next to the source, so relative paths in it (herdr resolves them against the config file's directory) keep their - meaning; `<.mam>/herdr-server-config.toml` only when that directory is not - writable. - * The merge edits text (stdlib has no TOML writer) and is then re-parsed: the - key must read false and everything else must equal the source. A merge that - fails the check falls back to a file holding only the key - herdr would - drop a broken file whole and restore again, silently. + meaning. Never the source itself and never a file this module did not + write for the same source: then a name with a hash of the source path. + `<.mam>/herdr-server-config.toml` only when the directory is not writable. + * The source is parsed, the key set, and the result serialized; the result + must parse back to exactly the source plus the key (any TOML spelling of + the key - header, dotted, inline - works). Then herdr's own `config check` + must accept the written file: herdr drops a file it cannot use whole + ("using defaults") and would restore again, silently. Anything that fails + falls back to a file holding only the key, which is checked the same way. -CLI: `python3 -m lib_py.herdr_config server-config [--fallback-dir DIR]` writes -the file and prints its path; warnings go to stderr; exit 1 when no file could -be written. +CLI: `python3 -m lib_py.herdr_config server-config [--fallback-dir DIR] +[--herdr PATH]` writes the file and prints its path; warnings go to stderr; +exit 1 when no usable file could be written. """ from __future__ import annotations import argparse +import datetime +import hashlib +import json +import math import os import re +import subprocess import sys import tempfile from typing import List, Mapping, Optional, Tuple @@ -40,27 +48,30 @@ OUTPUT_NAME = "mam-server-config.toml" FALLBACK_NAME = "herdr-server-config.toml" KEY = "resume_agents_on_restore" MINIMAL = f"[session]\n{KEY} = false\n" +LOSES_SETTINGS = f"the MAM server uses only {KEY} = false, without the rest of your settings" try: import tomllib except ImportError: # Python < 3.11: no parser to check a merge with tomllib = None -_SESSION_HEADER = re.compile(r"^\s*\[\s*session\s*\]\s*(#.*)?$") -_ANY_HEADER = re.compile(r"^\s*\[") -_KEY_LINE = re.compile(rf"^\s*{KEY}\s*=") +_BARE_KEY = re.compile(r"^[A-Za-z0-9_-]+$") +# Where a pane's server, socket and workspace come from; `config check` must not +# be steered by the caller's pane. +_SCOPE_ENV = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH") def _generated_source(path: str) -> Optional[str]: """The source recorded in a file generated here; '' when it is one without - a source line; None when it is not a generated file.""" + a source line; None when it is not a generated file (or unreadable).""" try: - with open(path, encoding="utf-8") as f: - if f.readline().rstrip("\n") != MARKER: - return None - line = f.readline().rstrip("\n") + with open(path, "rb") as f: + first, second = f.readline(), f.readline() except OSError: return None + if first.rstrip(b"\n").decode("utf-8", "replace") != MARKER: + return None + line = second.rstrip(b"\n").decode("utf-8", "replace") return line[len(SOURCE_PREFIX):] if line.startswith(SOURCE_PREFIX) else "" @@ -80,56 +91,116 @@ def user_config_path(env: Mapping[str, str]) -> str: return os.path.join(env.get("HOME", os.path.expanduser("~")), ".config", "herdr", "config.toml") -def _without_key(d: dict) -> dict: - d = dict(d) - session = dict(d.get("session") or {}) - session.pop(KEY, None) - if session: - d["session"] = session - else: - d.pop("session", None) - return d +# --- TOML writer for what tomllib returns ----------------------------------- + +def _str(s: str) -> str: + # json's escapes (\" \\ \b \f \n \r \t \uXXXX) are TOML's; DEL is not + # escaped by json but is not allowed raw in a TOML string. + return json.dumps(s, ensure_ascii=False).replace("\x7f", "\\u007f") -def _merge_text(user_text: str) -> str: - lines = user_text.splitlines() - header = next((i for i, l in enumerate(lines) if _SESSION_HEADER.match(l)), None) - if header is None: - sep = "" if user_text.endswith("\n") else "\n" - return f"{user_text}{sep}\n{MINIMAL}" - end = next((i for i in range(header + 1, len(lines)) if _ANY_HEADER.match(lines[i])), len(lines)) - key_line = next((i for i in range(header + 1, end) if _KEY_LINE.match(lines[i])), None) - if key_line is None: - lines.insert(header + 1, f"{KEY} = false") - else: - lines[key_line] = f"{KEY} = false" +def _key(k: str) -> str: + return k if _BARE_KEY.match(k) else _str(k) + + +def _value(v) -> str: + if isinstance(v, bool): + return "true" if v else "false" + if isinstance(v, int): + return str(v) + if isinstance(v, float): + if math.isnan(v): + return "nan" + if math.isinf(v): + return "inf" if v > 0 else "-inf" + return repr(v) + if isinstance(v, str): + return _str(v) + if isinstance(v, (datetime.datetime, datetime.date, datetime.time)): + return v.isoformat() + if isinstance(v, list): + return "[" + ", ".join(_value(x) for x in v) + "]" + if isinstance(v, dict): + return "{" + ", ".join(f"{_key(k)} = {_value(x)}" for k, x in v.items()) + "}" + raise TypeError(f"unexpected TOML value {type(v).__name__}") + + +def dumps(doc: dict) -> str: + """Top-level values, then one [table] per top-level table with its members + written inline - valid TOML for any document tomllib can return.""" + lines = [f"{_key(k)} = {_value(v)}" for k, v in doc.items() if not isinstance(v, dict)] + for k, v in doc.items(): + if isinstance(v, dict): + lines.append(f"[{_key(k)}]") + lines += [f"{_key(kk)} = {_value(vv)}" for kk, vv in v.items()] return "\n".join(lines) + "\n" def server_config_text(user_text: Optional[str]) -> Tuple[str, List[str]]: - """(the TOML body for the server, warnings). The body always parses and - always has session.resume_agents_on_restore = false.""" + """(the TOML body for the server, warnings). The body always parses, has + session.resume_agents_on_restore = false and otherwise equals the source - + or is MINIMAL, with a warning, when the source cannot be used.""" if not user_text or not user_text.strip(): return MINIMAL, [] if tomllib is None: - return MINIMAL, [f"this Python has no tomllib (3.11+); the MAM server uses only {KEY} = false, " - "without the rest of your settings"] + return MINIMAL, [f"this Python has no tomllib (3.11+); {LOSES_SETTINGS}"] try: user = tomllib.loads(user_text) except tomllib.TOMLDecodeError as e: - return MINIMAL, [f"herdr config does not parse ({e}); the MAM server uses only {KEY} = false"] - if (user.get("session") or {}).get(KEY) is False: - return user_text, [] - merged = _merge_text(user_text) + return MINIMAL, [f"herdr config does not parse ({e}); {LOSES_SETTINGS}"] + session = user.get("session", {}) + if not isinstance(session, dict): + # herdr rejects this file as a whole too. + return MINIMAL, [f"herdr config has a non-table `session`; {LOSES_SETTINGS}"] + expected = dict(user, session=dict(session, **{KEY: False})) try: - result = tomllib.loads(merged) - ok = result["session"][KEY] is False and _without_key(result) == _without_key(user) - except (tomllib.TOMLDecodeError, KeyError, TypeError): + text = dumps(expected) + ok = tomllib.loads(text) == expected + except (TypeError, tomllib.TOMLDecodeError): ok = False if not ok: - return MINIMAL, [f"could not set [session] {KEY} in the herdr config (dotted key or inline table?); " - f"the MAM server uses only {KEY} = false, without the rest of your settings"] - return merged, [] + return MINIMAL, [f"could not rewrite the herdr config faithfully; {LOSES_SETTINGS}"] + return text, [] + + +# --- herdr's own check ------------------------------------------------------ + +def herdr_rejects(path: str, herdr: str) -> Optional[str]: + """None when herdr would use the config at `path`, else herdr's diagnostics. + herdr 0.9.1 drops a file whole only on a parse or schema error, which it + reports as "config parse error: ...; using defaults"; other issues (an + unknown key, an unsupported sound file) cost only that one setting.""" + env = {k: v for k, v in os.environ.items() if k not in _SCOPE_ENV} + env["HERDR_CONFIG_PATH"] = path + res = subprocess.run([herdr, "config", "check"], env=env, capture_output=True, text=True, timeout=30) + out = (res.stdout + res.stderr).strip() + if res.returncode == 0 or not ("parse error" in out or "using defaults" in out): + return None + return out + + +# --- output ----------------------------------------------------------------- + +def _same_file(a: str, b: str) -> bool: + try: + return os.path.samefile(a, b) + except OSError: + return os.path.realpath(a) == os.path.realpath(b) + + +def output_path(source: str) -> str: + """Beside the source: OUTPUT_NAME, unless that is the source itself or a + file this module did not write for this source; then a per-source name.""" + d = os.path.dirname(source) + tag = hashlib.sha256(os.path.realpath(source).encode()).hexdigest()[:12] + for name in (OUTPUT_NAME, f"mam-server-config-{tag}.toml"): + target = os.path.join(d, name) + if _same_file(target, source): + continue + if os.path.lexists(target) and _generated_source(target) not in ("", source): + continue + return target + raise OSError(f"no free output name beside {source}") def _write_atomic(path: str, text: str, mode: int) -> None: @@ -149,7 +220,28 @@ def _write_atomic(path: str, text: str, mode: int) -> None: raise -def write_server_config(env: Mapping[str, str], fallback_dir: Optional[str] = None) -> Tuple[Optional[str], List[str]]: +def _write_checked(target: str, source: str, body: str, mode: int, herdr: Optional[str], + warnings: List[str]) -> bool: + """Write body (then MINIMAL if herdr rejects it); True when herdr accepts + what is on disk (or there is no herdr to ask).""" + for candidate in ((body, MINIMAL) if body != MINIMAL else (MINIMAL,)): + _write_atomic(target, f"{MARKER}\n{SOURCE_PREFIX}{source}\n{candidate}", mode) + if not herdr: + return True + try: + why = herdr_rejects(target, herdr) + except (OSError, subprocess.SubprocessError) as e: + warnings.append(f"cannot run `{herdr} config check` ({e}); the config is unchecked") + return True + if why is None: + return True + tail = "" if candidate == MINIMAL else f"; {LOSES_SETTINGS}" + warnings.append(f"herdr rejects the config built from {source}: {why}{tail}") + return False + + +def write_server_config(env: Mapping[str, str], fallback_dir: Optional[str] = None, + herdr: Optional[str] = None) -> Tuple[Optional[str], List[str]]: """Write the server config; (its path or None, warnings).""" source = user_config_path(env) warnings: List[str] = [] @@ -163,22 +255,22 @@ def write_server_config(env: Mapping[str, str], fallback_dir: Optional[str] = No pass except (OSError, UnicodeDecodeError) as e: # herdr cannot use it either, so it runs on defaults: same as no file. - warnings.append(f"cannot read herdr config {source}: {e}") + warnings.append(f"cannot read herdr config {source}: {e}; {LOSES_SETTINGS}") body, merge_warnings = server_config_text(user_text) warnings += merge_warnings - text = f"{MARKER}\n{SOURCE_PREFIX}{source}\n{body}" - target = os.path.join(os.path.dirname(source), OUTPUT_NAME) try: - _write_atomic(target, text, mode) - return target, warnings + target = output_path(source) + if _write_checked(target, source, body, mode, herdr, warnings): + return target, warnings + return None, warnings except OSError as e: - warnings.append(f"cannot write {target}: {e}") + warnings.append(f"cannot write beside {source}: {e}") if fallback_dir: target = os.path.join(fallback_dir, FALLBACK_NAME) try: - _write_atomic(target, text, mode) - warnings.append(f"wrote {target} instead; relative paths in the herdr config may not resolve") - return target, warnings + if _write_checked(target, source, body, mode, herdr, warnings): + warnings.append(f"wrote {target} instead; relative paths in the herdr config may not resolve") + return target, warnings except OSError as e: warnings.append(f"cannot write {target}: {e}") return None, warnings @@ -189,8 +281,9 @@ def main(argv=None) -> int: sub = p.add_subparsers(dest="cmd", required=True) sc = sub.add_parser("server-config", help="write the server config and print its path") sc.add_argument("--fallback-dir", default=None, help="where to write when the config dir is not writable") + sc.add_argument("--herdr", default=None, help="the real herdr binary, to `config check` the result") args = p.parse_args(argv) - path, warnings = write_server_config(os.environ, args.fallback_dir) + path, warnings = write_server_config(os.environ, args.fallback_dir, args.herdr) for w in warnings: print(f"WARNING: {w}", file=sys.stderr) if not path: diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py index 3ee1429..bb8a066 100644 --- a/tests/test_issue45_herdr_config.py +++ b/tests/test_issue45_herdr_config.py @@ -30,20 +30,32 @@ CASES = [ ("missing", None, True, False), ("empty", "", True, False), ("broken", "[session]\n[session]\n", False, True), + ("session_not_a_table", "session = 1\n[ui]\nconfirm_close = false\n", False, True), ("no_session", "[ui]\nconfirm_close = false\n", True, False), ("no_session_no_newline", "[ui]\nconfirm_close = false", True, False), ("session_without_key", "[session]\n# a comment\n\n[ui]\nconfirm_close = false\n", True, False), ("key_true", "[ui]\nconfirm_close = false\n[session]\nresume_agents_on_restore = true # mine\n", True, False), ("key_false", "[session]\nresume_agents_on_restore=false\n", True, False), ("sub_table_only", "[session.extra]\na = 1\n", True, False), - ("dotted_key", "session.resume_agents_on_restore = true\n", False, True), - ("inline_table", "session = { resume_agents_on_restore = true }\n", False, True), - ("header_inside_string", 'banner = """\n[session]\n"""\n', False, True), + # valid spellings a line edit cannot reach: the rest must survive (review R2) + ("dotted_key", "session.resume_agents_on_restore = true\n[ui]\nconfirm_close = false\n", True, False), + ("inline_table", "session = { resume_agents_on_restore = true }\n[ui]\nconfirm_close = false\n", True, False), + ("header_inside_string", 'banner = """\n[session]\n"""\n[ui]\nconfirm_close = false\n', True, False), + ("value_kinds", 'top = "a\\"b\\u007f\\u00e9"\n"odd key" = [1, 2.5, inf, true]\n' + 'when = 1979-05-27T07:32:00Z\n[ui.sound]\npath = "s/done.wav"\n' + '[[keys.bind]]\nkey = "a"\n[[keys.bind]]\nkey = "b"\n', True, False), ] def _without_key(d): - return hc._without_key(d) + d = dict(d) + session = dict(d.get("session") or {}) + session.pop("resume_agents_on_restore", None) + if session: + d["session"] = session + else: + d.pop("session", None) + return d @pytest.mark.parametrize("name,user,kept,warned", CASES, ids=[c[0] for c in CASES]) @@ -58,17 +70,10 @@ def test_merge_always_parses_with_restore_off(name, user, kept, warned): assert text == hc.MINIMAL -def test_existing_false_is_left_byte_for_byte(): - user = "[session]\nresume_agents_on_restore=false # keep\n" - assert hc.server_config_text(user) == (user, []) - - -def test_a_bad_value_type_falls_back_to_minimal(): - # herdr rejects a non-bool here as a parse error of the whole file. +def test_a_non_bool_restore_value_is_replaced(): + # herdr would reject the whole file for this; the key is ours to set anyway. text, warnings = hc.server_config_text('[session]\nresume_agents_on_restore = "no"\n') - assert tomllib.loads(text)["session"]["resume_agents_on_restore"] is False - # tomllib reads the string fine, so the merge replaces the line instead. - assert warnings == [] + assert tomllib.loads(text) == {"session": {"resume_agents_on_restore": False}} and warnings == [] def test_without_tomllib_restore_is_still_off(monkeypatch): @@ -187,14 +192,101 @@ def test_cli_prints_the_path(tmp_path): @pytest.mark.skipif(not HERDR, reason="herdr is not installed") @pytest.mark.parametrize("name,user,kept,warned", CASES, ids=[c[0] for c in CASES]) def test_real_herdr_accepts_every_result(tmp_path, name, user, kept, warned): + if kept and user: # a source herdr itself takes must stay one it takes + src = tmp_path / "source.toml" + src.write_text(user) + assert hc.herdr_rejects(str(src), HERDR) is None text, _ = hc.server_config_text(user) cfg = tmp_path / "server.toml" cfg.write_text(hc.MARKER + "\n" + text) - env = {k: v for k, v in os.environ.items() if not k.startswith("HERDR_")} - env.update(HOME=str(tmp_path), HERDR_CONFIG_PATH=str(cfg)) - res = subprocess.run([HERDR, "config", "check"], capture_output=True, text=True, env=env) - out = res.stdout + res.stderr - assert "parse error" not in out, out - # the fixtures use only keys herdr knows, apart from the made-up ones below - issues = [l for l in out.splitlines()[1:] if l.strip()] - assert all(l.startswith("unknown config key") and ("extra" in l or "banner" in l) for l in issues), out + assert hc.herdr_rejects(str(cfg), HERDR) is None + + +@pytest.mark.skipif(not HERDR, reason="herdr is not installed") +def test_herdr_rejects_tells_schema_errors_from_harmless_issues(tmp_path): + bad = tmp_path / "bad.toml" + bad.write_text('[ui]\nconfirm_close = "no"\n') + assert "using defaults" in hc.herdr_rejects(str(bad), HERDR) + soft = tmp_path / "soft.toml" + soft.write_text('made_up = 1\n[ui.sound]\npath = "x.wav"\n') + assert hc.herdr_rejects(str(soft), HERDR) is None + + +def _cli(tmp_path, source: Path, *extra) -> subprocess.CompletedProcess: + env = {k: v for k, v in os.environ.items() if not k.startswith("HERDR_") and k != "XDG_CONFIG_HOME"} + env.update(HOME=str(tmp_path), HERDR_CONFIG_PATH=str(source), + PYTHONPATH=str(REPO_ROOT / ".agents" / "skills")) + return subprocess.run([sys.executable, "-m", "lib_py.herdr_config", "server-config", *extra], + capture_output=True, text=True, env=env) + + +@pytest.mark.skipif(not HERDR, reason="herdr is not installed") +@pytest.mark.parametrize("body,kept", [ + (b"[ui]\nconfirm_close = false\n", True), # control: valid, kept + (b'[ui]\nconfirm_close = "no"\n', False), # tomllib yes, herdr's schema no (review R1) + (b"session = 1\n[ui]\nconfirm_close = false\n", False), # non-table session (R1) + (b"[ui]\nconfirm_close = false\n# \xff\xfe\n", False), # not UTF-8 (R1) +], ids=["valid", "schema-error", "session-scalar", "invalid-utf8"]) +def test_cli_always_hands_herdr_a_restore_off_config_it_accepts(tmp_path, body, kept): + source = tmp_path / "custom.toml" + source.write_bytes(body) + res = _cli(tmp_path, source, "--herdr", HERDR) + assert res.returncode == 0, res.stderr + out = Path(res.stdout.strip()) + assert out.parent == tmp_path and out != source + assert hc.herdr_rejects(str(out), HERDR) is None + parsed = tomllib.loads(out.read_text()) + assert parsed["session"]["resume_agents_on_restore"] is False + if kept: + assert parsed["ui"]["confirm_close"] is False and res.stderr == "" + else: + assert parsed == {"session": {"resume_agents_on_restore": False}} and "WARNING" in res.stderr + assert source.read_bytes() == body + + +@pytest.mark.skipif(not HERDR, reason="herdr is not installed") +def test_a_config_herdr_rejects_is_never_left_in_place(tmp_path): + # the merge itself is fine for tomllib; only herdr's schema says no + source = tmp_path / "config.toml" + source.write_text('[ui]\nconfirm_close = "no"\n[session]\nresume_agents_on_restore = true\n') + path, warnings = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}, herdr=HERDR) + assert tomllib.loads(Path(path).read_text()) == {"session": {"resume_agents_on_restore": False}} + assert any("herdr rejects" in w for w in warnings), warnings + + +# --- the user's file is never the output (review R3) ----------------------- + +def test_source_named_like_the_output_is_not_overwritten(tmp_path): + source = tmp_path / hc.OUTPUT_NAME + source.write_bytes(b"[ui]\nconfirm_close = false\n") + path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}) + assert source.read_bytes() == b"[ui]\nconfirm_close = false\n" + assert Path(path).parent == tmp_path and Path(path) != source + assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + # and the next start finds the same file again + assert hc.write_server_config({"HERDR_CONFIG_PATH": str(source)})[0] == path + + +def test_an_alias_of_the_source_is_not_overwritten(tmp_path): + source = tmp_path / "config.toml" + source.write_bytes(b"[ui]\nconfirm_close = false\n") + (tmp_path / hc.OUTPUT_NAME).symlink_to(source) + path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}) + assert source.read_bytes() == b"[ui]\nconfirm_close = false\n" + assert os.path.islink(tmp_path / hc.OUTPUT_NAME) + assert not os.path.samefile(path, source) + + +def test_a_foreign_file_with_the_output_name_is_kept(tmp_path): + source = tmp_path / "config.toml" + source.write_text("[ui]\nconfirm_close = false\n") + foreign = tmp_path / hc.OUTPUT_NAME + foreign.write_bytes(b"# someone else's\n") + path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}) + assert foreign.read_bytes() == b"# someone else's\n" and Path(path) != foreign + # a file generated for another source is someone else's too + other = tmp_path / "other.toml" + other.write_text("") + other_path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(other)}) + assert other_path not in (path, str(foreign)) + assert hc.user_config_path({"HERDR_CONFIG_PATH": path}) == str(source) -- 2.54.0 From 7c544c06ab55441d2f14eeef831e9410815f1c16 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 11:18:09 +0000 Subject: [PATCH 09/14] test(issue-45): S11 base follows the review-R1 heredoc change; VERSIONS notes Co-Authored-By: Claude Opus 5.5 --- VERSIONS.md | 4 ++-- tests/test_issue23_24_startup_path.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/VERSIONS.md b/VERSIONS.md index 2390a43..863e1f8 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -46,7 +46,7 @@ > **패치 (PATCH Release)**: 격리 herdr 서버가 에이전트를 정지하지 않은 채 꺼졌다 다시 뜨면, herdr 가 저장 레이아웃을 복원하며 에이전트를 자기 재개 명령으로(권한 플래그 없이, 지운 에이전트까지) 다시 띄웠다. herdr 0.9.1 의 `[session] resume_agents_on_restore = false` 로 MAM 이 띄우는 서버에서 이를 끈다(A). 끈 뒤에도 pane 은 빈 셸로 복원되므로, resume 이 그 세션 자신의 빈 셸을 닫는다(B). 이슈의 제안 2·4 는 범위 밖. #### 🌟 주요 개선 사항 -1. **서버 설정 (`lib_py/herdr_config.py`)**: herdr 가 읽는 사용자 설정(`HERDR_CONFIG_PATH` → `XDG_CONFIG_HOME` → `~/.config`)에서 그 키만 false 로 바꾼 `mam-server-config.toml` 을 원본 옆에 만든다(상대 경로가 같게 풀린다). 사용자 파일은 고치지 않는다. 텍스트로 합친 뒤 다시 파싱해, 키가 false 이고 나머지가 원본과 같지 않으면 키 하나만 있는 파일로 떨어진다(깨진 설정이면 herdr 가 조용히 복원을 켜기 때문). MAM pane 이 물려받은 생성 파일은 기록된 원본으로 거슬러 올라간다. +1. **서버 설정 (`lib_py/herdr_config.py`)**: herdr 가 읽는 사용자 설정(`HERDR_CONFIG_PATH` → `XDG_CONFIG_HOME` → `~/.config`)에서 그 키만 false 로 바꾼 `mam-server-config.toml` 을 원본 옆에 만든다(상대 경로가 같게 풀린다). 사용자 파일은 고치지 않는다: 출력이 원본 자신(별칭 포함)이거나 같은 원본으로 만든 파일이 아니면 원본별 이름을 쓴다. 파싱 → 키 설정 → 직렬화하고, 결과가 원본 + 키와 정확히 같아야 한다(헤더·점 표기·인라인 표 모두). 쓴 파일은 그 herdr 의 `config check` 를 통과해야 한다. herdr 는 파싱·스키마 오류면 파일 전체를 버리고("using defaults") 복원을 켜기 때문이다. 통과하지 못하면(깨짐, 비-bool 값, 표가 아닌 `session`, UTF-8 아님) 키 하나만 있는 파일을 쓰고 경고한다. MAM pane 이 물려받은 생성 파일은 기록된 원본으로 거슬러 올라간다. 2. **shim bootstrap (`lib.sh` heredoc)**: 서버를 띄울 때마다 설정을 다시 만들어 `HERDR_CONFIG_PATH` 로 넘긴다. 못 만들면 경고하고 예전처럼(복원 켜짐) 띄운다. 기본 `default` 서버와 사용자가 직접 띄운 서버는 바꾸지 않는다. 3. **복원된 빈 셸 (`lib_py/herdr_restore.py`, `resume_session.sh`)**: spawn 직전 한 번, `agent_session` 이 resume 하는 id 와 같고 live 에이전트가 없고 전경이 셸뿐인 pane 을 닫는다. 등록부는 보지 않는다(서버가 죽은 세션도 running 으로 남는다, Creator 이의 job 9f5f1ebe). `CP_PREFIX` 가 붙는 resume 이 재사용하지 못해 남던 빈 셸과 `$HOME` 으로 떨어진 빈 셸이 대상이다. 4. **S11 (`tests/test_issue23_24_startup_path.py`)**: 생성 shim 이 우연히 바뀌지 않게 하는 비교 기준을 heredoc 을 의도적으로 바꾼 ed73f67 로 옮겼다. @@ -55,7 +55,7 @@ - **실서버 테스트 (`tests/test_issue45_restore.py`, herdr 0.9.1)**: 짧은 일회용 HOME·세션과 가짜 claude 로, MAM bootstrap 경로에서 복원이 꺼짐(사용자 설정에 `[session]` 표가 있을 때 포함)과 같은 레이아웃을 일반 서버로 띄우면 `claude --resume` 이 뜨는 양성 대조, 닫기 조건, create → crash → resume 뒤 pane 하나만 남음을 확인한다. 수정을 끄면 A·B 테스트가 각각 실패함을 확인했다. - **단위 (`tests/test_issue45_herdr_config.py`)**: 합치기 12 가지 경우와 실제 `herdr config check`, 원본 경로 규칙, 쓰기·폴백. - **관련 스위트**: 새 두 파일 40 passed. issue9·shim contract·issue42·issue24 resume·uuid_target·issue21 prereqs·issue23_24 199 passed / 1 skipped(S11 기준 이동 후). tier1~4·issue21_phase2_loop_a 148 passed. 전체 스위트는 돌리지 않았다. -- **리뷰**: 전. +- **리뷰**: 1회차 codex NOT PASS(R1 herdr 스키마 검증 없음, R2 점 표기·인라인 표에서 설정 유실, R3 원본과 출력 이름 충돌 시 덮어씀). 811b080 에서 수정. ### 🚀 `v4.8.4` — `.claude/skills` 링크 검사가 두 로컬 배치를 받고 깨진 링크를 잡는다 (2026-10-04, PR #47) diff --git a/tests/test_issue23_24_startup_path.py b/tests/test_issue23_24_startup_path.py index 71addd3..24e5b44 100644 --- a/tests/test_issue23_24_startup_path.py +++ b/tests/test_issue23_24_startup_path.py @@ -297,9 +297,10 @@ def test_s11_this_branch_does_not_change_the_generated_shim(mam_sandbox): shared by every checkout bound to one control plane, so keeping it byte-identical to base is what makes "the control plane is not modified" true at runtime. Issue #45 changed the heredoc on purpose (the bootstrap - starts the server with restore-on-restart off), so the base is that commit.""" + starts the server with restore-on-restart off, and its review R1 passes + REAL_HERDR to the config check), so the base is that commit.""" repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - base = os.environ.get("MAM_BASE_SHA", "ed73f67") + base = os.environ.get("MAM_BASE_SHA", "811b080") show = subprocess.run(["git", "show", "%s:.agents/skills/lib.sh" % base], capture_output=True, text=True, cwd=repo) if show.returncode != 0: -- 2.54.0 From 35935f9e8a052ca9e9d9b806a62d17eb72afea78 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 4 Oct 2026 11:49:08 +0000 Subject: [PATCH 10/14] fix(issue-45): review round 2 R3 - the .mam fallback obeys the same no-overwrite rules The fallback wrote <.mam>/herdr-server-config.toml unchecked, so it could replace a foreign file there, the source itself, or a link to it. It now goes through output_path like the normal path (source identity incl. aliases, generated-for-this-source ownership, then a per-source name) and writes nothing when no name is safe. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/herdr_config.py | 23 +++++---- VERSIONS.md | 2 +- tests/test_issue45_herdr_config.py | 68 +++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/.agents/skills/lib_py/herdr_config.py b/.agents/skills/lib_py/herdr_config.py index dc5469c..e1012bb 100644 --- a/.agents/skills/lib_py/herdr_config.py +++ b/.agents/skills/lib_py/herdr_config.py @@ -14,9 +14,10 @@ to false. The user's file is never modified. from the server) is followed back to the source recorded in that file. * Output: `mam-server-config.toml` next to the source, so relative paths in it (herdr resolves them against the config file's directory) keep their - meaning. Never the source itself and never a file this module did not - write for the same source: then a name with a hash of the source path. - `<.mam>/herdr-server-config.toml` only when the directory is not writable. + meaning. `<.mam>/herdr-server-config.toml` only when that directory is not + writable. Either way never the source itself and never a file this module + did not write for the same source: then a name with a hash of the source + path, and no write at all when that is taken too. * The source is parsed, the key set, and the result serialized; the result must parse back to exactly the source plus the key (any TOML spelling of the key - header, dotted, inline - works). Then herdr's own `config check` @@ -188,19 +189,20 @@ def _same_file(a: str, b: str) -> bool: return os.path.realpath(a) == os.path.realpath(b) -def output_path(source: str) -> str: - """Beside the source: OUTPUT_NAME, unless that is the source itself or a - file this module did not write for this source; then a per-source name.""" - d = os.path.dirname(source) +def output_path(source: str, directory: Optional[str] = None, name: str = OUTPUT_NAME) -> str: + """`name` in `directory` (default: beside the source), unless that is the + source itself (aliases included) or a file this module did not write for + this source; then a per-source name. Every write goes through here.""" + d = directory or os.path.dirname(source) tag = hashlib.sha256(os.path.realpath(source).encode()).hexdigest()[:12] - for name in (OUTPUT_NAME, f"mam-server-config-{tag}.toml"): - target = os.path.join(d, name) + for n in (name, f"{name[:-len('.toml')]}-{tag}.toml"): + target = os.path.join(d, n) if _same_file(target, source): continue if os.path.lexists(target) and _generated_source(target) not in ("", source): continue return target - raise OSError(f"no free output name beside {source}") + raise OSError(f"no free output name in {d}") def _write_atomic(path: str, text: str, mode: int) -> None: @@ -268,6 +270,7 @@ def write_server_config(env: Mapping[str, str], fallback_dir: Optional[str] = No if fallback_dir: target = os.path.join(fallback_dir, FALLBACK_NAME) try: + target = output_path(source, fallback_dir, FALLBACK_NAME) if _write_checked(target, source, body, mode, herdr, warnings): warnings.append(f"wrote {target} instead; relative paths in the herdr config may not resolve") return target, warnings diff --git a/VERSIONS.md b/VERSIONS.md index 863e1f8..aedaf49 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -55,7 +55,7 @@ - **실서버 테스트 (`tests/test_issue45_restore.py`, herdr 0.9.1)**: 짧은 일회용 HOME·세션과 가짜 claude 로, MAM bootstrap 경로에서 복원이 꺼짐(사용자 설정에 `[session]` 표가 있을 때 포함)과 같은 레이아웃을 일반 서버로 띄우면 `claude --resume` 이 뜨는 양성 대조, 닫기 조건, create → crash → resume 뒤 pane 하나만 남음을 확인한다. 수정을 끄면 A·B 테스트가 각각 실패함을 확인했다. - **단위 (`tests/test_issue45_herdr_config.py`)**: 합치기 12 가지 경우와 실제 `herdr config check`, 원본 경로 규칙, 쓰기·폴백. - **관련 스위트**: 새 두 파일 40 passed. issue9·shim contract·issue42·issue24 resume·uuid_target·issue21 prereqs·issue23_24 199 passed / 1 skipped(S11 기준 이동 후). tier1~4·issue21_phase2_loop_a 148 passed. 전체 스위트는 돌리지 않았다. -- **리뷰**: 1회차 codex NOT PASS(R1 herdr 스키마 검증 없음, R2 점 표기·인라인 표에서 설정 유실, R3 원본과 출력 이름 충돌 시 덮어씀). 811b080 에서 수정. +- **리뷰**: 1회차 codex NOT PASS(R1 herdr 스키마 검증 없음, R2 점 표기·인라인 표에서 설정 유실, R3 원본과 출력 이름 충돌 시 덮어씀). 811b080 에서 수정. 2회차 codex NOT PASS(R3 의 폴백 경로는 같은 검사를 거치지 않음), 폴백도 같은 이름 선택을 거치도록 수정. ### 🚀 `v4.8.4` — `.claude/skills` 링크 검사가 두 로컬 배치를 받고 깨진 링크를 잡는다 (2026-10-04, PR #47) diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py index bb8a066..4e95973 100644 --- a/tests/test_issue45_herdr_config.py +++ b/tests/test_issue45_herdr_config.py @@ -170,11 +170,79 @@ def test_unwritable_config_dir_falls_back_to_mam_dir(tmp_path): assert path == str(mam / hc.FALLBACK_NAME) assert any("relative paths" in w for w in warnings), warnings assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + if HERDR: + assert hc.herdr_rejects(path, HERDR) is None assert hc.write_server_config(_env(tmp_path))[0] is None # no fallback given finally: cfg_dir.chmod(0o755) +# --- the fallback obeys the same rules (review round 2, R3) ---------------- + +def _tag(source: Path) -> str: + import hashlib + return hashlib.sha256(os.path.realpath(source).encode()).hexdigest()[:12] + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") +def test_a_foreign_fallback_file_is_kept(tmp_path): + cfg_dir = tmp_path / "cfg" + cfg_dir.mkdir() + source = cfg_dir / "config.toml" + source.write_text("[ui]\nconfirm_close = false\n") + cfg_dir.chmod(0o555) + mam = tmp_path / ".mam" + mam.mkdir() + foreign = mam / hc.FALLBACK_NAME + foreign.write_bytes(b"# user-owned\n[ui]\nconfirm_close = true\n") + try: + path, warnings = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}, str(mam), HERDR) + finally: + cfg_dir.chmod(0o755) + assert foreign.read_bytes() == b"# user-owned\n[ui]\nconfirm_close = true\n" + assert path == str(mam / f"herdr-server-config-{_tag(source)}.toml"), warnings + assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + + +@pytest.mark.parametrize("alias", [False, True], ids=["same-path", "symlink-alias"]) +def test_the_fallback_is_never_the_source(tmp_path, alias): + mam = tmp_path / ".mam" + mam.mkdir() + body = b"[ui]\nconfirm_close = false\n" + if alias: + source = tmp_path / "real.toml" + source.write_bytes(body) + (mam / hc.FALLBACK_NAME).symlink_to(source) + source_dir = tmp_path + else: + source = mam / hc.FALLBACK_NAME + source.write_bytes(body) + source_dir = mam + # both names beside the source are someone else's, so the fallback is used + for name in (hc.OUTPUT_NAME, f"mam-server-config-{_tag(source)}.toml"): + (source_dir / name).write_bytes(b"# foreign\n") + path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}, str(mam), HERDR) + assert source.read_bytes() == body + if alias: # os.replace would swap out the user's link itself + assert (mam / hc.FALLBACK_NAME).is_symlink() + assert path and not os.path.samefile(path, source) + assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False + + +def test_no_write_when_every_name_is_taken(tmp_path): + mam = tmp_path / ".mam" + mam.mkdir() + source = tmp_path / "config.toml" + source.write_text("[ui]\nconfirm_close = false\n") + taken = [tmp_path / hc.OUTPUT_NAME, tmp_path / f"mam-server-config-{_tag(source)}.toml", + mam / hc.FALLBACK_NAME, mam / f"herdr-server-config-{_tag(source)}.toml"] + for f in taken: + f.write_bytes(b"# foreign\n") + path, warnings = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}, str(mam)) + assert path is None and any("no free output name" in w for w in warnings), warnings + assert all(f.read_bytes() == b"# foreign\n" for f in taken) + + def test_cli_prints_the_path(tmp_path): env = dict(os.environ, HOME=str(tmp_path), PYTHONPATH=str(REPO_ROOT / ".agents" / "skills")) env.pop("HERDR_CONFIG_PATH", None) -- 2.54.0 From 36f0b8dfd60c67d5cf38f1c22eb9480a05e83df2 Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 5 Oct 2026 15:56:27 +0900 Subject: [PATCH 11/14] test(issue-45): the herdr-config tests collect on Python < 3.11; the docs say what that Python loses tests/test_issue45_herdr_config.py imported tomllib at module level. On Python 3.9 - macOS's /usr/bin/python3, which the venvs and the bootstrap's python3 use here - the import failed during collection and pytest ran no test of the whole suite. The import is optional now, and the 29 tests that parse a result with tomllib or expect the user's settings kept are marked needs_tomllib: on Python < 3.11 herdr_config deliberately writes the key-only config (restore still off, the other settings dropped, a warning on stderr), which test_without_tomllib_restore_is_still_off covers on every Python. Python 3.9: 22 passed / 29 skipped; Python 3.11: 51 passed. create SKILL.md (same line, so the #42 inventory is unchanged) and the 4.8.5 entry state that building the server config needs tomllib (Python 3.11+) in the python3 on PATH, and what an older python3 loses. Checked with Python 3.9 in a throwaway HOME: the warning reaches stderr (the bootstrap captures only stdout) and the user's file is untouched. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .../skills/multi-agent-mux-create/SKILL.md | 2 +- VERSIONS.md | 3 ++- tests/test_issue45_herdr_config.py | 24 ++++++++++++++++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index c97e5ed..5940282 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -117,7 +117,7 @@ Why use `--herdr-session`? - By default, all skills target `default` herdr session socket — fine for single-workspace use. - By using an isolated session via `HERDR_SESSION_NAME` (or `--herdr-session`), your agent sessions are completely separated from your default user workspace, ensuring 0% interference — this is backed by a genuinely separate `herdr` session/socket, not merely a workspace label. - To deliberately tear down an *entire* isolated group at once (all its workspaces and agents), use `herdr session stop ` followed by `herdr session delete ` — this only affects that named session, never the default one. -- An isolated server MAM starts does not relaunch saved agents when it restarts (#45): it runs with `HERDR_CONFIG_PATH=/mam-server-config.toml`, your herdr config with `[session] resume_agents_on_restore = false`. Its saved panes come back as empty shells; resume closes the one of the session it resumes. +- An isolated server MAM starts does not relaunch saved agents when it restarts (#45): it runs with `HERDR_CONFIG_PATH=/mam-server-config.toml`, your herdr config with `[session] resume_agents_on_restore = false`. Its saved panes come back as empty shells; resume closes the one of the session it resumes. Building that file needs `tomllib` (Python 3.11+) in the `python3` on PATH; with an older `python3` (macOS ships 3.9) restore is still off, but the file holds only that key, so the MAM server runs without your other herdr settings and the bootstrap prints a warning saying so. - That file is rebuilt each time MAM starts the server, so a change to your herdr config reaches a MAM server only when it is next started; `herdr server reload-config` rereads the generated file. A server you start yourself (`herdr session attach`) does not get it. --- diff --git a/VERSIONS.md b/VERSIONS.md index aedaf49..ca512f1 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -46,7 +46,7 @@ > **패치 (PATCH Release)**: 격리 herdr 서버가 에이전트를 정지하지 않은 채 꺼졌다 다시 뜨면, herdr 가 저장 레이아웃을 복원하며 에이전트를 자기 재개 명령으로(권한 플래그 없이, 지운 에이전트까지) 다시 띄웠다. herdr 0.9.1 의 `[session] resume_agents_on_restore = false` 로 MAM 이 띄우는 서버에서 이를 끈다(A). 끈 뒤에도 pane 은 빈 셸로 복원되므로, resume 이 그 세션 자신의 빈 셸을 닫는다(B). 이슈의 제안 2·4 는 범위 밖. #### 🌟 주요 개선 사항 -1. **서버 설정 (`lib_py/herdr_config.py`)**: herdr 가 읽는 사용자 설정(`HERDR_CONFIG_PATH` → `XDG_CONFIG_HOME` → `~/.config`)에서 그 키만 false 로 바꾼 `mam-server-config.toml` 을 원본 옆에 만든다(상대 경로가 같게 풀린다). 사용자 파일은 고치지 않는다: 출력이 원본 자신(별칭 포함)이거나 같은 원본으로 만든 파일이 아니면 원본별 이름을 쓴다. 파싱 → 키 설정 → 직렬화하고, 결과가 원본 + 키와 정확히 같아야 한다(헤더·점 표기·인라인 표 모두). 쓴 파일은 그 herdr 의 `config check` 를 통과해야 한다. herdr 는 파싱·스키마 오류면 파일 전체를 버리고("using defaults") 복원을 켜기 때문이다. 통과하지 못하면(깨짐, 비-bool 값, 표가 아닌 `session`, UTF-8 아님) 키 하나만 있는 파일을 쓰고 경고한다. MAM pane 이 물려받은 생성 파일은 기록된 원본으로 거슬러 올라간다. +1. **서버 설정 (`lib_py/herdr_config.py`)**: herdr 가 읽는 사용자 설정(`HERDR_CONFIG_PATH` → `XDG_CONFIG_HOME` → `~/.config`)에서 그 키만 false 로 바꾼 `mam-server-config.toml` 을 원본 옆에 만든다(상대 경로가 같게 풀린다). 사용자 파일은 고치지 않는다. 이 병합에는 `tomllib`(Python 3.11+)이 필요하다. PATH 의 `python3` 가 그보다 오래되면(macOS 기본 3.9) 복원은 꺼지지만 파일에 그 키만 담겨, MAM 서버는 사용자의 다른 herdr 설정 없이 뜨고 경고가 남는다: 출력이 원본 자신(별칭 포함)이거나 같은 원본으로 만든 파일이 아니면 원본별 이름을 쓴다. 파싱 → 키 설정 → 직렬화하고, 결과가 원본 + 키와 정확히 같아야 한다(헤더·점 표기·인라인 표 모두). 쓴 파일은 그 herdr 의 `config check` 를 통과해야 한다. herdr 는 파싱·스키마 오류면 파일 전체를 버리고("using defaults") 복원을 켜기 때문이다. 통과하지 못하면(깨짐, 비-bool 값, 표가 아닌 `session`, UTF-8 아님) 키 하나만 있는 파일을 쓰고 경고한다. MAM pane 이 물려받은 생성 파일은 기록된 원본으로 거슬러 올라간다. 2. **shim bootstrap (`lib.sh` heredoc)**: 서버를 띄울 때마다 설정을 다시 만들어 `HERDR_CONFIG_PATH` 로 넘긴다. 못 만들면 경고하고 예전처럼(복원 켜짐) 띄운다. 기본 `default` 서버와 사용자가 직접 띄운 서버는 바꾸지 않는다. 3. **복원된 빈 셸 (`lib_py/herdr_restore.py`, `resume_session.sh`)**: spawn 직전 한 번, `agent_session` 이 resume 하는 id 와 같고 live 에이전트가 없고 전경이 셸뿐인 pane 을 닫는다. 등록부는 보지 않는다(서버가 죽은 세션도 running 으로 남는다, Creator 이의 job 9f5f1ebe). `CP_PREFIX` 가 붙는 resume 이 재사용하지 못해 남던 빈 셸과 `$HOME` 으로 떨어진 빈 셸이 대상이다. 4. **S11 (`tests/test_issue23_24_startup_path.py`)**: 생성 shim 이 우연히 바뀌지 않게 하는 비교 기준을 heredoc 을 의도적으로 바꾼 ed73f67 로 옮겼다. @@ -55,6 +55,7 @@ - **실서버 테스트 (`tests/test_issue45_restore.py`, herdr 0.9.1)**: 짧은 일회용 HOME·세션과 가짜 claude 로, MAM bootstrap 경로에서 복원이 꺼짐(사용자 설정에 `[session]` 표가 있을 때 포함)과 같은 레이아웃을 일반 서버로 띄우면 `claude --resume` 이 뜨는 양성 대조, 닫기 조건, create → crash → resume 뒤 pane 하나만 남음을 확인한다. 수정을 끄면 A·B 테스트가 각각 실패함을 확인했다. - **단위 (`tests/test_issue45_herdr_config.py`)**: 합치기 12 가지 경우와 실제 `herdr config check`, 원본 경로 규칙, 쓰기·폴백. - **관련 스위트**: 새 두 파일 40 passed. issue9·shim contract·issue42·issue24 resume·uuid_target·issue21 prereqs·issue23_24 199 passed / 1 skipped(S11 기준 이동 후). tier1~4·issue21_phase2_loop_a 148 passed. 전체 스위트는 돌리지 않았다. +- **Python 3.9 (병합 워크스페이스)**: `tests/test_issue45_herdr_config.py` 가 모듈 머리에서 `import tomllib` 을 해 3.11 미만에서는 스위트 전체가 수집 단계에서 멈췄다. import 를 선택적으로 바꾸고 결과를 파싱하거나 사용자 설정 유지를 기대하는 29 건만 `tomllib` 없으면 skip(3.9: 22 passed / 29 skipped, 3.11: 51 passed). - **리뷰**: 1회차 codex NOT PASS(R1 herdr 스키마 검증 없음, R2 점 표기·인라인 표에서 설정 유실, R3 원본과 출력 이름 충돌 시 덮어씀). 811b080 에서 수정. 2회차 codex NOT PASS(R3 의 폴백 경로는 같은 검사를 거치지 않음), 폴백도 같은 이름 선택을 거치도록 수정. ### 🚀 `v4.8.4` — `.claude/skills` 링크 검사가 두 로컬 배치를 받고 깨진 링크를 잡는다 (2026-10-04, PR #47) diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py index 4e95973..c5d51f6 100644 --- a/tests/test_issue45_herdr_config.py +++ b/tests/test_issue45_herdr_config.py @@ -13,9 +13,13 @@ import shutil import stat import subprocess import sys -import tomllib from pathlib import Path +try: # Python 3.11+; on older Pythons herdr_config writes the key-only config + import tomllib +except ImportError: # pragma: no cover - depends on the interpreter + tomllib = None + import pytest REPO_ROOT = Path(__file__).resolve().parent.parent @@ -25,6 +29,12 @@ from lib_py import herdr_config as hc # noqa: E402 HERDR = shutil.which("herdr") +# Python < 3.11 has no tomllib: herdr_config then writes the key-only config +# (restore still off, the user's other settings dropped, with a warning), so +# the tests that parse a result or expect the user's settings kept need it. +# test_without_tomllib_restore_is_still_off covers that fallback everywhere. +needs_tomllib = pytest.mark.skipif(tomllib is None, reason="needs tomllib (Python 3.11+)") + # (name, user config text or None, expect the user's other settings kept, expect a warning) CASES = [ ("missing", None, True, False), @@ -58,6 +68,7 @@ def _without_key(d): return d +@needs_tomllib @pytest.mark.parametrize("name,user,kept,warned", CASES, ids=[c[0] for c in CASES]) def test_merge_always_parses_with_restore_off(name, user, kept, warned): text, warnings = hc.server_config_text(user) @@ -70,6 +81,7 @@ def test_merge_always_parses_with_restore_off(name, user, kept, warned): assert text == hc.MINIMAL +@needs_tomllib def test_a_non_bool_restore_value_is_replaced(): # herdr would reject the whole file for this; the key is ours to set anyway. text, warnings = hc.server_config_text('[session]\nresume_agents_on_restore = "no"\n') @@ -121,6 +133,7 @@ def test_source_is_herdr_config_path_unless_generated(tmp_path): str(tmp_path / "x" / "herdr" / "config.toml") +@needs_tomllib def test_written_next_to_source_with_its_mode(tmp_path): cfg_dir = tmp_path / "home" / ".config" / "herdr" cfg_dir.mkdir(parents=True) @@ -140,6 +153,7 @@ def test_written_next_to_source_with_its_mode(tmp_path): assert [p.name for p in cfg_dir.iterdir() if p.name.endswith(".tmp")] == [] +@needs_tomllib def test_written_without_any_user_config(tmp_path): path, warnings = hc.write_server_config(_env(tmp_path)) assert path == str(tmp_path / "home" / ".config" / "herdr" / hc.OUTPUT_NAME) and warnings == [] @@ -147,6 +161,7 @@ def test_written_without_any_user_config(tmp_path): assert tomllib.loads(Path(path).read_text()) == {"session": {"resume_agents_on_restore": False}} +@needs_tomllib def test_rewritten_on_every_call(tmp_path): cfg_dir = tmp_path / "home" / ".config" / "herdr" cfg_dir.mkdir(parents=True) @@ -157,6 +172,7 @@ def test_rewritten_on_every_call(tmp_path): assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False +@needs_tomllib @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") def test_unwritable_config_dir_falls_back_to_mam_dir(tmp_path): cfg_dir = tmp_path / "home" / ".config" / "herdr" @@ -184,6 +200,7 @@ def _tag(source: Path) -> str: return hashlib.sha256(os.path.realpath(source).encode()).hexdigest()[:12] +@needs_tomllib @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") def test_a_foreign_fallback_file_is_kept(tmp_path): cfg_dir = tmp_path / "cfg" @@ -204,6 +221,7 @@ def test_a_foreign_fallback_file_is_kept(tmp_path): assert tomllib.loads(Path(path).read_text())["ui"]["confirm_close"] is False +@needs_tomllib @pytest.mark.parametrize("alias", [False, True], ids=["same-path", "symlink-alias"]) def test_the_fallback_is_never_the_source(tmp_path, alias): mam = tmp_path / ".mam" @@ -243,6 +261,7 @@ def test_no_write_when_every_name_is_taken(tmp_path): assert all(f.read_bytes() == b"# foreign\n" for f in taken) +@needs_tomllib def test_cli_prints_the_path(tmp_path): env = dict(os.environ, HOME=str(tmp_path), PYTHONPATH=str(REPO_ROOT / ".agents" / "skills")) env.pop("HERDR_CONFIG_PATH", None) @@ -295,6 +314,7 @@ def _cli(tmp_path, source: Path, *extra) -> subprocess.CompletedProcess: (b"session = 1\n[ui]\nconfirm_close = false\n", False), # non-table session (R1) (b"[ui]\nconfirm_close = false\n# \xff\xfe\n", False), # not UTF-8 (R1) ], ids=["valid", "schema-error", "session-scalar", "invalid-utf8"]) +@needs_tomllib def test_cli_always_hands_herdr_a_restore_off_config_it_accepts(tmp_path, body, kept): source = tmp_path / "custom.toml" source.write_bytes(body) @@ -312,6 +332,7 @@ def test_cli_always_hands_herdr_a_restore_off_config_it_accepts(tmp_path, body, assert source.read_bytes() == body +@needs_tomllib @pytest.mark.skipif(not HERDR, reason="herdr is not installed") def test_a_config_herdr_rejects_is_never_left_in_place(tmp_path): # the merge itself is fine for tomllib; only herdr's schema says no @@ -324,6 +345,7 @@ def test_a_config_herdr_rejects_is_never_left_in_place(tmp_path): # --- the user's file is never the output (review R3) ----------------------- +@needs_tomllib def test_source_named_like_the_output_is_not_overwritten(tmp_path): source = tmp_path / hc.OUTPUT_NAME source.write_bytes(b"[ui]\nconfirm_close = false\n") -- 2.54.0 From e8d9151226d8139c7b5688e83d035a3a747aeb2d Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 5 Oct 2026 15:59:26 +0900 Subject: [PATCH 12/14] test(issue-45): review - the real-server tests run on macOS, and the bootstrap config check follows the bootstrap's python3 claude round 1, B2 and B3 (B1 was the previous commit). - B2, tests/test_issue45_restore.py: the throwaway root is resolved, because on macOS /tmp links to /private/tmp and herdr reports real paths (two B tests compared /tmp/... with /private/tmp/...). The server's environment is read from /proc on Linux and from `ps -E` elsewhere (macOS has no /proc; two A tests failed with FileNotFoundError). - B3, tests/test_issue9_server_env.py: the bootstrap builds the config with the python3 on PATH, which here is 3.9 without tomllib, so the test's merged-settings expectation could not hold and its own `import tomllib` failed. The test now parses the result with that same python3; when it has no tomllib, it expects the key-only config and the bootstrap's "no tomllib" warning instead. macOS, Python 3.9 venv: test_issue45_restore, test_issue9_server_env and test_issue45_herdr_config 42 passed / 29 skipped (the tomllib-only checks). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- tests/test_issue45_restore.py | 18 +++++++++++++++--- tests/test_issue9_server_env.py | 18 +++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/tests/test_issue45_restore.py b/tests/test_issue45_restore.py index 66eb254..188e722 100644 --- a/tests/test_issue45_restore.py +++ b/tests/test_issue45_restore.py @@ -157,10 +157,22 @@ class Rig: return self.argv_log.exists() and f"--resume {session_id}" in self.argv_log.read_text() +def _process_env(pid: int) -> list: + """A process's environment as KEY=VALUE strings: /proc on Linux; on macOS + `ps -E` appends it to the command line, so the entries are matched there.""" + proc = Path(f"/proc/{pid}/environ") + if proc.exists(): + return [e.decode(errors="replace") for e in proc.read_bytes().split(b"\0") if e] + out = subprocess.run(["ps", "-E", "-ww", "-o", "command=", "-p", str(pid)], + capture_output=True, text=True, timeout=10).stdout + return out.split() + + @pytest.fixture def rig(mam_sandbox, monkeypatch): # A pytest tmp_path is too long for a unix socket path under it. - root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp")) + # Resolved: on macOS /tmp is a link to /private/tmp and herdr reports real paths. + root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp")).resolve() session = f"mamt45-{os.getpid()}-{uuid.uuid4().hex[:4]}" for k in SCOPE_ENV: monkeypatch.delenv(k, raising=False) @@ -224,8 +236,8 @@ def test_a_mam_server_does_not_relaunch_saved_agents(rig, user_session_table): info = rig.hd("pane", "process-info", "--pane", pane["pane_id"])["process_info"] assert info["foreground_process_group_id"] == info["shell_pid"], info # and it got there through HERDR_CONFIG_PATH - environ = Path(f"/proc/{rig.server_pids()[0]}/environ").read_bytes().split(b"\0") - assert f"HERDR_CONFIG_PATH={rig.user_cfg.parent / herdr_config.OUTPUT_NAME}".encode() in environ + want = f"HERDR_CONFIG_PATH={rig.user_cfg.parent / herdr_config.OUTPUT_NAME}" + assert want in _process_env(rig.server_pids()[0]), want # Positive control: the same saved layout under a plain server relaunches # the agent, so the "not resumed" above is not vacuous. diff --git a/tests/test_issue9_server_env.py b/tests/test_issue9_server_env.py index 9edca48..cc8e7c7 100644 --- a/tests/test_issue9_server_env.py +++ b/tests/test_issue9_server_env.py @@ -154,7 +154,6 @@ def test_bootstrap_hands_the_server_a_no_restore_config(mam_sandbox): """Issue #45: the server starts with HERDR_CONFIG_PATH = the user's herdr config plus [session] resume_agents_on_restore = false (that herdr obeys it is test_issue45_restore.py, on a real server).""" - import tomllib env = _install_fake_herdr(mam_sandbox) user_cfg = mam_sandbox / ".config" / "herdr" / "config.toml" user_cfg.parent.mkdir(parents=True) @@ -167,8 +166,21 @@ def test_bootstrap_hands_the_server_a_no_restore_config(mam_sandbox): server_env = json.loads(Path(env["FAKE_HERDR_ENV_DUMP"]).read_text()) assert server_env.get("HERDR_CONFIG_PATH") == str(user_cfg.parent / "mam-server-config.toml") assert "MAM_SERVER_CFG" not in server_env - cfg = tomllib.loads(Path(server_env["HERDR_CONFIG_PATH"]).read_text()) - assert cfg == {"ui": {"confirm_close": False}, "session": {"resume_agents_on_restore": False}} + # The bootstrap builds the file with the python3 on PATH; parse it with that + # same python3. Without tomllib there (Python < 3.11, macOS's 3.9) the file is + # the key-only config and the bootstrap warns that the other settings are gone. + out = Path(server_env["HERDR_CONFIG_PATH"]) + parse = subprocess.run(["python3", "-c", "import json, sys, tomllib; " + "print(json.dumps(tomllib.loads(open(sys.argv[1]).read())))", str(out)], + capture_output=True, text=True, env=env, timeout=30) + if parse.returncode == 0: + assert json.loads(parse.stdout) == {"ui": {"confirm_close": False}, + "session": {"resume_agents_on_restore": False}} + else: + assert "No module named 'tomllib'" in parse.stderr, parse.stderr + text = out.read_text() + assert "resume_agents_on_restore = false" in text and "confirm_close" not in text, text + assert "no tomllib" in res.stderr, res.stderr def test_no_autostart_skips_bootstrap_but_still_routes_the_call(mam_sandbox): -- 2.54.0 From ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9 Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 5 Oct 2026 17:50:15 +0900 Subject: [PATCH 13/14] fix(issue-45): review round 2 - an unrunnable config check is not a pass, odd pids never close a pane, nan does not drop settings codex round 2 (PR #48). - R1: when `herdr config check` could not run (timeout, OSError), the merged config was handed to the server as checked. A merge with a schema error makes herdr drop the whole file, restore back on - the #45 failure. The merged body is no longer used unchecked: _write_checked falls to the fixed MINIMAL config with a warning. When the check cannot run on MINIMAL either, MINIMAL is still used (with its own warning) rather than no config: returning none would leave restore on, and MINIMAL is a fixed two-line file the real-herdr test accepts. Unknown non-zero exits stay as they were - herdr exits non-zero for harmless warnings (an unknown section) that do not drop the file. - R2: _shell_in_foreground compared shell_pid and the foreground group with ==, so 0/0, False/False, ""/"" or -1/-1 closed the pane. Both must now be positive ints (bool excluded) before they are compared. - R3: the re-parse check used ==, and nan != nan, so a valid TOML nan threw away every other user setting even on Python 3.11. _same_doc compares documents recursively with nan equal to nan. Tests: R1 (timeout, OSError) and R3 (top-level, array, nested nan) in test_issue45_herdr_config.py; R2 in the new tests/test_issue45_restore_unit.py (a herdr stub: zero, false, empty, negative, null, string, true, float; missing info; a real shell closed; a busy pane kept). On the previous code 12 of them fail. Python 3.11: 67 passed; Python 3.9: 33 passed / 34 skipped (tomllib-only). Real-herdr restore tests and related files on 3.9: 197 passed / 1 skipped. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib_py/herdr_config.py | 25 +++++++++++- .agents/skills/lib_py/herdr_restore.py | 10 ++++- tests/test_issue45_herdr_config.py | 38 +++++++++++++++++ tests/test_issue45_restore_unit.py | 56 ++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 tests/test_issue45_restore_unit.py diff --git a/.agents/skills/lib_py/herdr_config.py b/.agents/skills/lib_py/herdr_config.py index e1012bb..ed54918 100644 --- a/.agents/skills/lib_py/herdr_config.py +++ b/.agents/skills/lib_py/herdr_config.py @@ -137,6 +137,18 @@ def dumps(doc: dict) -> str: return "\n".join(lines) + "\n" +def _same_doc(a, b) -> bool: + """Equality of two parsed TOML documents in which nan equals nan (a valid + TOML value that a plain == never matches, review R3).""" + if isinstance(a, float) and isinstance(b, float) and math.isnan(a) and math.isnan(b): + return True + if isinstance(a, dict) and isinstance(b, dict): + return a.keys() == b.keys() and all(_same_doc(a[k], b[k]) for k in a) + if isinstance(a, list) and isinstance(b, list): + return len(a) == len(b) and all(_same_doc(x, y) for x, y in zip(a, b)) + return type(a) is type(b) and a == b + + def server_config_text(user_text: Optional[str]) -> Tuple[str, List[str]]: """(the TOML body for the server, warnings). The body always parses, has session.resume_agents_on_restore = false and otherwise equals the source - @@ -156,7 +168,7 @@ def server_config_text(user_text: Optional[str]) -> Tuple[str, List[str]]: expected = dict(user, session=dict(session, **{KEY: False})) try: text = dumps(expected) - ok = tomllib.loads(text) == expected + ok = _same_doc(tomllib.loads(text), expected) except (TypeError, tomllib.TOMLDecodeError): ok = False if not ok: @@ -233,7 +245,16 @@ def _write_checked(target: str, source: str, body: str, mode: int, herdr: Option try: why = herdr_rejects(target, herdr) except (OSError, subprocess.SubprocessError) as e: - warnings.append(f"cannot run `{herdr} config check` ({e}); the config is unchecked") + # Not a pass (review R1): an unchecked merge with a schema error + # makes herdr drop the whole file - restore back on. The merged + # body is not used unchecked; MINIMAL is a fixed file herdr 0.9.1 + # accepts (test_real_herdr_accepts_every_result), and returning no + # config would leave restore on, so it is used with a warning. + if candidate != MINIMAL: + warnings.append(f"cannot run `{herdr} config check` ({e}); not using an unchecked merge; " + f"{LOSES_SETTINGS}") + continue + warnings.append(f"cannot run `{herdr} config check` ({e}); using the fixed minimal config unchecked") return True if why is None: return True diff --git a/.agents/skills/lib_py/herdr_restore.py b/.agents/skills/lib_py/herdr_restore.py index 07b86e6..c1a5f23 100644 --- a/.agents/skills/lib_py/herdr_restore.py +++ b/.agents/skills/lib_py/herdr_restore.py @@ -43,8 +43,14 @@ def _herdr(herdr: str, session: str, *args: str) -> dict: def _shell_in_foreground(herdr: str, session: str, pane_id: str) -> bool: info = _herdr(herdr, session, "pane", "process-info", "--pane", pane_id).get("process_info") or {} - shell = info.get("shell_pid") - return shell is not None and info.get("foreground_process_group_id") == shell + shell, fg = info.get("shell_pid"), info.get("foreground_process_group_id") + # Only a real pid proves anything (review R2): None, 0, -1, False, "" or a + # string must leave the pane alone, however they compare. + return _is_pid(shell) and _is_pid(fg) and fg == shell + + +def _is_pid(v) -> bool: + return isinstance(v, int) and not isinstance(v, bool) and v > 0 def close_restored_shells(session_id: str, herdr: str = "herdr", diff --git a/tests/test_issue45_herdr_config.py b/tests/test_issue45_herdr_config.py index c5d51f6..904a8cc 100644 --- a/tests/test_issue45_herdr_config.py +++ b/tests/test_issue45_herdr_config.py @@ -380,3 +380,41 @@ def test_a_foreign_file_with_the_output_name_is_kept(tmp_path): other_path, _ = hc.write_server_config({"HERDR_CONFIG_PATH": str(other)}) assert other_path not in (path, str(foreign)) assert hc.user_config_path({"HERDR_CONFIG_PATH": path}) == str(source) + + +# --- review round 2 (codex R1, R3) ------------------------------------------- + +@needs_tomllib +@pytest.mark.parametrize("exc", [subprocess.TimeoutExpired(["herdr", "config", "check"], 30), + OSError("herdr went away")], ids=["timeout", "oserror"]) +def test_an_unrunnable_check_never_hands_over_an_unchecked_merge(tmp_path, monkeypatch, exc): + """R1: a check that cannot run is not a pass. A merge with a schema error + would make herdr drop the whole file (restore back on), so the result is + the fixed minimal config, with a warning - never the unchecked merge.""" + source = tmp_path / "config.toml" + source.write_text('[ui]\nconfirm_close = "invalid-bool"\n') + + def boom(path, herdr): + raise exc + monkeypatch.setattr(hc, "herdr_rejects", boom) + path, warnings = hc.write_server_config({"HERDR_CONFIG_PATH": str(source)}, herdr="/fake/herdr") + assert path is not None + text = Path(path).read_text() + assert "invalid-bool" not in text and "confirm_close" not in text, text + assert tomllib.loads(text) == {"session": {"resume_agents_on_restore": False}} + assert any("not using an unchecked merge" in w for w in warnings), warnings + assert source.read_text() == '[ui]\nconfirm_close = "invalid-bool"\n' + + +@needs_tomllib +@pytest.mark.parametrize("nan_part", ["[custom]\nvalue = nan\n", "[custom]\nvalues = [1.0, nan, -nan]\n", + "[custom.deep]\nt = { a = nan }\n"], ids=["top", "array", "nested"]) +def test_a_nan_value_does_not_drop_the_other_settings(nan_part): + """R3: nan is valid TOML and never == itself; the re-parse check must treat + it as equal, or the user's other settings are thrown away.""" + body, warnings = hc.server_config_text("[ui]\nconfirm_close = false\n" + nan_part) + assert warnings == [], warnings + parsed = tomllib.loads(body) + assert parsed["ui"]["confirm_close"] is False + assert parsed["session"]["resume_agents_on_restore"] is False + assert "custom" in parsed diff --git a/tests/test_issue45_restore_unit.py b/tests/test_issue45_restore_unit.py new file mode 100644 index 0000000..7a7d407 --- /dev/null +++ b/tests/test_issue45_restore_unit.py @@ -0,0 +1,56 @@ +"""Issue #45 (B), review round 2 R2: resume closes a restored pane only when +herdr's answer proves the shell is in the foreground. Odd pid values - missing, +null, 0, negative, bool, string - must leave the pane alone. herdr is a stub +here; the real-server behaviour is tests/test_issue45_restore.py.""" +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / ".agents" / "skills")) +from lib_py import herdr_restore as hr # noqa: E402 + +SID = "probe-id" + + +def _stub(monkeypatch, process_info): + calls = [] + + def fake(herdr, session, *args): + calls.append(args) + if args[:2] == ("pane", "list"): + return {"panes": [{"pane_id": "probe-pane", "agent": None, "agent_session": {"value": SID}}]} + if args[:2] == ("pane", "process-info"): + return {"process_info": process_info} + return {} + monkeypatch.setattr(hr, "_herdr", fake) + return calls + + +@pytest.mark.parametrize("shell,fg", [(0, 0), (False, False), ("", ""), (-1, -1), (None, None), + ("123", "123"), (True, True), (1.0, 1.0)], + ids=["zero", "false", "empty", "negative", "null", "string", "true", "float"]) +def test_an_odd_pid_answer_leaves_the_pane_alone(monkeypatch, shell, fg): + calls = _stub(monkeypatch, {"shell_pid": shell, "foreground_process_group_id": fg}) + assert hr.close_restored_shells(SID, session="m45-review-probe") == [] + assert ("pane", "close", "probe-pane") not in calls + + +def test_a_missing_process_info_leaves_the_pane_alone(monkeypatch): + calls = _stub(monkeypatch, {}) + assert hr.close_restored_shells(SID, session="m45-review-probe") == [] + assert ("pane", "close", "probe-pane") not in calls + + +def test_a_real_shell_in_the_foreground_is_closed(monkeypatch): + calls = _stub(monkeypatch, {"shell_pid": 4242, "foreground_process_group_id": 4242}) + assert hr.close_restored_shells(SID, session="m45-review-probe") == ["probe-pane"] + assert ("pane", "close", "probe-pane") in calls + + +def test_a_busy_pane_is_left_alone(monkeypatch): + calls = _stub(monkeypatch, {"shell_pid": 4242, "foreground_process_group_id": 4300}) + assert hr.close_restored_shells(SID, session="m45-review-probe") == [] + assert ("pane", "close", "probe-pane") not in calls -- 2.54.0 From bf8a583afbe9abe1bf5e13350b61a175e566eec3 Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 5 Oct 2026 18:56:38 +0900 Subject: [PATCH 14/14] docs(v4.8.5): archive PR #48 reviews (three rounds) and record the merge-workspace review PR #48 already carries the 4.8.5 lockstep bump. Merge-workspace review: round 1 (@35935f9) claude agy grok NOT PASS (test portability: tomllib on Python 3.9, /proc and /tmp on macOS), codex no verdict (model at capacity, budget spent); fixed in 36f0b8d and e8d9151. Round 2 (@e8d9151) codex NOT PASS (unrunnable config check passed an unchecked merge, odd pids closed a pane, nan dropped the user's settings); fixed in ab93776. Round 3 (@ab93776) 4/4 PASS. Full suite at ab93776 on macOS / Python 3.9: 1456 passed / 0 failed / 38 skipped. Reports and the round-3 identity sidecars go to .agents/reports//. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .../report-3a93c30b.md | 30 +++++ .../report-416d9768.md | 76 +++++++++++ .../report-ab18016d.identity.json | 14 ++ .../report-ab18016d.md | 50 +++++++ .../report-714c5189.md | 123 ++++++++++++++++++ .../report-b8dbd280.identity.json | 14 ++ .../report-b8dbd280.md | 93 +++++++++++++ .../report-3e6f0467.identity.json | 14 ++ .../reviewer-agy-01/report-3e6f0467.md | 115 ++++++++++++++++ .../reviewer-agy-01/report-3f82b402.md | 105 +++++++++++++++ .../reviewer-agy-01/report-f3699a96.md | 107 +++++++++++++++ .../report-2a661116.identity.json | 14 ++ .../reviewer-grok-01/report-2a661116.md | 40 ++++++ .../reviewer-grok-01/report-667d5542.md | 69 ++++++++++ .../reviewer-grok-01/report-b4719c9e.md | 32 +++++ VERSIONS.md | 2 + 16 files changed, 898 insertions(+) create mode 100644 .agents/reports/planner-reviewer-claude-01/report-3a93c30b.md create mode 100644 .agents/reports/planner-reviewer-claude-01/report-416d9768.md create mode 100644 .agents/reports/planner-reviewer-claude-01/report-ab18016d.identity.json create mode 100644 .agents/reports/planner-reviewer-claude-01/report-ab18016d.md create mode 100644 .agents/reports/planner-reviewer-codex-01/report-714c5189.md create mode 100644 .agents/reports/planner-reviewer-codex-01/report-b8dbd280.identity.json create mode 100644 .agents/reports/planner-reviewer-codex-01/report-b8dbd280.md create mode 100644 .agents/reports/reviewer-agy-01/report-3e6f0467.identity.json create mode 100644 .agents/reports/reviewer-agy-01/report-3e6f0467.md create mode 100644 .agents/reports/reviewer-agy-01/report-3f82b402.md create mode 100644 .agents/reports/reviewer-agy-01/report-f3699a96.md create mode 100644 .agents/reports/reviewer-grok-01/report-2a661116.identity.json create mode 100644 .agents/reports/reviewer-grok-01/report-2a661116.md create mode 100644 .agents/reports/reviewer-grok-01/report-667d5542.md create mode 100644 .agents/reports/reviewer-grok-01/report-b4719c9e.md diff --git a/.agents/reports/planner-reviewer-claude-01/report-3a93c30b.md b/.agents/reports/planner-reviewer-claude-01/report-3a93c30b.md new file mode 100644 index 0000000..0c2cfe8 --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-3a93c30b.md @@ -0,0 +1,30 @@ +# 리뷰 리포트 — Job 3a93c30b (PR #48 2회차, 35935f9..e8d9151) + +- 리뷰어: planner-reviewer-claude-01 (claude) +- 리뷰 identity: loop-20261005T081844Z-e92124 / base c5c7eff / head e8d9151 / tree 8b39188 — 검증 경로에서 `git rev-parse HEAD 'HEAD^{tree}'` 일치, 미커밋 변경 없음 +- 범위: 커밋 36f0b8d, e8d9151. `git diff --stat 35935f9 e8d9151`: create SKILL.md 1줄, VERSIONS.md, 테스트 3개. lib.sh·lib_py·스크립트 변경 없음(제품 코드 불변). + +## 결론 요약 +1회차의 차단 3건(B1~B3)과 문서 지적(D1)이 모두 닫혔다. 이 macOS 호스트(Python 3.9 venv)에서 지정 6개 파일이 실패 없이 통과하고, 실제 herdr 서버 테스트 5개가 skip 없이 돈다. 차단 결함 없음. + +## 1. 1회차 지적 종결 +| 지적 (1회차) | 수정 | 확인 | +|---|---|---| +| B1 test_issue45_herdr_config.py:16 최상단 `import tomllib` → 3.9 에서 수집 중단 | try/except 로 선택적 import(:18-21), 결과를 파싱하거나 설정 유지를 기대하는 테스트만 `needs_tomllib` skip(:36) | 3.9: 수집 정상, tomllib 의존 검사만 skip. Python 3.11(Homebrew + PYTHONPATH): **51 passed**. 3.9 에서 남는 22건 중 test_without_tomllib_restore_is_still_off(:91-96)가 tomllib 없을 때 MINIMAL + "no tomllib" 경고를 직접 확인하므로 3.9 경로도 검사된다 | +| B2 test_issue45_restore.py `/proc` 와 `/tmp` | 루트 `.resolve()`(:175), 서버 환경은 `/proc` 가 있으면 거기서, 없으면 `ps -E -ww`(:160-169, :239-240) | test_issue45_restore.py 5개 모두 **실행·통과**(skip 아님). 1회차에 내가 스크래치 사본으로 확인한 것과 같은 두 수정이며, 그때 A·B 수정을 끄면 각각 실패함(A: `--resume `, B: pane 2개)을 확인했다. 제품 코드가 그 뒤 바뀌지 않았으므로 대조 결과는 그대로 유효하다 | +| B3 test_issue9_server_env.py:157 tomllib, bootstrap 의 python3 가정 | bootstrap 과 같은 PATH 의 `python3` 로 결과를 파싱, tomllib 이 없으면 키 하나짜리 파일 + `confirm_close` 없음 + 경고 "no tomllib" 를 기대(:169-183) | 통과. 이 호스트에서는 else 분기(3.9)가 실행된다. `subprocess.run(["python3"…], env=env)` 는 POSIX 에서 env 의 PATH 로 실행 파일을 찾으므로 bootstrap 과 같은 인터프리터다. 경고 문구는 herdr_config.py:147 와 일치 | +| D1 (비차단) python3 < 3.11 이면 사용자 설정 없이 뜬다는 문서 부재 | create SKILL.md:120 와 VERSIONS 4.8.5 항목 1, 품질 검증 항목에 추가 | 내용은 코드(lib.sh:241 의 bare `python3`, herdr_config.py:146-147)와 일치 | + +## 2. 테스트 실행 +| 실행 | 결과 | +|---|---| +| 지정 6개 파일 (pr-48 .venv, Python 3.9.6) | **152 passed / 30 skipped / 0 failed** (216 s). skip = tomllib 필요 29건 + .claude/skills 1건(워크트리 정상). test_issue45_restore 는 skip 0 | +| tests/test_issue45_herdr_config.py (python3.11) | **51 passed** | +- 격리: 실행 전후 herdr 프로세스는 기존 `multi-agent-mux` 서버·attach 클라이언트와 오케스트레이터의 `agent wait` 뿐(잔류 서버 없음), `/private/tmp/m45*` 없음, `~/.config/herdr` 목록·config.toml md5·sessions 목록 불변. MQTT_* unset, .mam.env 미export. 실제 .mam/jobs 에 새 항목 없음. + +## 3. 비차단 관찰 +- **N1 VERSIONS.md 4.8.5 항목 1 문장 흐름**: 새 문장이 "…경고가 남는다:" 로 끝나고 바로 뒤에 원래 있던 "출력이 원본 자신(별칭 포함)이거나 … 원본별 이름을 쓴다" 가 이어져, 출력 이름 규칙이 Python 버전 설명의 결과처럼 읽힌다. 원래는 "사용자 파일은 고치지 않는다:" 뒤에 붙던 설명이다. 새 문장을 항목 끝으로 옮기면 된다. +- **N2 `_process_env` 의 macOS 분기**(test_issue45_restore.py:166-169)는 `ps -E` 출력을 공백으로 나누므로 공백이 든 경로는 맞추지 못한다. 테스트 루트가 `/private/tmp/m45…` 로 고정이라 지금은 영향 없다. +- **N3 tomli 백포트 대신 문서화한 결정**에 대한 의견: 복원은 어느 경우에도 꺼지므로(#45 의 핵심) 안전 측면에서 문제없고, 이 호스트의 herdr 설정이 키 하나뿐이라는 사정에서 합리적이다. 나중에 사용자 설정이 커지면, 새 의존성 없이 PATH 에서 3.11+ 인터프리터를 찾는 방법도 고려할 만하다. + +[VERDICT: PASS] diff --git a/.agents/reports/planner-reviewer-claude-01/report-416d9768.md b/.agents/reports/planner-reviewer-claude-01/report-416d9768.md new file mode 100644 index 0000000..c6dc7c7 --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-416d9768.md @@ -0,0 +1,76 @@ +# 리뷰 리포트 — Job 416d9768 (PR #48, fix/issue-45) + +- 리뷰어: planner-reviewer-claude-01 (claude) +- 리뷰 identity: loop-20261005T064728Z-a4e9c8 / base c5c7eff / head 35935f9 / tree dff9db1 — 검증 경로에서 `git rev-parse HEAD 'HEAD^{tree}'` 일치, 미커밋 변경 없음 + +## 결론 요약 +제품 동작(A: MAM 서버 복원 꺼짐, B: 자기 세션의 빈 셸만 닫기)은 이 macOS 호스트의 실제 herdr(0.9.1)에서 동작함을 확인했다. 그러나 **브리프가 지정한 검증 명령이 이 호스트(리포지토리 .venv = Python 3.9.6)에서 수집 단계에서 중단**되고, 그 파일을 빼고 돌려도 **새 테스트 5개가 macOS 에서 실패**한다. 원인은 모두 테스트의 Linux·Python 3.11 가정이며 수정은 작다. 스킬 frontmatter 가 `platforms: [linux, macos]` 이므로 차단으로 판단한다. + +## 차단 (테스트 이식성) +### B1. tests/test_issue45_herdr_config.py:16 — 모듈 최상단 `import tomllib` +- Python 3.9 에서 `ModuleNotFoundError` → pytest 가 "Interrupted: 1 error during collection" 으로 **세션 전체를 중단**한다. 브리프 지정 명령(7개 파일)은 결과 0건, rc 2. 오케스트레이터의 전체 스위트도 같은 .venv 면 같은 이유로 중단된다. +- 제품 쪽(lib_py/herdr_config.py:54-57)은 tomllib 부재를 처리하는데 테스트만 강제한다. `tomllib = pytest.importorskip("tomllib")` 처럼 모듈 skip 또는 tomllib 이 필요 없는 경우만 남기는 처리가 필요하다. +- 참고: 같은 파일을 Homebrew python3.11 + 기존 venv 의 pytest(PYTHONPATH)로 돌리면 **51 passed**. 내용 자체는 정상이다. + +### B2. tests/test_issue45_restore.py:227, :163 — Linux 전용 가정 (macOS 4 failed) +| 테스트 | 실패 | 원인 | +|---|---|---| +| test_a_…[plain-user-config], test_a_…[user-config-with-session-table] | FileNotFoundError `/proc//environ` | :227 `/proc` 는 macOS 에 없다 | +| test_b_only_the_resumed_sessions_restored_shell_is_closed | `'/private/tmp/m45…' == '/tmp/m45…'` (:263) | :163 `mkdtemp(dir="/tmp")` — macOS 에서 /tmp 는 /private/tmp 링크이고 herdr 는 실경로를 보고한다 | +| test_b_resume_leaves_no_restored_shell_behind | StopIteration (:325, cwd 비교) | 같은 원인 | +- 검증: 스크래치 사본에서 **이 두 군데만** 고친 테스트(`.resolve()` 추가, `/proc` 대신 `ps -E -ww -o command= -p `)는 **5 passed**(65 s). 즉 제품은 맞고 테스트만 이식되지 않았다. 양성 대조도 이 사본에서 동작한다. + +### B3. tests/test_issue9_server_env.py:157 — 테스트 안 `import tomllib`, 그리고 bootstrap 의 python3 가정 +- 3.9 에서 import 로 실패. 또 3.11 로 테스트를 돌려도 bootstrap 은 `python3`(lib.sh:241, 이 호스트는 /usr/bin/python3 3.9.6)로 herdr_config 를 실행하므로 결과는 MINIMAL 이고, :170 의 `cfg == {"ui": …, "session": …}` 단언은 실패한다. `python3` 에 tomllib 이 없으면 skip(또는 MINIMAL 을 기대)해야 한다. + +## 검토 항목별 판단 +### 1. A 의 설정 병합 (lib_py/herdr_config.py) +- 원본 보호: 출력은 항상 output_path(:192-205)를 거친다 — 원본 자신(samefile/realpath, 별칭 포함)이나 다른 원본용·비생성 파일이면 해시 이름으로, 그것도 막히면 OSError → 쓰지 않음. 폴백(.mam)도 같은 함수(:273, R3 2회차 수정). 쓰기는 같은 디렉터리 임시 파일 + os.replace(:208-222), 원본 권한 복사(:255). +- 재귀: HERDR_CONFIG_PATH 가 생성 파일이면 2행의 `# source:` 로 원본을 따라간다(:65-92). source 행이 없으면 XDG/HOME 으로. +- 병합 정확성: tomllib parse → `session` 에 키만 덮어씀 → dumps → 재파싱이 기대값과 **정확히 같을 때만** 사용(:156-164). 인라인 표·점 표기·따옴표 키·배열은 파싱 결과(dict)로 다루므로 표기와 무관. 주석·표기 형식은 생성 파일에서 사라지지만 원본은 그대로이고 의미는 같다. 실패 시 MINIMAL + 경고. +- herdr 검사: 쓴 뒤 `herdr config check`(:169-180)가 "parse error"/"using defaults" 를 내면 MINIMAL 로 재시도, 그것도 거부되면 None → bootstrap 이 경고하고 예전처럼(복원 켜짐) 기동(lib.sh:241-245). 단위 테스트가 실제 herdr 로 이를 확인(3.11 에서 51 passed). +- 사용자 디렉터리 흔적: `~/.config/herdr/mam-server-config.toml`(또는 해시 이름) 1개, 중단 시 `.mam-server-config.*.tmp` 가능. create SKILL.md:120-121 에 위치가 적혀 있다. +- 비차단 D1 (아래): python3 < 3.11 호스트에서는 병합이 일어나지 않는다. + +### 2. 적용 경로 +- MAM 이 서버를 띄우는 곳은 shim 의 headless bootstrap 한 곳(lib.sh:261 `"$REAL_HERDR" --session "$_MAM_SESSION" server`)뿐이고 A 가 여기에 걸려 있다. create·resume·delegate 는 모두 shim 을 거친다. 다른 `server` 기동은 grep 으로 찾지 못했다. MAM_NO_AUTOSTART 호출자(status/reconcile)는 서버를 띄우지 않는다(lib.sh:196-201). +- 걸리지 않는 경로: 사용자가 `herdr session attach`/`herdr --session X` 로 직접 띄운 서버, 기본 `default` 서버, 이미 떠 있는 서버. create SKILL.md:121 이 "rebuilt each time MAM starts the server … next started", "A server you start yourself (`herdr session attach`) does not get it" 로 적고 있다(합의된 한계와 일치). + +### 3. B 의 닫기 조건 (lib_py/herdr_restore.py) +- 조건(:54-65): 세션 id 비어 있지 않음, 서버가 default 아님, `agent` 없음, `agent_session.value == id`, 전경 = shell_pid(shell_pid 없으면 False). 사용자 pane(agent_session 없음)·다른 세션·전경 프로세스 있음·에이전트 살아 있음은 제외 — 실제 서버 테스트(사본)에서 사용자 셸·Z(실행 중)·X2(sleep 60)가 남고 X·Y 만 닫힘을 확인. +- fail-safe: herdr 오류·JSON 오류는 예외 → exit 1(:79-83) → resume 은 경고만(resume_session.sh:240-242) 하고 진행. default 서버는 herdr 를 부르지도 않는다(test_b_default_server_is_never_touched 통과). Class A fresh-spawn(UUID 없음)은 즉시 반환. + +### 4. 테스트의 격리·결정성 +- test_issue45_restore 는 HOME 을 `/tmp/m45*` 로, 세션을 `mamt45--<4hex>` 로 두고 HERDR_*·XDG_CONFIG_HOME·HERDR_CONFIG_PATH 를 지운다(:42-44, :161-168). 정리는 그 세션 이름·그 루트 경로로만 pkill(:186-195). conftest 도 HERDR_CONFIG_PATH·XDG_CONFIG_HOME 을 지운다. +- 실행 전후 확인(실패 실행 3회, 사본 실행 3회 모두): herdr 프로세스는 기존 `multi-agent-mux` 서버·attach 클라이언트뿐(잔류 없음), `/tmp/m45*` 없음, `~/.config/herdr` 목록·config.toml md5·sessions 목록 불변. +- 양성 대조: 사본에서 lib.sh 의 `HERDR_CONFIG_PATH` 주입을 끄면 test_a 가 `--resume ` 로 실패(#45 재현), resume 의 close 호출을 끄면 test_b_resume 가 pane 2개로 실패. 테스트가 수정에 실제로 의존한다. +- 결정성: 폴링 + 고정 대기(최대 수 초)라 느린 호스트에서 다소 민감할 수 있으나 3회 반복 모두 같은 결과였다. + +### 5. 문서·인벤토리 +- 버전: lib.sh·eager.py·SKILL.md 8개·VERSIONS 가 4.8.5 로 일치(test_version_consistency 통과). +- skill_code_blocks.json: blocks 의 (skill, class, kind) 순서 동일, create 블록 4개의 start/end 이동과 resume removed 항목의 evidence 줄 번호(+8) 이동뿐. 분류 변경 없음. +- **D1 (비차단, 문서)**: VERSIONS 4.8.5 항목 1과 create SKILL.md:120 은 서버 설정을 "사용자 설정 + 키"로 설명하지만, bootstrap 은 `python3`(lib.sh:241)로 실행되므로 python3 < 3.11 인 호스트(이 macOS 의 /usr/bin/python3 3.9.6)에서는 **항상 키 하나만 있는 파일**로 기동하고 사용자 herdr 설정(예: default_shell·키 바인딩)이 MAM 서버에 적용되지 않으며, 기동마다 경고가 나온다. 복원은 꺼지므로 안전하지만, 이 한계를 문서에 적거나 3.11+ 인터프리터를 찾는 편이 좋다. VERSIONS 의 "실패 시 키 하나만" 목록에도 Python 3.11 미만이 빠져 있다. + +### 6. purge 기준 실패 +검증 경로에서 tests/test_issue10_13_purge.py **23 passed**(macOS). 이 PR 과 무관하며 "12 실패"는 Linux 기준이다. + +### 7. 경계(#17) +새 판정(병합·검사·출력 경로, 닫기 조건)은 모두 Python(herdr_config.py, herdr_restore.py)에 있다. bash 쪽 추가는 호출과 빈 값 경고(lib.sh:241-245, resume_session.sh:240-242)뿐이다. + +## 테스트 실행 요약 +| 실행 | 결과 | +|---|---| +| 브리프 지정 7개 파일 (pr-48 .venv, Python 3.9.6) | **수집 오류로 중단, 0건 실행, rc 2** (B1) | +| herdr_config 제외 6개 파일 | **5 failed** / 192 passed / 1 skipped (B2 4건, B3 1건; skip = .claude/skills, 워크트리 정상) | +| test_issue45_herdr_config.py (python3.11) | 51 passed | +| test_issue45_restore.py 이식성 2곳만 고친 사본 | 5 passed; A·B 수정을 끄면 각각 실패 | +| test_issue10_13_purge.py | 23 passed | +MQTT_* unset, .mam.env 미export. 실제 .mam/jobs 에 새 항목 없음. + +## 수정 제안 (최소) +1. test_issue45_herdr_config.py:16 → `tomllib = pytest.importorskip("tomllib")`. +2. test_issue45_restore.py:163 → `Path(tempfile.mkdtemp(...)).resolve()`; :227 → `/proc` 가 없으면 `ps -E -ww -o command= -p ` 로 확인(또는 그 단언만 Linux 한정). +3. test_issue9_server_env.py:157 → bootstrap 이 쓸 `python3` 에 tomllib 이 없으면 skip 하거나 MINIMAL 을 기대. +4. (비차단) D1 문서화. + +[VERDICT: NOT PASS] diff --git a/.agents/reports/planner-reviewer-claude-01/report-ab18016d.identity.json b/.agents/reports/planner-reviewer-claude-01/report-ab18016d.identity.json new file mode 100644 index 0000000..08b0dff --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-ab18016d.identity.json @@ -0,0 +1,14 @@ +{ + "loop_id": "loop-20261005T085016Z-c0f6eb", + "base_sha": "e8d9151226d8139c7b5688e83d035a3a747aeb2d", + "head_sha": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "tree_sha": "f49ffdc8e95dc316f752fd81b2d55a0efddf1a12", + "review_commit": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "reviewer": "planner-reviewer-claude-01", + "job_id": "ab18016d", + "verdict": "PASS", + "transport": "herdr", + "evidence": "review-tree", + "isolation": "worktree", + "verified_at": "2026-10-05T09:05:24Z" +} \ No newline at end of file diff --git a/.agents/reports/planner-reviewer-claude-01/report-ab18016d.md b/.agents/reports/planner-reviewer-claude-01/report-ab18016d.md new file mode 100644 index 0000000..61f00ca --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-ab18016d.md @@ -0,0 +1,50 @@ +# 리뷰 리포트 — Job ab18016d (PR #48 3회차 델타, e8d9151..ab93776) + +- 리뷰어: planner-reviewer-claude-01 (claude) +- 리뷰 identity: loop-20261005T085016Z-c0f6eb / base e8d9151 / head ab93776 / tree f49ffdc — 검증 경로에서 `git rev-parse HEAD 'HEAD^{tree}'` 일치, 미커밋 변경 없음 +- 범위(항목 4): lib_py/herdr_config.py(+25/−2), lib_py/herdr_restore.py(+10/−2), 테스트 2개. bash·문서·버전 변경 없음. 새 판정은 모두 Python 에 있다(#17). + +## 결론 요약 +codex R1~R3 가 닫혔다. R1 의 "MINIMAL 무검사 사용" 판단에 동의한다. `_same_doc` 은 nan 외에는 이전 `==` 와 같거나 더 엄격하며, 그 엄격함이 오탐(불필요한 MINIMAL 폴백)을 만들지 않음을 확인했다. 차단 결함 없음. + +## 1. R1~R3 종결 +| 지적 (codex 2회차) | 수정 | 확인 | +|---|---|---| +| R1 `config check` 를 실행 못 하면(timeout·OSError) 병합 결과를 검사된 것으로 넘김 | herdr_config.py:245-258 — 병합 본문이면 경고 후 `continue` 로 MINIMAL 시도, MINIMAL 의 검사도 실행 못 하면 MINIMAL 을 경고와 함께 사용 | test_an_unrunnable_check_never_hands_over_an_unchecked_merge[timeout/oserror]: 스키마 오류가 든 사용자 설정이 생성 파일에 들어가지 않고 MINIMAL + "not using an unchecked merge" 경고, 원본 불변. 이전 제품 코드에서 2건 실패 | +| R2 `_shell_in_foreground` 가 0/0·False/False·""/""·-1/-1 에서 True | herdr_restore.py:44-53 — 두 값이 bool 아닌 양의 int 일 때만 비교 | test_issue45_restore_unit.py: zero·false·empty·negative·null·string·true·float 모두 닫지 않음, 정상 pid 같음 → 닫음, 다름 → 둠. 이전 코드에서 7건 실패(null 은 이전에도 통과) | +| R3 재파싱 `==` 가 nan≠nan 이라 nan 하나로 사용자 설정 전체를 버림 | herdr_config.py:140-150 `_same_doc`, :171 에서 사용 | test_a_nan_value_does_not_drop_the_other_settings[top/array/nested]: 경고 없이 설정 유지. 이전 코드에서 3건 실패 | +이전 제품 코드(e8d9151 의 두 파일)로 되돌린 스크래치 사본에서 새 테스트는 **12 failed / 55 passed**(3.11), 현재 코드에서 **67 passed**. + +## 2. R1 판단 — MINIMAL 무검사 사용 vs 경로 반환 안 함 +동의한다. 근거: +- 경로를 반환하지 않으면 lib.sh:241-245 가 `HERDR_CONFIG_PATH` 없이 서버를 띄운다 → herdr 는 사용자 설정(복원 기본 켜짐)을 읽어 **#45 가 확실히 재발**한다. +- MINIMAL 을 넘기면, herdr 가 그것을 받아들이는 경우(고정 2줄, 실제 herdr 0.9.1 에서 test_real_herdr_accepts_every_result 가 확인) 복원이 꺼지고, 만약 거부하더라도 herdr 는 기본값으로 돌아 결과는 "경로 없음"과 같은 복원 켜짐이다. 즉 MINIMAL 은 복원 측면에서 "경로 없음"보다 나쁜 경우가 없고, 잃는 것은 이미 경고되는 사용자 설정뿐이다. +- 검사가 **실행되어 거부**한 경우는 여전히 None(:259-262)이다. "검사 불가"와 "검사 거부"를 다르게 다루는 것이 일관적이다(거부는 herdr 가 그 파일을 버린다는 증거). +- 알 수 없는 nonzero 종료를 통과로 두는 것(:178)도 기존 근거(무해한 경고에도 nonzero)와 같고 이번 변경 범위 밖이다. + +## 3. `_same_doc` 의 의미 (항목 3) +3.11 에서 이전 `==` 와 직접 비교: +| 경우 | 이전 == | _same_doc | +|---|---|---| +| nan / nan, 중첩 nan | False | **True** (의도된 변경) | +| 1 / 1.0, True / 1, 0 / False | True | **False** (더 엄격) | +| -0.0 / 0.0, inf / inf, 같은 time, 같은 문자열 | True | True | +| date / datetime | False | False | +| [1,2] / [2,1] (배열 순서) | False | False | +| dict 키 순서만 다름, 표 배열 [{…}] | True | True | +- 더 엄격해진 int/float/bool 구분은 직렬화가 타입을 보존하는 한 오탐을 만들지 않는다: dumps(:107-126)는 int→정수, float→repr(소수점·지수 유지), bool→true/false 로 쓴다. 날짜·오프셋 datetime·소수 초 local time·표 배열·inf·nan·−nan·따옴표 점 키·1e100 이 든 문서로 server_config_text 를 돌려 **설정 유지, 경고 없음**을 확인했다. 오히려 왕복에서 1 이 1.0 으로 바뀌는 것 같은 결함은 이제 잡힌다. +- 참고: `-nan` 은 `nan` 으로 쓰여 부호가 사라진다(nan 끼리 같다고 보므로 통과). herdr 설정에서 nan 부호가 의미를 가질 일은 없어 무해하다. + +## 4. 테스트 실행 +| 실행 | 결과 | +|---|---| +| 지정 4개 파일 (pr-48 .venv, Python 3.9, 실제 herdr 포함) | **53 passed / 34 skipped / 0 failed** (76 s). skip = tomllib 전용; test_issue45_restore 는 skip 0 | +| herdr_config + restore_unit (지정 venv311, Python 3.11.15) | **67 passed** | +| 같은 테스트, 이전 제품 코드 사본 | 12 failed / 55 passed | +격리: 실행 전후 herdr 프로세스는 기존 multi-agent-mux 서버·attach·오케스트레이터 `agent wait` 뿐, `/private/tmp/m45*` 없음, `~/.config/herdr` 목록·config.toml md5·sessions 불변. MQTT_* unset, .mam.env 미export. + +## 5. 비차단 관찰 +- R1 테스트는 두 검사 모두 실행 불가인 경우만 다룬다. "병합 검사는 실행 불가, MINIMAL 검사는 실행되어 통과" 경로(:256 → 다음 반복의 :258)는 코드상 자명하지만 별도 테스트는 없다. +- 2회차의 VERSIONS 4.8.5 항목 1 문장 순서 지적(N1)은 이번 델타에 포함되지 않았다(문서 변경 없음). + +[VERDICT: PASS] diff --git a/.agents/reports/planner-reviewer-codex-01/report-714c5189.md b/.agents/reports/planner-reviewer-codex-01/report-714c5189.md new file mode 100644 index 0000000..bc51b0c --- /dev/null +++ b/.agents/reports/planner-reviewer-codex-01/report-714c5189.md @@ -0,0 +1,123 @@ +# PR #48 — Codex review, job 714c5189 + +## 판정: NOT PASS + +테스트 이식성 수정은 확인했다. 다만 현재 제품 코드에서 아래 R1–R3을 재현했다. 기존 스위트가 통과해도 설정 검증 실패 시 복원이 다시 켜질 수 있고, 비정상 프로세스 응답에서 pane을 닫으며, 유효한 NaN 설정에서 다른 설정을 유실한다. 국소 수정과 회귀 테스트로 해결 가능한 범위이므로 재계획 escalation은 필요하지 않다. + +## 검증 identity 및 범위 + +- loop: `loop-20261005T081844Z-e92124` +- base: `c5c7eff6f758c76ac37a138dbc3c301f3f7e10cf` +- head: `e8d9151226d8139c7b5688e83d035a3a747aeb2d` +- tree: `8b39188453ff8a7263aa4400792fabc99188dad6` +- 검증 cwd: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T081844Z-e92124/1` +- Codex는 PR 전체 `c5c7eff..e8d9151`을 검토했다. 두 후속 커밋은 테스트와 문서만 변경했으며 R1–R3 제품 코드는 이전 head와 동일하다. +- 검증 전후 HEAD/tree 동일, `git status --porcelain` 출력 없음. 제품 코드·테스트·문서는 수정하지 않았다. + +## 차단 사항 + +### R1 — P1: config check 실행 실패를 성공으로 반환해 스키마 오류 파일을 서버에 넘긴다 + +**근거:** `.agents/skills/lib_py/herdr_config.py:235–237`은 `OSError` 또는 `subprocess.SubprocessError`를 받으면 경고 후 `True`를 반환한다. 호출부 `:265–266`은 해당 파일 경로를 성공 결과로 반환하고, `.agents/skills/lib.sh:255–256`은 그 경로를 서버의 `HERDR_CONFIG_PATH`로 설정한다. `VERSIONS.md:49`의 “config check를 통과해야 한다” 계약과 다르다. + +**재현:** + +1. Python 3.11에서 임시 원본 파일에 다음을 쓴다. + ```toml + [ui] + confirm_close = "invalid-bool" + ``` +2. `herdr_config.herdr_rejects`가 `subprocess.TimeoutExpired(["herdr", "config", "check"], 30)`를 던지도록 주입하고 `write_server_config({"HERDR_CONFIG_PATH": source}, herdr="/review/fake-herdr")`를 호출한다. +3. 반환된 경로의 내용을 확인한다. 별도로 버리는 HOME/XDG/세션에서 실제 herdr의 `config check`로 그 내용을 검사한다. + +**기대:** 검증하지 못한 병합 파일을 “사용 가능한 설정”으로 반환하지 않아야 한다. 최소 설정으로의 안전한 전환을 시도하고, 그것도 검증 불가하면 실패를 명시해야 한다. + +**실제:** 유효한 경로와 “the config is unchecked” 경고를 반환하며, 파일에는 `confirm_close = "invalid-bool"`이 남는다. 동일 내용의 실제 herdr 검사는 exit 1, `invalid type ... expected a boolean; using defaults`를 반환했다. 이 파일로 서버가 시작하면 파일 전체가 거부되어 `resume_agents_on_restore=false`도 적용되지 않는다. 검사 실행 timeout을 주입한 재현이며, 실제 운영 서버에서 timeout을 유발하지 않았다. + +**수정 방향:** 예외를 성공으로 취급하지 말고 명시적 검사 실패로 처리한다. 최소 설정의 검증에도 실패하면 경로를 반환하지 않도록 하고, 호출부의 기존 실패 경고와 연결한다. timeout/OSError 회귀 검사를 추가한다. `:178`의 알 수 없는 nonzero 종료도 무조건 허용하지 말고, 파일 전체를 버리지 않는 것으로 확인된 경고와 검사 자체 실패를 구별하는 편이 안전하다. + +### R2 — P2: 비정상 PID 값의 단순 동등 비교로 pane 닫기를 허용한다 + +**근거:** `.agents/skills/lib_py/herdr_restore.py:44–47`은 `shell_pid is not None` 및 전경 그룹과의 동등 여부만 검사한다. `:63–65`는 이 결과만으로 `pane close`를 호출한다. + +**재현:** + +1. `_herdr`를 stub으로 바꾸고 `pane list`가 아래 pane을 반환하게 한다. + ```python + {"pane_id": "probe-pane", "agent": None, + "agent_session": {"value": "probe-id"}} + ``` +2. `pane process-info`가 `{"process_info": {"shell_pid": 0, "foreground_process_group_id": 0}}`을 반환하게 한다. +3. `close_restored_shells("probe-id", session="m45-review-probe")`를 호출하고 stub 호출 이력을 검사한다. 같은 검사를 `False`, 빈 문자열, `-1`로 반복한다. + +**기대:** 유효한 셸 PID/전경 프로세스 그룹인지 입증하지 못한 응답은 pane을 그대로 두어야 한다. + +**실제:** 네 경우 모두 `("pane", "close", "probe-pane")`를 호출하고 `["probe-pane"]`를 반환했다. 실제 pane은 닫지 않았으며 응답 주입으로 닫기 요청을 확인했다. 현재 실제 herdr가 이 값을 반환한다고 주장하는 것은 아니다. 브리프가 명시한 “응답이 이상할 때 fail-safe” 조건을 위반하는 재현이다. + +**수정 방향:** 두 값이 bool이 아닌 양의 정수인지 검사한 뒤 동등 여부를 비교한다. 누락·null·0·음수·문자열·bool 응답에서 닫기 요청이 없는지 회귀 검사한다. + +### R3 — P2: 유효한 TOML NaN을 불일치로 판단해 무관한 사용자 설정을 버린다 + +**근거:** `.agents/skills/lib_py/herdr_config.py:159`의 일반 dict 동등 비교에서 재파싱된 `nan`은 원래 `nan`과 같지 않다. 따라서 `:162–163`이 전체 설정을 MINIMAL로 교체한다. `:112–114`에는 NaN 직렬화 지원까지 있지만 동등 검사가 이를 거부한다. + +**재현:** + +```python +body, warnings = herdr_config.server_config_text( + "[ui]\nconfirm_close = false\n[custom]\nvalue = nan\n" +) +``` + +Python 3.11에서 `tomllib.loads(body)`의 `ui.confirm_close`를 확인한다. + +**기대:** 원본 설정과 복원 키만 false로 바꾼 결과가 유지되어야 한다. NaN 값의 의미상 동등성도 처리해야 한다. + +**실제:** 출력은 `[session]\nresume_agents_on_restore = false\n`뿐이다. `ui.confirm_close=false`가 사라지고 “could not rewrite ... faithfully” 경고가 나온다. 버리는 환경의 실제 herdr 검사에서 원본은 unknown `[custom]` 섹션 경고만 냈으며 `parse error/using defaults`는 없었다. 즉 추가 섹션을 무시하는 것과 정상 UI 설정까지 버리는 것은 다르다. Python 3.11에서도 발생하므로 사용자가 승인한 Python 3.9 최소 설정 폴백과 별개의 문제다. + +**수정 방향:** 중첩 dict/list의 구조 및 값 비교 시 두 NaN을 의미상 동등하게 취급한다. 최상위·중첩·배열 NaN을 포함해 무관한 유효 설정이 유지되는 회귀 검사를 추가한다. + +## 실행 결과 + +| 검증 | 결과 | +| --- | --- | +| 지정 Python 3.9, 브리프의 6개 파일 + `tests/test_issue10_13_purge.py` | **175 passed, 30 skipped**, 226.24초 | +| 위 결과 중 purge 파일 | **23 passed**; 실패 없음 | +| Python 3.11, `tests/test_issue45_herdr_config.py` | **51 passed**, 0.42초 | +| 현재 head의 독립 경계 탐침: 검사 timeout 1 + 비정상 PID 4 + NaN 1 | **6 failed**, 0.03초; R1–R3 재현 | +| `git diff --check c5c7eff..HEAD` | 통과 | +| 변경 bash 두 파일 `bash -n`, 신규 Python 두 모듈 AST parse | 통과 | +| 검증 후 작업 트리 / 실제 테스트 서버 정리 | clean / 해당 runner의 서버·세션 잔류 없음 | + +30 skips 중 29건은 tomllib 미설치 환경에서 의도된 설정 병합 검사 skip이다. 나머지 1건은 기존 checkout 의존 검사다. 따라서 Python 3.9 결과만으로 3.11 병합 기능이 검증됐다고 간주하지 않았고, 3.11의 51건을 따로 실행했다. + +- 선택 테스트 로그: `/tmp/714c5189-selected.log` +- Python 3.11 설정 로그: `/tmp/714c5189-config311.log` +- 독립 탐침: `/tmp/714c5189_extra_probes.py` +- 독립 탐침 로그: `/tmp/714c5189-probes.log` + +독립 탐침 재실행(위 검증 cwd에서): +```sh +PYTHONDONTWRITEBYTECODE=1 /tmp/270e8aaa-py311/bin/python -m pytest -p no:cacheprovider -q --basetemp=/tmp/714c5189-probes-rerun /tmp/714c5189_extra_probes.py +``` +이 탐침은 실제 herdr를 실행하거나 실제 pane을 닫지 않는다. 임시 파일과 stub만 사용한다. + +## 브리프 1–7 확인 + +1. **병합/충돌:** 기본 TOML 표기, 기존 session 키, 원본 없음·깨짐, 생성 파일의 원본 추적, 원본/외부 파일 충돌 및 폴백 검사는 3.11 스위트에서 통과했다. 원본 파일은 유지되고 생성 파일이 원본 옆에 남는다. 충돌 시 원본별 해시 이름, 쓰기 불가 시 .mam 폴백과 상대 경로 의미 변경 경고가 있다. 단 R1/R3 때문에 전체 무손실·검사 계약은 통과로 볼 수 없다. +2. **기동 경로:** `lib.sh:193–212,241–261`의 named-session headless bootstrap에 적용된다. create/resume도 이 shim을 이용한다. default 서버, MAM_NO_AUTOSTART 조회 경로, 이미 실행 중인 서버, 사용자가 직접 attach로 띄운 서버에는 새 설정을 적용하지 않는다. create SKILL.md:119–120 및 VERSIONS.md:50의 다음 기동/직접 기동 예외 설명과 부합한다. 이미 떠 있는 서버를 재설정하는 기능은 추가하지 않았다. +3. **pane 정리:** 정상 응답의 다른 session id, 사용자 pane, live agent, 셸 이외 전경 프로세스 보호와 create→crash→resume 정리는 실서버 테스트에서 통과했다. 다만 비정상 응답은 R2처럼 닫히므로 fail-safe 항목은 미충족이다. +4. **실서버 테스트/격리:** 후속 수정의 resolve된 임시 HOME 및 macOS ps 환경 조회로 두 A 테스트와 B 테스트가 통과했다. A의 마지막 양성 대조군도 실행돼 일반 서버에서 같은 저장 레이아웃의 agent resume을 확인했다. 실행 wrapper에서 HERDR_*를 모두 제거하고 버리는 HOME/XDG_CONFIG_HOME을 사용했다. 실제 테스트는 uuid4 기반 `mamt45-66197-*` 세션만 사용했으며, 종료 후 해당 서버 프로세스 및 세션을 담은 임시 루트가 없음을 확인했다. 사용자 서버/설정/소켓 및 .mam.env를 사용하거나 변경하지 않았다. +5. **문서/버전/#42:** v4.8.5 bump는 승인 범위이며 일관성 검사를 통과했다. Python 3.9에서 다른 설정을 보존하지 않는 동작은 새 문서에 명시됐고 사용자의 문서화 결정을 차단 사유로 삼지 않았다. 인벤토리는 create 문서의 start/end 줄 번호와 removed 항목의 근거 줄 번호만 변경됐고 class/hash 재분류는 없다. 검사 통과 필수라는 문서와 R1 코드의 불일치는 수정해야 한다. +6. **purge 12건 기준 실패:** 지정 Python 3.9에서는 23건 모두 통과했다. 이전 head의 추가 Python 3.11 실행에서는 같은 호스트에서도 12건이 실패했다. PR에 purge 코드/테스트 변경은 없으며, PR 모듈을 전혀 import하지 않은 SQLite 최소 재현으로 원인을 분리했다: DB 옆에 가짜 WAL/SHM을 만든 뒤 열어 SELECT하면 Python 3.9.6/SQLite 3.51.0은 두 파일을 보존하고, Python 3.11.15/SQLite 3.53.3은 둘 다 제거한다. 기존 `agy.py:74–76`의 DB 검사와 `tests/test_issue10_13_purge.py:83–111` fixture 가정의 환경 차이다. “Linux에서만 발생”으로 단정하면 부정확하지만 이 PR이 도입한 회귀라는 근거는 없다. +7. **Python 경계:** 새 설정 정책은 herdr_config.py, pane 판정은 herdr_restore.py에 있다. resume bash는 호출과 경고만 추가하며, lib.sh는 Python 생성 결과를 서버 환경으로 전달한다. 신규 비즈니스 판정을 bash에 추가한 것으로 보지 않는다. + +## 후속 커밋으로 해결된 사항 + +| 이전 이식성 문제 | 현재 상태 | +| --- | --- | +| Python 3.9에서 무조건 tomllib import로 수집 중단 | 해결: 선택 import와 해당 29건 skip, 3.11에서 별도 51건 통과 | +| macOS에 없는 /proc 환경 조회 | 해결: ps -E 대체 경로로 실제 A 검사 및 양성 대조 통과 | +| /tmp와 /private/tmp 문자열 차이 | 해결: 임시 루트 resolve 후 B 검사 통과 | +| issue9 테스트의 실행 Python과 bootstrap Python 차이 | 해결: PATH python3 기준 검사 및 폴백 경고 검증 통과 | + +[VERDICT: NOT PASS] diff --git a/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.identity.json b/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.identity.json new file mode 100644 index 0000000..9f91e8a --- /dev/null +++ b/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.identity.json @@ -0,0 +1,14 @@ +{ + "loop_id": "loop-20261005T085016Z-c0f6eb", + "base_sha": "e8d9151226d8139c7b5688e83d035a3a747aeb2d", + "head_sha": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "tree_sha": "f49ffdc8e95dc316f752fd81b2d55a0efddf1a12", + "review_commit": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "reviewer": "planner-reviewer-codex-01", + "job_id": "b8dbd280", + "verdict": "PASS", + "transport": "herdr", + "evidence": "review-tree", + "isolation": "worktree", + "verified_at": "2026-10-05T09:05:24Z" +} \ No newline at end of file diff --git a/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.md b/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.md new file mode 100644 index 0000000..7e07c29 --- /dev/null +++ b/.agents/reports/planner-reviewer-codex-01/report-b8dbd280.md @@ -0,0 +1,93 @@ +# PR #48 — 3회차 Codex 델타 리뷰 + +## 판정: PASS + +이전 R1–R3이 해결됐다. 고정 MINIMAL 설정을 검사 실행 불가 시 경고와 함께 사용하는 R1의 대안도 이번 지원 범위에서 수용한다. 차단 결함은 발견하지 않았다. 문서 표현 정리 1건은 비차단 의견이다. + +## 검증 identity / 범위 + +- job: `b8dbd280` +- loop: `loop-20261005T085016Z-c0f6eb` +- base: `e8d9151226d8139c7b5688e83d035a3a747aeb2d` +- head: `ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9` +- tree: `f49ffdc8e95dc316f752fd81b2d55a0efddf1a12` +- 검증 경로: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T085016Z-c0f6eb/1` +- `git diff e8d9151..HEAD`의 제품 모듈 2개와 테스트 파일 2개만 델타 리뷰했다. 검증 전후 HEAD/tree 일치, 작업 트리 clean. +- 소스 수정·커밋·브랜치 변경 없음. 실행 중 사용자 herdr 서버·설정·소켓, .mam.env를 사용하거나 변경하지 않았다. + +## 이전 지적 종료 표 + +| 지적 | 현재 근거 | 검증 및 판단 | +| --- | --- | --- | +| R1: 검사 timeout/OSError에도 스키마 오류 병합 파일 반환 | `.agents/skills/lib_py/herdr_config.py:247–258` | 검사 불가한 병합 파일은 폐기하고 MINIMAL로 전환한다. timeout/OSError 회귀 검사와 이전 독립 탐침 통과. **해결**, 아래 대안 판단 참조. | +| R2: 0/False/빈 문자열/음수의 동등 비교로 pane 닫기 | `.agents/skills/lib_py/herdr_restore.py:44–53` | 양쪽 값이 bool이 아닌 양의 정수일 때만 비교한다. 비정상·누락 응답에서 닫기 없음, 정상 셸은 닫고 busy pane은 유지한다. 이전 비정상 PID 탐침 4건도 통과. **해결**. | +| R3: NaN 때문에 무관한 정상 설정까지 유실 | `.agents/skills/lib_py/herdr_config.py:140–149,171` | 재귀 비교에서 NaN끼리 동등하게 처리한다. 중첩/배열 NaN 및 기존 UI 설정 유지 검사, 이전 독립 탐침 통과. **해결**. | + +## R1: MINIMAL을 검사 실행 불가 시 사용하는 선택 + +이전 보고서에서는 MINIMAL 검사도 실패하면 경로를 반환하지 않는 방안을 제안했다. 이번 구현은 그 제안을 그대로 따르지는 않지만, 원래 결함의 핵심인 **검사되지 않은 사용자 병합 내용의 사용**을 막으며 #45의 복원 비활성화 목적을 더 잘 보존하므로 수용한다. + +- MINIMAL은 사용자 스키마 값과 분리된 고정 `[session]\nresume_agents_on_restore = false\n` 본문이다. +- 실제 herdr 설정 검사와 실서버 복원 검사가 이 설정을 받아들이고 복원을 끄는 것을 검증했다. +- 첫 검사 timeout 뒤 두 번째 MINIMAL 검사가 성공하는 경로를 별도 주입 검사했다. 호출 2회, 반환 파일은 MINIMAL이었다. +- 두 번 모두 검사 실행이 불가능한 경우에도 병합 내용은 남지 않고, `using the fixed minimal config unchecked` 경고로 미검증 상태를 밝힌다. +- 검사기를 실행했고 파일을 명시적으로 거부한 경우는 예외와 다르다. 병합/MINIMAL 양쪽에 명시적 거부 응답을 주입하면 여전히 경로를 반환하지 않는다(`:259–263`). 이 흐름도 별도로 확인했다. +- 아무 파일도 넘기지 않는 기존 bootstrap 경로는 복원이 켜진 기본값으로 시작할 수 있으므로, 알려진 고정 설정을 넘기는 선택에는 구체적인 근거가 있다. + +이 판단은 PR이 대상으로 삼은 herdr의 알려진 설정 스키마에 대한 것이다. “실행하지 못한 검사가 통과했다”는 의미는 아니다. + +알 수 없는 nonzero 종료의 판정은 이번 커밋에서 바뀌지 않았다. herdr가 무해한 unknown-section 경고에도 nonzero를 반환하므로, 모든 nonzero를 파일 전체 거부로 취급하는 변경을 R1 종료의 조건으로 요구하지 않는다. + +## _same_doc 의미 확인 + +**이전 일반 `==`와 완전히 동일하지 않다.** `:149`가 동일한 타입을 요구하므로 NaN 처리 외에 int/float/bool 교차 동등성도 더 엄격해졌다. 이는 TOML 재직렬화의 타입 보존 검증에 적절하며, 현재 writer가 유지하는 유효 값의 정상 왕복을 깨뜨리는 사례는 찾지 못했다. + +| 비교 | 이전 == | 현재 _same_doc | 판단 | +| --- | --- | --- | --- | +| `1` / `1.0` | true | false | int/float 타입 변경을 감지 | +| `True` / `1`, `False` / `0.0` | true | false | bool/숫자 혼동 방지 | +| 같은 타입의 int/bool/string | 기존 값 비교 | 동일 | 유지 | +| `nan` / `nan` | false | true | 의도한 수정 | +| `nan` / 일반 숫자 | false | false | 유지 | +| `0.0` / `-0.0`, 같은 infinity | true | true | 유지 | +| 같은 datetime, 같은 순간의 다른 UTC offset | true | true | 유지 | +| date / datetime | false | false | 유지 | +| 배열 순서·길이 차이 | false | false | 유지 | +| dict 키 차이 | false | false | 유지 | +| dict/list 안의 새로 생성한 NaN들 | false | true | 재귀 수정 | + +datetime/time offset, 중첩 NaN, 배열 길이·순서 등을 포함한 독립 비교 18건을 실행해 기대 결과를 확인했다. 이 비교는 파서가 반환하는 TOML 자료형을 대상으로 하며 임의 사용자 정의 Python 객체를 위한 범용 동등 비교를 요구하지 않는다. + +## 실행 결과 + +모든 실행 cwd는 위 검증 경로였다. Python bytecode 및 pytest cache를 끄고 basetemp는 검증 경로 밖에 두었다. + +| 검사 | 결과 | +| --- | --- | +| 지정 Python 3.9: herdr_config, restore_unit, 실제 restore, issue9_server_env | **53 passed, 34 skipped**, 72.81초 | +| 브리프의 Python 3.11: herdr_config, restore_unit | **67 passed**, 0.40초 | +| 이전 리뷰의 독립 결함 탐침 파일 그대로 현재 head에서 실행 | **6 passed**, 0.02초 (이전 head에서는 6 failed) | +| _same_doc 독립 비교 | **18건 통과** | +| 검사 timeout→MINIMAL 성공 / 양쪽 명시적 거부 | **2개 흐름 통과** | +| git diff --check 및 변경 Python 모듈 AST parse | 통과 | + +3.9의 34 skip은 tomllib이 필요한 검사다. 이를 보완해 3.11에서 해당 두 파일 전체 67건을 실행했다. 실제 herdr 복원 및 양성 대조 테스트도 3.9 실행에 포함되어 통과했다. + +- 3.9 로그: `/tmp/b8dbd280-selected39.log` +- 3.11 로그: `/tmp/b8dbd280-selected311.log` +- 이전 탐침 파일: `/tmp/714c5189_extra_probes.py` +- 이전 탐침 재실행: `PYTHONDONTWRITEBYTECODE=1 /tmp/270e8aaa-py311/bin/python -m pytest -p no:cacheprovider -q --basetemp=/tmp/b8dbd280-prior-probes /tmp/714c5189_extra_probes.py` + +실행 wrapper에서 상속된 HERDR_*를 제거하고 버리는 HOME/XDG_CONFIG_HOME을 사용했다. 실제 테스트 세션은 uuid4 기반 `mamt45-84511-*`였으며, 종료 후 해당 서버 프로세스 및 해당 세션을 담은 임시 루트가 남아 있지 않음을 확인했다. + +## 비차단 의견 C1 — 문서의 검사 필수 표현을 예외와 맞추기 + +`.agents/skills/lib_py/herdr_config.py:23–26,239–240`과 `VERSIONS.md:49`에는 결과가 반드시 herdr 검사에서 승인된다는 표현이 남아 있다. 이번 `:247–258`의 고정 MINIMAL 예외는 코드 주석과 런타임 경고로 설명되지만 이 문서 표현에는 반영되지 않았다. + +“사용자 설정을 병합한 파일은 검사 불가 시 사용하지 않으며, 검사기를 실행할 수 없는 경우 고정 최소 설정을 경고와 함께 사용한다”는 예외를 문서와 docstring에 추가하면 정확해진다. 제품의 실패 처리와 경고는 충분히 명시적이므로 이 문서 보완을 이번 PASS의 차단 조건으로 삼지 않는다. + +## 범위 + +변경은 기존 세 지적의 설정 검사 실패 처리, PID 검증, NaN 재파싱 비교와 회귀 테스트에 한정된다. bash 정책 추가, 버전 변경, 인벤토리 분류 변경, 승인되지 않은 already-running 검사나 무명 에이전트 종료 기능은 없다. 재계획은 필요하지 않다. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-agy-01/report-3e6f0467.identity.json b/.agents/reports/reviewer-agy-01/report-3e6f0467.identity.json new file mode 100644 index 0000000..998e8a5 --- /dev/null +++ b/.agents/reports/reviewer-agy-01/report-3e6f0467.identity.json @@ -0,0 +1,14 @@ +{ + "loop_id": "loop-20261005T085016Z-c0f6eb", + "base_sha": "e8d9151226d8139c7b5688e83d035a3a747aeb2d", + "head_sha": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "tree_sha": "f49ffdc8e95dc316f752fd81b2d55a0efddf1a12", + "review_commit": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "reviewer": "reviewer-agy-01", + "job_id": "3e6f0467", + "verdict": "PASS", + "transport": "herdr", + "evidence": "review-tree", + "isolation": "worktree", + "verified_at": "2026-10-05T09:05:24Z" +} \ No newline at end of file diff --git a/.agents/reports/reviewer-agy-01/report-3e6f0467.md b/.agents/reports/reviewer-agy-01/report-3e6f0467.md new file mode 100644 index 0000000..6710309 --- /dev/null +++ b/.agents/reports/reviewer-agy-01/report-3e6f0467.md @@ -0,0 +1,115 @@ +# PR #48 (fix/issue-45, Closes #45) Round 3 Delta Review Report + +- **Reviewer Agent**: `agy` (session: `herdr:reviewer-agy-01`) +- **Job ID**: `3e6f0467` +- **Verification Directory**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T085016Z-c0f6eb/1` +- **Base Commit**: `e8d9151226d8139c7b5688e83d035a3a747aeb2d` (Round 2 head) +- **Head Commit**: `ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9` (Round 3 head) +- **Delta Under Inspection**: `git diff e8d9151 ab93776` (Commit `ab93776`) + +--- + +## 1. Executive Summary + +In Round 2, `claude`, `agy`, and `grok` reached a `PASS` verdict after verifying that Python 3.9 macOS environment and portability blockers were eliminated. However, `codex` reported three valid edge-case issues in the product code (`R1`: unchecked merge pass-through on `herdr config check` execution failure; `R2`: unsafe type coercion in `_shell_in_foreground` comparing non-PIDs with `==`; `R3`: IEEE 754 `NaN != NaN` dropping valid TOML configurations containing `nan`). + +Commit `ab93776` surgically addresses all three findings (R1, R2, R3) in `lib_py/herdr_config.py` and `lib_py/herdr_restore.py`, accompanied by targeted unit tests in `tests/test_issue45_herdr_config.py` and `tests/test_issue45_restore_unit.py`. + +All review criteria have been audited, analyzed, and verified across both Python 3.9 and Python 3.11 test environments. The modifications are minimal, robust, fail-safe, and fully within scope. + +--- + +## 2. Review Findings (R1 ~ R3) Resolution Matrix + +| # | Item | Defect Description | Resolution in `ab93776` | Status | +|---|---|---|---|---| +| **R1** | Unchecked merge pass-through on check failure | If `herdr config check` failed to execute (timeout, `OSError`), `_write_checked` previously treated the merged file as valid and returned `True`. If the unvalidated merge contained a schema error, `herdr` would silently discard the file on startup and re-enable restore, causing Issue #45 to recur. | In `lib_py/herdr_config.py:_write_checked`, an execution error during candidate validation skips any unchecked merged body and advances to `MINIMAL`. If `MINIMAL` also encounters a check execution error, it writes `MINIMAL` with a warning rather than returning no config. | **CLOSED** (Verified) | +| **R2** | Loose equality in `_shell_in_foreground` | `_shell_in_foreground` compared `shell` and `foreground_process_group_id` directly with `==`, causing matching non-PIDs (`0 == 0`, `False == False`, `"" == ""`, `-1 == -1`) to evaluate to `True` and mistakenly close active panes. | Added `_is_pid(v)` helper: `isinstance(v, int) and not isinstance(v, bool) and v > 0`. `_shell_in_foreground` now requires both `shell` and `fg` to be valid positive integer PIDs. | **CLOSED** (Verified) | +| **R3** | `NaN` equality dropping valid configs | Standard Python equality `tomllib.loads(text) == expected` fails for valid TOML `nan` values because `float('nan') != float('nan')`, causing all user settings to be discarded in favor of minimal config. | Implemented `_same_doc(a, b)` which recursively verifies dictionary keys and list items while treating `math.isnan(a) and math.isnan(b)` as equal and enforcing strict type equality (`type(a) is type(b)`). | **CLOSED** (Verified) | + +--- + +## 3. Deep-Dive on Specific Review Criteria + +### 3.1. Assessment of R1: Unchecked MINIMAL Fallback vs. Returning None +- **Context & Design Trade-off**: Codex suggested that if validating `MINIMAL` fails or cannot be executed, `write_server_config` should return `None` (no path). The Creator instead decided to emit a warning and return the `MINIMAL` config. +- **Analysis**: + - If `write_server_config` returns `None`, `lib.sh` starts `herdr` without setting `HERDR_CONFIG_PATH`. Herdr's built-in default is `resume_agents_on_restore = true`. When the server starts, every previously saved agent session would be relaunched without permissions, resurrecting the primary vulnerability of Issue #45. + - In contrast, `MINIMAL` is a static, immutable, two-line string: + ```toml + [session] + resume_agents_on_restore = false + ``` + This fixed content has zero dynamic variables and has been rigorously verified against `herdr 0.9.1+` (`test_real_herdr_accepts_every_result`). + - If `herdr config check` cannot be executed due to a transient execution error (e.g., subprocess timeout or file descriptor exhaustion), deploying `MINIMAL` provides the safest available defense-in-depth against unauthorized agent auto-restoration. +- **Verdict on Criterion 2**: The Creator's decision is sound, defensible, and superior in fail-safety compared to returning `None`. + +### 3.2. Assessment of R3: Equivalence of `_same_doc` +- `_same_doc(a, b)` enforces: + 1. `math.isnan(a) and math.isnan(b)` returns `True` for floats. + 2. Dict comparison checks `a.keys() == b.keys()` and recurses on all values. + 3. List comparison checks `len(a) == len(b)` and recurses element-wise in sequence, strictly preserving list order. + 4. Primitive comparison checks `type(a) is type(b) and a == b`. +- **Type Discrimination**: In Python, `0 == False`, `1 == True`, and `1 == 1.0` evaluate to `True`. Because TOML maintains distinct types for boolean, integer, and float, the added check `type(a) is type(b)` ensures that serialization does not inadvertently morph types (e.g. integer `1` into float `1.0`). +- **Verdict on Criterion 3**: `_same_doc` preserves identical behavior to `==` for all non-nan types while providing strictly higher fidelity TOML AST verification. + +### 3.3. Scope and Surgical Precision +- Modified lines: + - `lib_py/herdr_config.py`: +31 lines, -2 lines. + - `lib_py/herdr_restore.py`: +9 lines, -3 lines. + - `tests/test_issue45_herdr_config.py`: +38 lines. + - `tests/test_issue45_restore_unit.py`: +56 lines (new unit test file for stubbed PID verification). +- No extraneous changes or unrelated formatting edits were introduced. + +--- + +## 4. Verification and Test Results + +Tests were executed directly within the isolated review verification path: +`/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T085016Z-c0f6eb/1` + +### 4.1. Round 3 Target Test Suite (Python 3.9 venv) +```bash +AGENT_PYTHON_BIN=/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ +PYTHONDONTWRITEBYTECODE=1 \ +/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ + -m pytest -p no:cacheprovider -q --basetemp=/tmp/pytest-pr48-r3 \ + tests/test_issue45_herdr_config.py \ + tests/test_issue45_restore_unit.py \ + tests/test_issue45_restore.py \ + tests/test_issue9_server_env.py +``` +- **Exit Code**: `0` +- **Result**: `53 passed, 34 skipped in 72.58s (0:01:12)` + +### 4.2. Python 3.11 Environment Verification +```bash +PYTHONPATH=.agents/skills \ +/private/tmp/claude-501/-Users-godopu16-PuKi-laa-canary-projects-multi-agent-mux/dbe6dd10-a210-4b46-b613-58a1860499b6/scratchpad/venv311/bin/python \ + -m pytest -p no:cacheprovider -q --basetemp=/tmp/pytest-pr48-r3-py311 \ + tests/test_issue45_herdr_config.py \ + tests/test_issue45_restore_unit.py +``` +- **Exit Code**: `0` +- **Result**: `67 passed in 0.42s` + +### 4.3. Full Auxiliary & Consistency Suite (Python 3.9 venv) +```bash +AGENT_PYTHON_BIN=/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ +PYTHONDONTWRITEBYTECODE=1 \ +/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ + -m pytest -p no:cacheprovider -q --basetemp=/tmp/pytest-pr48-r3-full \ + tests/test_issue23_24_startup_path.py \ + tests/test_issue42_skill_blocks.py \ + tests/test_version_consistency.py +``` +- **Exit Code**: `0` +- **Result**: `110 passed, 1 skipped in 137.12s (0:02:17)` + +--- + +## 5. Conclusion & Final Verdict + +All findings from the previous round have been resolved completely, test coverage is thorough and passing across both Python 3.9 and 3.11, and no regressions or scope violations exist. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-agy-01/report-3f82b402.md b/.agents/reports/reviewer-agy-01/report-3f82b402.md new file mode 100644 index 0000000..db2e197 --- /dev/null +++ b/.agents/reports/reviewer-agy-01/report-3f82b402.md @@ -0,0 +1,105 @@ +# PR #48 (fix/issue-45, Closes #45) Round 2 Delta Review Report + +- **Reviewer Agent**: `agy` (session: `herdr:reviewer-agy-01`) +- **Job ID**: `3f82b402` +- **Verification Directory**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T081844Z-e92124/1` +- **Base Commit**: `c5c7eff6f758c76ac37a138dbc3c301f3f7e10cf` (main, v4.8.4) +- **Head Commit**: `e8d9151226d8139c7b5688e83d035a3a747aeb2d` (v4.8.5) +- **Delta Under Inspection**: `35935f9`..`e8d9151` (commits `36f0b8d` and `e8d9151`) + +--- + +## 1. Executive Summary + +In Round 1 (`f3699a96`), the review concluded with `[VERDICT: NOT PASS]` strictly due to cross-platform and environment compatibility issues on macOS under Python 3.9 (missing `tomllib` in Python < 3.11, `/proc/{pid}/environ` absence on macOS, and `/tmp` vs `/private/tmp` symlink divergence causing test assertion failures). + +The Creator has applied targeted, high-quality fixes across commits `36f0b8d` and `e8d9151`. The changes are strictly confined to test suites and documentation, leaving production code (`lib.sh`, `lib_py/herdr_config.py`, `lib_py/herdr_restore.py`, `resume_session.sh`) stable and untouched. + +All 5 issues identified during Round 1 by `agy` have been completely and cleanly resolved. The test suite passes cleanly both under the host's Python 3.9 environment (152 passed, 30 skipped) and Python 3.11 (51 passed, 0 skipped). + +--- + +## 2. Round 1 Findings Resolution Matrix + +| # | Round 1 Finding (Job `f3699a96`) | File & Line | Round 2 Resolution (Commits `36f0b8d`, `e8d9151`) | Verification Status | +|---|---|---|---|---| +| **1** | Unconditional `import tomllib` crashing pytest collection under Python 3.9 | `tests/test_issue45_herdr_config.py:16` | Wrapped `import tomllib` in a `try...except ImportError` block; defined `@needs_tomllib` decorator skipping tests requiring TOML parsing when `tomllib is None` (22 passed, 29 skipped under 3.9; 51 passed under 3.11). | **CLOSED** (Verified) | +| **2** | Hardcoded `import tomllib` causing execution failure in `test_bootstrap_hands_the_server_a_no_restore_config` under Python 3.9 | `tests/test_issue9_server_env.py:154` | Removed inline `import tomllib`. Replaced with a dynamic subprocess parser invoking the active `python3` binary on PATH. Verifies full JSON parsed output when `tomllib` is present, or asserts fallback to minimal config and stderr warning when absent. | **CLOSED** (Verified) | +| **3** | Linux-specific `/proc/{pid}/environ` path causing `FileNotFoundError` on macOS | `tests/test_issue45_restore.py:227` | Added helper function `_process_env(pid)` that detects `/proc/{pid}/environ` existence, falling back gracefully to `ps -E -ww -o command= -p ` on macOS / BSD environments. | **CLOSED** (Verified) | +| **4** | macOS `/tmp` vs `/private/tmp` symlink mismatch causing failure in `assert cwd == str(rig.root)` | `tests/test_issue45_restore.py:263` | In fixture `rig`, resolved the temporary directory with `.resolve()` (`root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp")).resolve()`), ensuring canonical `/private/tmp` alignment with `herdr`'s reported cwd. | **CLOSED** (Verified) | +| **5** | macOS `/tmp` vs `/private/tmp` symlink mismatch causing `StopIteration` during pane cwd search | `tests/test_issue45_restore.py:325` | Resolving `root` via `Path(...).resolve()` resolved the cwd mismatch for `next(p for p in ... if p.get('cwd') == str(rig.root))`. | **CLOSED** (Verified) | + +--- + +## 3. Delta Code Review Details (`35935f9`..`e8d9151`) + +### 3.1. `tests/test_issue45_herdr_config.py` (Commit `36f0b8d`) +- **Import Guarding**: Gracefully handles missing `tomllib` standard library on Python < 3.11. +- **Skipping Strategy**: 29 unit tests asserting deep AST / dictionary equivalence of merged TOML configs are decorated with `@needs_tomllib`. Tests testing paths, file naming, collision avoidance, and permissions continue to execute under Python 3.9. +- **Verification**: Under Python 3.9, 22 tests pass and 29 skip without errors. Under Python 3.11, all 51 tests pass. + +### 3.2. `tests/test_issue45_restore.py` (Commit `e8d9151`) +- **Canonical Root Resolution**: `root = Path(tempfile.mkdtemp(prefix="m45", dir="/tmp")).resolve()` resolves macOS symlinks cleanly. +- **Cross-Platform Process Inspection**: `_process_env(pid: int)` cleanly splits `/proc/{pid}/environ` by `\0` when present, or extracts environment variables from `ps -E -ww -o command= -p `. + +### 3.3. `tests/test_issue9_server_env.py` (Commit `e8d9151`) +- Seamlessly validates dual behavior: + - If `python3` on `PATH` has `tomllib`, confirms full configuration merge `{"ui": {"confirm_close": False}, "session": {"resume_agents_on_restore": False}}`. + - If `python3` lacks `tomllib`, confirms minimal config fallback (`resume_agents_on_restore = false`), absence of unmerged user keys (`confirm_close`), and emission of the `"no tomllib"` warning on stderr. + +### 3.4. Documentation (`SKILL.md` & `VERSIONS.md`) +- `multi-agent-mux-create/SKILL.md:121` explicitly documents the runtime requirement: + > *"Building that file needs `tomllib` (Python 3.11+) in the `python3` on PATH; with an older `python3` (macOS ships 3.9) restore is still off, but the file holds only that key, so the MAM server runs without your other herdr settings and the bootstrap prints a warning saying so."* +- `VERSIONS.md` under `v4.8.5` changelog reflects both the Python 3.11+ requirement and the test suite portability improvements across macOS and Linux. + +--- + +## 4. Verification & Test Execution Results + +All verification was conducted strictly inside the designated review worktree (`/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T081844Z-e92124/1`). + +### 4.1. Primary Test Suite (Python 3.9 Host Environment) +```bash +AGENT_PYTHON_BIN=/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ +PYTHONDONTWRITEBYTECODE=1 \ +/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/bin/python \ + -m pytest -p no:cacheprovider -q --basetemp=/tmp/pytest-pr48-r2 \ + tests/test_issue45_herdr_config.py \ + tests/test_issue45_restore.py \ + tests/test_issue9_server_env.py \ + tests/test_issue23_24_startup_path.py \ + tests/test_issue42_skill_blocks.py \ + tests/test_version_consistency.py +``` +**Result**: +- **Exit Code**: `0` +- **Output**: `152 passed, 30 skipped in 211.50s (0:03:31)` + +### 4.2. Python 3.11 Compatibility Sanity Check +```bash +PYTHONPATH=.agents/skills:/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/pr-48/.venv/lib/python3.9/site-packages \ +/opt/homebrew/bin/python3.11 \ + -m pytest -p no:cacheprovider -q --basetemp=/tmp/pytest-pr48-py311 \ + tests/test_issue45_herdr_config.py +``` +**Result**: +- **Exit Code**: `0` +- **Output**: `51 passed in 0.40s` + +--- + +## 5. Non-Blocking Architectural Note + +- **Python < 3.11 Config Merging**: In Round 1, grok suggested bundling a `tomli` backport for environments with Python < 3.11. The project decision to rely on standard library `tomllib` (Python 3.11+) and fall back to minimal config with an explicit warning is well-founded: + 1. Multi-Agent-Mux relies on zero extra external runtime dependencies for core shell shims. + 2. On hosts running macOS default Python 3.9, the default user `herdr` configuration is minimal, and restoring without extra settings safely satisfies Issue #45. + 3. The behavior is fully documented in `create/SKILL.md` and `VERSIONS.md`. + Therefore, this design choice is accepted as complete and non-blocking. + +--- + +## 6. Final Verdict + +All Round 1 blockers have been resolved with precision. No regressions or unintended mutations were introduced. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-agy-01/report-f3699a96.md b/.agents/reports/reviewer-agy-01/report-f3699a96.md new file mode 100644 index 0000000..5b1e3f0 --- /dev/null +++ b/.agents/reports/reviewer-agy-01/report-f3699a96.md @@ -0,0 +1,107 @@ +# Final Review Report: Job f3699a96 (PR #48 / Issue #45) + +## 1. Review Identity & Metadata +- **Loop ID**: `loop-20261005T064728Z-a4e9c8` +- **Job ID**: `f3699a96` +- **Reviewer**: agy (session: `herdr:reviewer-agy-01`) +- **Base Commit**: `c5c7eff6f758c76ac37a138dbc3c301f3f7e10cf` (main, v4.8.4) +- **Head Commit**: `35935f9e8a052ca9e9d9b806a62d17eb72afea78` (10 commits) +- **Tree Hash**: `dff9db13f3e018528d04a024acda942f501513ff` +- **Verification Path**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1` +- **Git Status**: Clean (working tree clean, no uncommitted changes) +- **Whitespace Check (`git diff base...HEAD --check`)**: Clean (0 errors) + +--- + +## 2. Review Findings & Reasons for NOT PASS + +During verification with the specified test command (`AGENT_PYTHON_BIN=... python -m pytest ...`), multiple critical test failures occurred on macOS / Python < 3.11: + +### 1) Unconditional `import tomllib` at Module Level in Test Files +- **Failure 1**: [`tests/test_issue45_herdr_config.py:16`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue45_herdr_config.py#L16) + ```python + import tomllib + ``` + - **Error**: `ModuleNotFoundError: No module named 'tomllib'` + - **Impact**: Pytest fails collection immediately with exit code 2. The entire test module cannot be run on Python < 3.11 (such as Python 3.9 in the project virtualenv). + - **Context**: While [`lib_py/herdr_config.py:152-155`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/.agents/skills/lib_py/herdr_config.py#L152-L155) guards the import with `try ... except ImportError: tomllib = None`, the test module itself imports `tomllib` unconditionally at line 16 and in test functions. + +- **Failure 2**: [`tests/test_issue9_server_env.py:1592`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue9_server_env.py#L1592) + ```python + def test_bootstrap_hands_the_server_a_no_restore_config(mam_sandbox): + import tomllib + ``` + - **Error**: `ModuleNotFoundError: No module named 'tomllib'` + - **Impact**: Test fails immediately on Python < 3.11. + +--- + +### 2) Non-Portable Linux Assumptions in Real Herdr Server Test (`test_issue45_restore.py`) +Four out of 5 tests in [`tests/test_issue45_restore.py`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue45_restore.py) fail on macOS: + +- **Failure 3 (Linux `/proc` filesystem hardcoding)**: [`tests/test_issue45_restore.py:227`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue45_restore.py#L227) + ```python + environ = Path(f"/proc/{rig.server_pids()[0]}/environ").read_bytes().split(b"\0") + ``` + - **Error**: `FileNotFoundError: [Errno 2] No such file or directory: '/proc/26854/environ'` + - **Impact**: Tests `test_a_mam_server_does_not_relaunch_saved_agents[plain-user-config]` and `test_a_mam_server_does_not_relaunch_saved_agents[user-config-with-session-table]` both crash. macOS (Darwin) does not have a `/proc` filesystem. Reading `/proc//environ` is non-portable. + +- **Failure 4 (macOS `/tmp` vs `/private/tmp` symlink mismatch)**: [`tests/test_issue45_restore.py:263`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue45_restore.py#L263) + ```python + assert _pane_with(rig, ids["Y"])["cwd"] == str(rig.root) + ``` + - **Error**: `AssertionError: assert '/private/tmp/m45phdbspj1' == '/tmp/m45phdbspj1'` + - **Impact**: `test_b_only_the_resumed_sessions_restored_shell_is_closed` fails. On macOS, `/tmp` is a symlink to `/private/tmp`. Herdr canonicalizes working directories to `/private/tmp/...`. Exact string comparison against uncanonicalized `str(rig.root)` fails. Comparing with `Path(cwd).resolve()` or `os.path.samefile` is required. + +- **Failure 5 (Pane lookup failure due to uncanonicalized `/tmp` path)**: [`tests/test_issue45_restore.py:325`](file:///var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1/tests/test_issue45_restore.py#L325) + ```python + pane = next(p for p in rig.panes() if p.get("cwd") == str(ws)) + ``` + - **Error**: `StopIteration` + - **Impact**: `test_b_resume_leaves_no_restored_shell_behind` fails. `ws` is constructed under `/tmp/...`, while `p.get("cwd")` is reported as `/private/tmp/...`. The generator finds no match and raises `StopIteration`. + +--- + +## 3. Evaluation of Brief Review Questions + +1. **A's config merging**: + - On Python 3.11+, parsing and rewriting logic in `lib_py/herdr_config.py` preserves tables, dotted keys, and verifies with `tomllib.loads(text) == expected` followed by `herdr config check`. + - On Python < 3.11 (e.g. system Python 3.9), `tomllib` is absent, causing `server_config_text()` to emit a warning and fall back to `MINIMAL` (`[session]\nresume_agents_on_restore = false`), thereby losing user-defined herdr settings on Python 3.9 systems. +2. **Coverage across server boot paths**: + - `lib.sh:230-236` runs `python3 -m lib_py.herdr_config server-config` during headless bootstrap. + - Accurately documented that manually attached/started servers outside MAM do not pick up `mam-server-config.toml`. +3. **B's pane closing conditions**: + - Clean fail-safe in `lib_py/herdr_restore.py`: checks `HERDR_SESSION_NAME` is not default, ensures no live agent is running in the pane, checks matching `agent_session`, and verifies foreground process group is the shell. If herdr cannot be queried, it fails-safe by leaving panes untouched and emitting a warning in `resume_session.sh:614`. +4. **Test isolation & portability**: + - Test suites properly use temporary HOME and isolated session names, preventing mutation of live user servers or configs. + - However, portability on macOS is broken due to hardcoded `/proc` access and uncanonicalized `/tmp` symlink path comparisons. +5. **Version and Inventory**: + - Framework and 8 skills were updated to `4.8.5`. + - `tests/data/skill_code_blocks.json` updated line ranges for `create` and `resume` while preserving classification. +6. **PR purge failure note**: + - Baseline purge test passes 23/23 on macOS and is unrelated. +7. **Boundary (#17)**: + - Complex logic is implemented in Python (`herdr_config.py`, `herdr_restore.py`), while shell scripts delegate to them. + +--- + +## 4. Test Suite Execution Summary + +Executed in verification worktree (`/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1`): + +| Test Suite | Result | Details | +|---|---|---| +| `tests/test_issue45_herdr_config.py` | **FAILED** | Collection error: `ModuleNotFoundError: No module named 'tomllib'` at line 16 | +| `tests/test_issue9_server_env.py` | **FAILED** | `test_bootstrap_hands_the_server_a_no_restore_config` failed at line 1592 (`ModuleNotFoundError`) | +| `tests/test_issue45_restore.py` | **FAILED** (4/5 failed) | Lines 227 (`/proc` FileNotFoundError), 263 (`/tmp` vs `/private/tmp` AssertionError), 325 (`StopIteration`) | +| `tests/test_issue23_24_startup_path.py` | 27 passed | Clean | +| `tests/test_issue42_skill_blocks.py` | 37 passed, 1 skipped | Clean | +| `tests/test_version_consistency.py` | 3 passed | Clean | + +--- + +## 5. Conclusion + +Due to the critical test failures on macOS and Python < 3.11 (`ModuleNotFoundError: tomllib` in test suites, Linux `/proc` hardcoding, and macOS `/private/tmp` path matching failures in `test_issue45_restore.py`), the review verdict is NOT PASS. + +[VERDICT: NOT PASS] diff --git a/.agents/reports/reviewer-grok-01/report-2a661116.identity.json b/.agents/reports/reviewer-grok-01/report-2a661116.identity.json new file mode 100644 index 0000000..b33de4b --- /dev/null +++ b/.agents/reports/reviewer-grok-01/report-2a661116.identity.json @@ -0,0 +1,14 @@ +{ + "loop_id": "loop-20261005T085016Z-c0f6eb", + "base_sha": "e8d9151226d8139c7b5688e83d035a3a747aeb2d", + "head_sha": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "tree_sha": "f49ffdc8e95dc316f752fd81b2d55a0efddf1a12", + "review_commit": "ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9", + "reviewer": "reviewer-grok-01", + "job_id": "2a661116", + "verdict": "PASS", + "transport": "herdr", + "evidence": "review-tree", + "isolation": "worktree", + "verified_at": "2026-10-05T09:05:24Z" +} \ No newline at end of file diff --git a/.agents/reports/reviewer-grok-01/report-2a661116.md b/.agents/reports/reviewer-grok-01/report-2a661116.md new file mode 100644 index 0000000..a89df4e --- /dev/null +++ b/.agents/reports/reviewer-grok-01/report-2a661116.md @@ -0,0 +1,40 @@ +# Review: PR #48 round 3 @ ab93776 + +- Role: Reviewer (grok) +- Identity: loop `loop-20261005T085016Z-c0f6eb`, base `e8d9151226d8139c7b5688e83d035a3a747aeb2d`, head `ab93776d6d1b8ca534c14b6be53f61dc3a0fa2a9`, tree `f49ffdc8e95dc316f752fd81b2d55a0efddf1a12`, one commit, clean snapshot +- Scope: `git diff e8d9151 HEAD`. Four files, +125/−4, `git diff --check` clean. Main stayed `c5c7eff`. +- Verified read-only at `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T085016Z-c0f6eb/1`. + +## Codex R1–R3 + +| Codex | ab93776 | +|---|---| +| R1. `herdr_rejects` raising (`TimeoutExpired`, `OSError`) was treated as a pass, so an unchecked merge could be handed to the server. A schema error in that file makes herdr drop the whole file and turn restore back on. | `_write_checked` (`herdr_config.py:247-258`) no longer returns success on that exception. If the candidate is the merged body, it warns `not using an unchecked merge` and continues. The loop's next candidate is `MINIMAL` (`:241`). The merged text is not what remains: `test_an_unrunnable_check_never_hands_over_an_unchecked_merge` writes `confirm_close = "invalid-bool"`, raises on the check, and the file on disk parses as only `resume_agents_on_restore = false`. The user file is unchanged. | +| R2. `_shell_in_foreground` used `==`, so `0/0`, `False/False`, `""/""`, and `-1/-1` closed a pane. | A value counts only when it is an `int`, not a `bool`, and `> 0` (`herdr_restore.py:46-55`). Both pids must pass, and they must be equal. `tests/test_issue45_restore_unit.py` stubs herdr: those eight odd pairs, a missing `process_info`, and a busy pane (`4242` vs `4300`) do not close; `4242/4242` does. | +| R3. Reparse used `==`, and `nan != nan`, so one valid `nan` dropped the rest of the user's settings. | `_same_doc` (`herdr_config.py:140-149`) treats two floats as equal when both are `nan`, and walks dicts and lists. `server_config_text` uses it at `:171`. The three cases (top-level, array including `-nan`, nested inline table) keep `confirm_close` and warn nothing. | + +## MINIMAL when the check cannot run + +Codex asked to return no path if the check of `MINIMAL` also cannot run. This commit writes `MINIMAL` and warns `using the fixed minimal config unchecked` (`:257-258`). + +That is the right call. `lib.sh` starts the server with restore on when the config path is empty. Returning `None` would reopen #45 in the one case where no check can be obtained (timeout, missing binary, `OSError`). `MINIMAL` is the constant `[session]\nresume_agents_on_restore = false\n` (`:51`), not a user document, and `test_real_herdr_accepts_every_result` already shows herdr 0.9.1 accepts it whenever `config check` can run. The unchecked write happens only when that command cannot be executed, so there is no schema verdict to wait for. Unknown nonzero exits are still ignored unless the output contains `parse error` or `using defaults` (`herdr_rejects`, `:190-191`); herdr uses a nonzero status for a dropped setting that does not drop the file. + +## `_same_doc` beside `==` + +For values a TOML round trip produces, it matches `==`, plus `nan`. Dict keys are compared as a set (`a.keys() == b.keys()`), list order is kept (`zip`), datetimes use `==`, and `inf` / `-inf` follow `==`. + +Two pairs differ, both stricter: `True == 1` is true and `_same_doc(True, 1)` is false; `1 == 1.0` is true and `_same_doc(1, 1.0)` is false. The writer emits a bool before the int branch (`_value`, `:108-111`) and emits a float with `repr` (`:117`), so a document `tomllib` parsed does not come back as the other type. A round trip that did change type fails closed into `MINIMAL` with a warning, which `==` would have accepted. + +## Scope + +The delta is `herdr_config.py`, `herdr_restore.py`, the two new tests in `test_issue45_herdr_config.py`, and `tests/test_issue45_restore_unit.py`. No skill text, inventory, or version line. 4.8.5 is not on main; the fallback to the key-only file is already what `VERSIONS.md` describes. + +## Tests + +Python 3.9.6, the four files in the brief, `--basetemp=/tmp/2a66follow.bPQAFP`: **53 passed, 34 skipped** in 77.84s. The skips are the `needs tomllib` cases, including the new R1 and R3 tests. `test_issue45_restore_unit.py` ran. + +Python 3.11, the same venv named in the brief, `tests/test_issue45_herdr_config.py` and `tests/test_issue45_restore_unit.py`, `HOME` pointed at an empty temp directory: **67 passed** in 0.41s. + +No `mamt45` process remained. `~/.config/herdr/config.toml` mtime stayed `1784456394`, the directory mtime stayed `1791183576`, and `mam-server-config.toml` was not created there. Snapshot `git status` was empty. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-grok-01/report-667d5542.md b/.agents/reports/reviewer-grok-01/report-667d5542.md new file mode 100644 index 0000000..788c552 --- /dev/null +++ b/.agents/reports/reviewer-grok-01/report-667d5542.md @@ -0,0 +1,69 @@ +# Review: PR #48 (fix/issue-45) @ 35935f9 + +- Role: Reviewer (grok) +- Identity: loop `loop-20261005T064728Z-a4e9c8`, base `c5c7eff6f758c76ac37a138dbc3c301f3f7e10cf` (main, v4.8.4), head `35935f9e8a052ca9e9d9b806a62d17eb72afea78`, tree `dff9db13f3e018528d04a024acda942f501513ff`, 10 commits, clean snapshot +- Verified read-only at `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T064728Z-a4e9c8/1`. Main stayed `c5c7eff`. No product edit, no commit, no push. +- Diff vs base: 25 files, `tests/test_issue10_13_purge.py` absent. Interpreter for the required pytest: `pr-48/.venv` Python 3.9.6. The shim's `python3` is `/usr/bin/python3`, also 3.9.6. CI (`deploy/gitea-ci.yml:94`) is Python 3.10. `tomllib` is stdlib only from 3.11. `requirements-dev.txt` is `pytest>=8.0`. `/usr/bin/python3` has no `tomli`. The venv has `tomli` and no `tomllib`. + +The required command exits at collection. With `--continue-on-collection-errors` added only so the rest of the list could run: **5 failed, 192 passed, 1 skipped, 1 error** in 254.58s (`--basetemp=/tmp/667follow.YrMavO`). The first command, as written in the brief, was **1 collection error in 0.22s, exit 2**, and ran no test. + +## What fails + +### 1. The merge tests do not load on the Python this repo runs + +`tests/test_issue45_herdr_config.py:16` is a top-level `import tomllib`. Collection raises `ModuleNotFoundError`. `tests/test_issue9_server_env.py:157` imports it inside `test_bootstrap_hands_the_server_a_no_restore_config` and fails the same way. + +`lib_py/herdr_config.py:54-57` catches that import and sets `tomllib = None`. `server_config_text` then returns `MINIMAL` plus a warning (`:146-147`). A throwaway `HOME=/tmp/m45probe-min` run of `/usr/bin/python3 -m lib_py.herdr_config server-config` printed that warning and wrote only: + +```toml +# generated by multi-agent-mux (Issue #45) - do not edit +# source: /tmp/m45probe-min/home/.config/herdr/config.toml +[session] +resume_agents_on_restore = false +``` + +The throwaway user file's checksum was unchanged. That is the accepted "no parser, restore still off" path. It is also the path the shim takes on this host and on CI 3.10. + +`VERSIONS.md:49` and `multi-agent-mux-create/SKILL.md:120` describe the generated file as the user's config with that one key forced false. `test_issue9_server_env.py:171` asserts the merged document (`ui.confirm_close` kept). On this `python3` the file is `MINIMAL`, so fixing only the test import still fails `:171`. + +Alternative: in `herdr_config.py:54-57` and in both test imports, use `tomli` when `tomllib` is missing (`tomli.loads` / `tomli.TOMLDecodeError` match). Add `tomli` to `requirements-dev.txt`, and install it for the `python3` that `lib.sh:241` actually runs (`/usr/bin/python3` here, not the venv). Keep the `MINIMAL` warning only when neither library imports. With `tomli` assigned onto the module, the merge kept inline tables, dotted keys, quoted keys, arrays, and comments semantically, and a broken document or a non-table `session` fell back to `MINIMAL`. `herdr config check` on `confirm_close = "no"` in a throwaway file exited 1 with `parse error` and `using defaults`, and `herdr_rejects` (`:169-180`) reported it. + +### 2. The #45 positive control never runs on macOS + +`tests/test_issue45_restore.py:227-228` reads `/proc//environ`. Both parametrizations of `test_a_mam_server_does_not_relaunch_saved_agents` raised `FileNotFoundError` there. The asserts above that line had already passed: the fake claude did not log `--resume`, the restored pane had no agent, and the foreground pid was the shell. The positive control at `:230-236` (plain server must relaunch) did not execute. + +Alternative: build the `HERDR_CONFIG_PATH=...` needle, read `/proc//environ` when that file exists, and otherwise use `ps eww -p -o command=` and search the raw output. Run `:230-236` even when the environ read fails, so a missing `/proc` cannot skip the control that shows restore-on still relaunches. + +### 3. Restored cwd is the resolved path + +`tests/test_issue45_restore.py:263`: + +```text +AssertionError: assert '/private/tmp/m45swco8dxx' == '/tmp/m45swco8dxx' +``` + +`mkdtemp(dir="/tmp")` returns `/tmp/...`. On this Mac `/tmp` is `/private/tmp`, and herdr's pane `cwd` is the resolved path. The close calls start at `:267`, so the selective-close asserts (live agent Z, busy `sleep` X2, user pane U, close only X and Y) did not run. `test_b_resume_leaves_no_restored_shell_behind` got create rc 0 and `status == running`, then `StopIteration` at `:325` on `p.get("cwd") == str(ws)` with that same unresolved `/tmp` path. + +`test_b_default_server_is_never_touched` passed: `HERDR_SESSION_NAME=default` returns before invoking `--herdr /nonexistent/herdr` (`herdr_restore.py:54-55`). + +Alternative: compare `Path(cwd).resolve()` with `Path(rig.root).resolve()` at `:263`, and `Path(p["cwd"]).resolve() == Path(ws).resolve()` at `:325`. + +## What holds + +1. User file bytes are not the output. `output_path` (`herdr_config.py:192-205`) skips the source (including `samefile`) and a file whose marker source is not this source. Both names taken raises `OSError`; the fallback directory goes through the same helper (`:270-276`). Writes are `mkstemp` + `os.replace` (`:208-216`). A generated `HERDR_CONFIG_PATH` is followed one hop via the `# source:` line (`:79-87`). The trace beside the user config is `mam-server-config.toml` (or `mam-server-config-.toml`), marker plus `# source: `. A `nan` value makes `tomllib.loads(text) == expected` false (`:159`, `nan != nan`) and falls back to `MINIMAL` with a warning. The user file stays. + +2. The only server spawn is the shim heredoc `lib.sh:212-261` (`$REAL_HERDR --session server`). create (`create_session.sh:27` sources `lib.sh`, `:264` `_herdr new-session`) and resume (`resume_session.sh:8`, `:245`) go through that shim. The bootstrap runs only when the name is not `default`, `MAM_NO_AUTOSTART` is not `1`, and `session list` says the server is down (`lib.sh:193-212`). `status.sh:13` sets `MAM_NO_AUTOSTART=1` before sourcing. `reconcile.sh:28` sets it before herdr calls; the bootstrap is in the shim, not at source time. An already-running server is left as it was. `create/SKILL.md:120-121` and `VERSIONS.md:50` say the file is rebuilt the next time MAM starts the server, and a server the user starts with `herdr session attach` does not get it. + +3. A pane is closed only when the session name is set and not `default`, the pane has no live agent, `agent_session.value` equals the id being resumed, and `shell_pid` equals `foreground_process_group_id` (`herdr_restore.py:54-65`). Empty id and `default` return `[]` before herdr. `main` maps `OSError`, `SubprocessError`, and `ValueError` to exit 1 (`:81-83`). `resume_session.sh:240-242` calls that and on failure only prints a warning; dry-run has already exited at `:225-228`. Bash does not decide which pane. There is a gap between `process-info` and `pane close` in which a command can start; a failed query still closes nothing further. + +5. `MAM_VERSION` and `FRAMEWORK_VERSION` are `4.8.5`. The eight `SKILL.md` frontmatters match. `tests/data/skill_code_blocks.json` changes line numbers and the resume script anchors (`:237/:241/:247-250` → `:245/:249/:255-258`). No class, kind, or sha field changes. `test_version_consistency` and `test_issue42_skill_blocks` are in the 192 passes. S11's base moved to `811b080` (`tests/test_issue23_24_startup_path.py`) because the heredoc change is intentional. + +6. `tests/test_issue10_13_purge.py` is not in the diff. Those claimed full-suite failures are not produced by this PR. This review did not run that file. + +7. New judgment is in `herdr_config.py` and `herdr_restore.py`. The new bash is "config path empty → warning, start anyway" (`lib.sh:243-245`) and the resume warning above. The spawn snippet copies `MAM_SERVER_CFG` into the child as `HERDR_CONFIG_PATH` and drops `MAM_SERVER_CFG` from the child (`lib.sh:253-256`). + +## Isolation + +Restore tests use `mkdtemp(prefix="m45", dir="/tmp")`, session `mamt45--`, and strip `HERDR_WORKSPACE_ID`, `HERDR_PANE_ID`, `HERDR_TAB_ID`, `HERDR_SOCKET_PATH`, `HERDR_SESSION`, `HERDR_ENV`, `HERDR_CONFIG_PATH`, and `XDG_CONFIG_HOME` (`test_issue45_restore.py:41-42`, `:160-164`). After the run, no `mamt45` process remained. `~/.config/herdr/config.toml` mtime stayed `1784456394`, the directory mtime stayed `1791183576`, and no `mam-server-config.toml` appeared there. The live `multi-agent-mux` server log grew on its own during the window. Snapshot `git status` was empty afterward. Throwaway pytest dirs were removed. + +[VERDICT: NOT PASS] diff --git a/.agents/reports/reviewer-grok-01/report-b4719c9e.md b/.agents/reports/reviewer-grok-01/report-b4719c9e.md new file mode 100644 index 0000000..1027331 --- /dev/null +++ b/.agents/reports/reviewer-grok-01/report-b4719c9e.md @@ -0,0 +1,32 @@ +# Review: PR #48 round 2 @ e8d9151 + +- Role: Reviewer (grok) +- Identity: loop `loop-20261005T081844Z-e92124`, base `c5c7eff6f758c76ac37a138dbc3c301f3f7e10cf`, head `e8d9151226d8139c7b5688e83d035a3a747aeb2d`, tree `8b39188453ff8a7263aa4400792fabc99188dad6`, clean snapshot, ancestor of both `c5c7eff` and round-1 `35935f9` +- Scope for this reviewer: `git diff 35935f9 e8d9151` only (commits `36f0b8d`, `e8d9151`). Five files, +56/−9, `git diff --check` clean. No product code. Main stayed `c5c7eff`. +- Verified read-only at `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261005T081844Z-e92124/1`. + +## Round-1 findings + +| Round 1 | e8d9151 | +|---|---| +| `tests/test_issue45_herdr_config.py:16` imported `tomllib` at module level, so Python 3.9/3.10 could not collect the file. `tests/test_issue9_server_env.py:157` did the same inside the bootstrap test, and expected the merged document from a 3.9 `python3` that writes only the key. | Import is `try/except` (`test_issue45_herdr_config.py:18-21`). `needs_tomllib` skips the 29 tests that parse a result or expect the user's other settings (`:36`, applied at `:71` and the other parse sites). `test_without_tomllib_restore_is_still_off` still runs. `test_issue9_server_env.py:173-183` parses with the `python3` on `PATH` and, when that interpreter has no `tomllib`, expects the key-only file and `no tomllib` in the shim stderr. `create/SKILL.md:120` and `VERSIONS.md:49` say a `python3` older than 3.11 leaves restore off, drops the other herdr settings, and warns. | +| `tests/test_issue45_restore.py:227` read `/proc//environ` before the restore-on control, so macOS never reached that control. | `_process_env` (`:160-168`) reads `/proc//environ` when it exists, otherwise `ps -E -ww -o command=`. The control stays after that check (`:242-248`). On this Mac the herdr server's `ps -E` line contains `HERDR_CONFIG_PATH=/mam-server-config.toml`, and both `test_a` cases passed, so the control ran. | +| `:263` and `:325` compared pane `cwd` to the unresolved `/tmp/...` from `mkdtemp`. herdr reports `/private/tmp/...`. | The fixture resolves the temp root (`:175`) before `HOME` is set, so later `str(rig.root)` and `str(ws)` match herdr. `test_b_only_the_resumed_sessions_restored_shell_is_closed` and `test_b_resume_leaves_no_restored_shell_behind` passed. | + +The tomli backport suggested in round 1 is not in this delta. The docs now describe the path this host's `/usr/bin/python3` (3.9.6, no `tomli`) actually takes. That matches the decision to document it. + +`tests/data/skill_code_blocks.json` is unchanged. The create skill edit is the same line 120, so no fence moved. `test_issue42_skill_blocks` passed, with the usual worktree skip at `:266` because `.claude/skills` is absent in the snapshot. + +## Tests + +Required command, PR venv Python 3.9.6, `--basetemp=/tmp/b471follow.eYGqXM` (no `mam-review` in the path), MQTT variables unset: + +**152 passed, 30 skipped** in 214.33s. The 30 skips are the 29 `needs tomllib` cases plus `test_issue42_skill_blocks.py:266`. + +Same file on a throwaway Python 3.11.15 venv (`/opt/homebrew/bin/python3.11`), `HOME` set to an empty temp directory: + +**51 passed** in 0.38s, none skipped. + +Afterward no `mamt45` process remained. `~/.config/herdr/config.toml` mtime stayed `1784456394`, the directory mtime stayed `1791183576`, and `mam-server-config.toml` was not created there. Snapshot `git status` was empty. + +[VERDICT: PASS] diff --git a/VERSIONS.md b/VERSIONS.md index ca512f1..2649f80 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -55,6 +55,8 @@ - **실서버 테스트 (`tests/test_issue45_restore.py`, herdr 0.9.1)**: 짧은 일회용 HOME·세션과 가짜 claude 로, MAM bootstrap 경로에서 복원이 꺼짐(사용자 설정에 `[session]` 표가 있을 때 포함)과 같은 레이아웃을 일반 서버로 띄우면 `claude --resume` 이 뜨는 양성 대조, 닫기 조건, create → crash → resume 뒤 pane 하나만 남음을 확인한다. 수정을 끄면 A·B 테스트가 각각 실패함을 확인했다. - **단위 (`tests/test_issue45_herdr_config.py`)**: 합치기 12 가지 경우와 실제 `herdr config check`, 원본 경로 규칙, 쓰기·폴백. - **관련 스위트**: 새 두 파일 40 passed. issue9·shim contract·issue42·issue24 resume·uuid_target·issue21 prereqs·issue23_24 199 passed / 1 skipped(S11 기준 이동 후). tier1~4·issue21_phase2_loop_a 148 passed. 전체 스위트는 돌리지 않았다. +- **리뷰 (병합 워크스페이스, 네 리뷰어)**: 1회차(@35935f9) claude·agy·grok NOT PASS(테스트 이식성: Python 3.9 의 `tomllib`, macOS 의 `/proc`·`/tmp`), codex 판정 없음(모델 용량 초과로 1 시간 예산 소진). 36f0b8d·e8d9151 에서 수정. 2회차(@e8d9151) claude·agy·grok PASS, codex NOT PASS(R1 `config check` 를 실행하지 못하면 검사 안 된 병합을 넘김 → 스키마 오류면 herdr 가 파일 전체를 버려 복원이 다시 켜짐, R2 0·False·-1 같은 pid 응답에도 pane 을 닫음, R3 TOML `nan` 이 재파싱 비교를 깨 사용자 설정을 모두 버림). ab93776 에서 수정: 검사 못 한 병합은 쓰지 않고 MINIMAL(그 검사도 못 하면 MINIMAL 을 경고와 함께 — 아무것도 안 넘기면 복원이 켜진 채 남는다), pid 는 양의 정수만, nan 끼리 같다고 보는 재귀 비교. 3회차(@ab93776) **4/4 PASS**. 리포트와 identity 사이드카는 `.agents/reports//`. +- **전체 스위트 (병합 워크스페이스, macOS·Python 3.9, MQTT 환경변수 없이)**: ab93776 **1456 passed / 0 failed / 38 skipped**(65 분; skip 중 34 건은 Python 3.9 의 tomllib 전용 검사). 1회차 head 35935f9 는 herdr_config 테스트가 수집 단계에서 스위트를 멈췄고, 그 파일을 빼면 1418 passed / 5 failed(1회차 지적 테스트). purge 테스트도 통과(PR 본문의 12 건 기준 실패는 Linux, #28). - **Python 3.9 (병합 워크스페이스)**: `tests/test_issue45_herdr_config.py` 가 모듈 머리에서 `import tomllib` 을 해 3.11 미만에서는 스위트 전체가 수집 단계에서 멈췄다. import 를 선택적으로 바꾸고 결과를 파싱하거나 사용자 설정 유지를 기대하는 29 건만 `tomllib` 없으면 skip(3.9: 22 passed / 29 skipped, 3.11: 51 passed). - **리뷰**: 1회차 codex NOT PASS(R1 herdr 스키마 검증 없음, R2 점 표기·인라인 표에서 설정 유실, R3 원본과 출력 이름 충돌 시 덮어씀). 811b080 에서 수정. 2회차 codex NOT PASS(R3 의 폴백 경로는 같은 검사를 거치지 않음), 폴백도 같은 이름 선택을 거치도록 수정. -- 2.54.0