diff --git a/tests/test_c1_tui_readiness.py b/tests/test_c1_tui_readiness.py index 4a3b46a..e292e0b 100644 --- a/tests/test_c1_tui_readiness.py +++ b/tests/test_c1_tui_readiness.py @@ -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}" + ) + + + diff --git a/tests/test_herdr_shim_contract.py b/tests/test_herdr_shim_contract.py index 559fb6e..c34023f 100644 --- a/tests/test_herdr_shim_contract.py +++ b/tests/test_herdr_shim_contract.py @@ -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}" +