test(herdr): add regression tests for pane misattribution and idle-pane reuse

- Add test_r2 and test_r3 for bare-kind fallback restriction and R-1 exact-match enforcement - Add test_h24 and test_h25 for idle-pane reuse and env_flags preservation
2026-08-30 11:02:51 +09:00
parent b327bc7132
commit 526f7a59b7
2 changed files with 141 additions and 0 deletions
@@ -622,3 +622,75 @@ echo "RC=$rc|PID=$pid"
    assert "RC=1|PID=" in res.stdout, f"Expected fail-closed RC=1 and empty PID: {res.stdout}"


+def test_r2_resolve_pane_id_does_not_misattribute_unrelated_same_kind_pane():
+    """BUG-1: a single unrelated same-kind pane must not be silently returned
+    as the target — the generic agent_kind fallback only applies when the
+    caller searched by the bare kind itself, not when a specific session
+    name's own pane is missing but another agy/claude/... pane exists."""
+    helper = _lib_helper_src()
+    script = helper + """
+# Ambient HERDR_WORKSPACE_ID (this session itself runs inside a herdr pane)
+# must not leak into the test — it would scope-filter the mock pane list
+# and mask the fallback logic under test.
+unset HERDR_WORKSPACE_ID 2>/dev/null || true
+
+# Mock real herdr pane list returning exactly ONE unrelated agy pane —
+# this is the exact single-match scenario the old code misattributed.
+_real_herdr() {
+  cat << 'EOF'
+{"result": {"panes": [
+  {"pane_id": "w1:p1", "label": "orchestrator-agy", "agent": "agy"}
+]}}
+EOF
+}
+
+# creator-agy-01's own pane is gone (it died); yaml still lists it as agy-kind.
+mkdir -p .mam
+cat << 'EOF' > .mam/agent-sessions.yaml
+herdr_sessions:
+  - name: creator-agy-01
+    pane:
+      cmd: agy
+EOF
+
+rc=0
+pid=$(_resolve_herdr_pane_id "creator-agy-01") || rc=$?
+echo "RC=$rc|PID=$pid"
+"""
+    res = _run_lib_helpers(script)
+    assert res.returncode == 0, res.stderr + res.stdout
+    assert "RC=1|PID=" in res.stdout, (
+        f"Expected fail-closed RC=1 (must NOT misattribute the unrelated "
+        f"orchestrator-agy pane as creator-agy-01): {res.stdout}"
+    )
+
+
+def test_r3_resolve_pane_id_bare_kind_multiple_panes_fails_closed():
+    """Finding B: a bare-kind search (e.g. target='agy') when multiple panes
+    of that agent kind exist must fail-closed (RC=1), not match the first
+    pane via an un-guarded exact match loop."""
+    helper = _lib_helper_src()
+    script = helper + """
+unset HERDR_WORKSPACE_ID 2>/dev/null || true
+
+_real_herdr() {
+  cat << 'EOF'
+{"result": {"panes": [
+  {"pane_id": "w1:p1", "label": "orchestrator-agy", "agent": "agy"},
+  {"pane_id": "w1:p2", "label": "creator-agy-01", "agent": "agy"}
+]}}
+EOF
+}
+
+rc=0
+pid=$(_resolve_herdr_pane_id "agy") || rc=$?
+echo "RC=$rc|PID=$pid"
+"""
+    res = _run_lib_helpers(script)
+    assert res.returncode == 0, res.stderr + res.stdout
+    assert "RC=1|PID=" in res.stdout, (
+        f"Expected fail-closed RC=1 for ambiguous multiple same-kind panes on bare-kind target: {res.stdout}"
+    )
+
+
+
@@ -481,3 +481,72 @@ def test_h23_persisted_workspace_id_scopes_without_env(mam_sandbox, mock_herdr,
    persisted = ws_file.read_text().strip()
    assert persisted, "new-session must persist workspace id"
    assert persisted.startswith("w")
+
+
+def test_h24_new_session_reuses_idle_pane_without_split(mam_sandbox, mock_herdr, mock_agents):
+    """BUG-4: new-session into an existing workspace must reuse an already
+    idle (agent-less) pane instead of unconditionally splitting a new one."""
+    tmp_path = mam_sandbox
+    state = _load_mock_state(mock_herdr)
+    state["panes"] = [{
+        "pane_id": "w1:p_idle",
+        "workspace_id": "w1",
+        "cwd": str(tmp_path),
+        "buffer": "",
+    }]
+    _write_mock_state(mock_herdr, state)
+    n_calls = len(state.get("calls", []))
+
+    res = _run_lib(
+        tmp_path,
+        f'herdr new-session -d -s "test-creator-claude" -c "{tmp_path}" "claude --dangerously-skip-permissions"',
+    )
+    assert res.returncode == 0, f"Stderr: {res.stderr}\nStdout: {res.stdout}"
+
+    final_state = _load_mock_state(mock_herdr)
+    agents = final_state.get("agents", {})
+    matched = [d for n, d in agents.items() if "test-creator-claude" in n]
+    assert matched, agents
+    assert matched[0].get("pane_id") == "w1:p_idle", (
+        f"Expected the idle pane to be reused as-is, not a freshly split one: {matched}"
+    )
+
+    new_calls = final_state.get("calls", [])[n_calls:]
+    split_calls = [c for c in new_calls if len(c) >= 2 and c[0] == "pane" and c[1] == "split"]
+    assert not split_calls, f"Expected no pane split call when an idle pane was available: {split_calls}"
+
+
+def test_h25_new_session_idle_pane_preserves_env_and_ws_id(mam_sandbox, mock_herdr, mock_agents):
+    """Finding A: when an idle pane is reused, caller-provided KEY=VAL environment
+    flags (and HERDR_WORKSPACE_ID) must be prepended via env to final_cmd so they
+    are not silently lost when pane split / workspace create is bypassed."""
+    tmp_path = mam_sandbox
+    state = _load_mock_state(mock_herdr)
+    state["panes"] = [{
+        "pane_id": "w1:p_idle",
+        "workspace_id": "w1",
+        "cwd": str(tmp_path),
+        "buffer": "",
+    }]
+    _write_mock_state(mock_herdr, state)
+    n_calls = len(state.get("calls", []))
+
+    res = _run_lib(
+        tmp_path,
+        f'herdr new-session -d -s "test-creator-claude" -c "{tmp_path}" "MY_TEST_FLAG=active claude --dangerously-skip-permissions"',
+    )
+    assert res.returncode == 0, f"Stderr: {res.stderr}\nStdout: {res.stdout}"
+
+    final_state = _load_mock_state(mock_herdr)
+    agents = final_state.get("agents", {})
+    matched = [d for n, d in agents.items() if "test-creator-claude" in n]
+    assert matched, agents
+    assert matched[0].get("pane_id") == "w1:p_idle"
+    cmd = matched[0].get("command", "")
+    assert "MY_TEST_FLAG=active" in cmd, f"Expected MY_TEST_FLAG=active in agent command: {cmd}"
+    assert "HERDR_WORKSPACE_ID=w1" in cmd, f"Expected HERDR_WORKSPACE_ID=w1 in agent command: {cmd}"
+
+    new_calls = final_state.get("calls", [])[n_calls:]
+    split_calls = [c for c in new_calls if len(c) >= 2 and c[0] == "pane" and c[1] == "split"]
+    assert not split_calls, f"Expected no pane split call when an idle pane was available: {split_calls}"
+