From f19f94099fc77afdb6e2b92bb4d3c732af178154 Mon Sep 17 00:00:00 2001 From: Godopu Date: Thu, 8 Oct 2026 07:53:58 +0900 Subject: [PATCH] fix(issue-17): resume closes herdr 0.9.3's placeholder for a removed directory With the cwd premise fixed, test_b still failed on herdr 0.9.3: a pane whose saved directory is gone is restored as a placeholder - `restore_error` "Saved directory is unavailable...", no process - and `pane process-info` answers pane_not_found. close_restored_shells raised on it and exited 1 before looking at the other panes, so the resumed session's removed-worktree pane (the case #45 closes on herdr 0.9.1, where it is a shell in $HOME) was left behind. Such a pane has nothing running, so it now meets condition 4. Only that exact answer counts - restore_error set AND pane_not_found; any other process-info failure still stops the run with exit 1 as before. Tests: test_issue45_restore.py passes on herdr 0.9.3 (5/5). Unit tests with a stub herdr: the placeholder is closed (fails at 56d0bfd); pane_not_found without restore_error, or restore_error with another error, still raises. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe --- .agents/skills/lib_py/herdr_restore.py | 20 ++++++++++-- tests/test_issue45_restore_unit.py | 42 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/.agents/skills/lib_py/herdr_restore.py b/.agents/skills/lib_py/herdr_restore.py index 679732b..ac1059d 100644 --- a/.agents/skills/lib_py/herdr_restore.py +++ b/.agents/skills/lib_py/herdr_restore.py @@ -3,7 +3,8 @@ 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. When that directory is gone, herdr 0.9.1 opens the shell in $HOME and herdr 0.9.3 -keeps the old path; nothing below looks at the cwd. A pane keeps the +keeps the old path with no shell in it (see 4.); nothing below looks at the +cwd. 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 @@ -13,7 +14,10 @@ all of these hold: 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). + 4. its foreground process is the shell itself (nothing running in it) - or + it has no process at all: herdr 0.9.3 restores a pane whose directory is + gone as a placeholder (`restore_error` set) that `pane process-info` + reports as pane_not_found. The registry is not consulted: a session whose server died stays `running` there, so "running" cannot tell a live pane from a restored one. @@ -51,6 +55,16 @@ def _shell_in_foreground(herdr: str, session: str, pane_id: str) -> bool: return _is_pid(shell) and _is_pid(fg) and fg == shell +def _nothing_running(herdr: str, session: str, pane: dict) -> bool: + try: + return _shell_in_foreground(herdr, session, pane["pane_id"]) + except subprocess.CalledProcessError as e: + # Only herdr's own placeholder counts: any other failure still stops the run. + if pane.get("restore_error") and "pane_not_found" in (e.stderr or "") + (e.stdout or ""): + return True + raise + + def _is_pid(v) -> bool: return isinstance(v, int) and not isinstance(v, bool) and v > 0 @@ -68,7 +82,7 @@ def close_restored_shells(session_id: str, herdr: str = "herdr", continue if (p.get("agent_session") or {}).get("value") != session_id: continue - if not _shell_in_foreground(herdr, session, pane_id): + if not _nothing_running(herdr, session, p): continue _herdr(herdr, session, "pane", "close", pane_id) print(f"closed restored pane {pane_id} (cwd={p.get('cwd', '')}, agent_session={session_id})", diff --git a/tests/test_issue45_restore_unit.py b/tests/test_issue45_restore_unit.py index 7a7d407..7db20d9 100644 --- a/tests/test_issue45_restore_unit.py +++ b/tests/test_issue45_restore_unit.py @@ -4,6 +4,7 @@ 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 subprocess import sys from pathlib import Path @@ -54,3 +55,44 @@ 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 + + +# herdr 0.9.3: a pane whose saved directory is gone is restored as a placeholder +# with no process; process-info answers pane_not_found (real-server check: +# test_issue45_restore.py test_b_only_the_resumed_sessions_restored_shell_is_closed). +PLACEHOLDER_ERR = '{"error":{"code":"pane_not_found","message":"pane not found"},"id":"cli:pane:process_info"}' + + +def _stub_failing(monkeypatch, restore_error, stderr): + calls = [] + + def fake(herdr, session, *args): + calls.append(args) + if args[:2] == ("pane", "list"): + pane = {"pane_id": "probe-pane", "agent": None, "agent_session": {"value": SID}} + if restore_error: + pane["restore_error"] = restore_error + return {"panes": [pane]} + if args[:2] == ("pane", "process-info"): + raise subprocess.CalledProcessError(1, ["herdr", *args], output="", stderr=stderr) + return {} + monkeypatch.setattr(hr, "_herdr", fake) + return calls + + +def test_a_placeholder_for_a_missing_directory_is_closed(monkeypatch): + calls = _stub_failing(monkeypatch, "Saved directory is unavailable. Restore the directory and restart " + "this session.", PLACEHOLDER_ERR) + assert hr.close_restored_shells(SID, session="m45-review-probe") == ["probe-pane"] + assert ("pane", "close", "probe-pane") in calls + + +@pytest.mark.parametrize("restore_error,stderr", [ + (None, PLACEHOLDER_ERR), # pane_not_found without herdr's marker + ("Saved directory is unavailable.", '{"error":{"code":"server_error"}}'), # another failure +], ids=["no-restore-error", "other-error"]) +def test_any_other_process_info_failure_still_stops_the_run(monkeypatch, restore_error, stderr): + calls = _stub_failing(monkeypatch, restore_error, stderr) + with pytest.raises(subprocess.CalledProcessError): + hr.close_restored_shells(SID, session="m45-review-probe") + assert ("pane", "close", "probe-pane") not in calls