tests/test_herdr_shim_contract.py

@@ -516,10 +516,13 @@ def test_h24_new_session_reuses_idle_pane_without_split(mam_sandbox, mock_herdr,
     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."""
+def test_h25_new_session_with_env_flags_skips_the_idle_pane_and_uses_env_on_split(mam_sandbox, mock_herdr, mock_agents):
+    """Finding A, revised for Issue #21 R1: caller-provided KEY=VAL environment
+    flags (and HERDR_WORKSPACE_ID) must not be silently lost - and must not be
+    put after `agent start --` either, because herdr passes those tokens to the
+    agent executable (--kind) as arguments. With env flags an idle pane is not
+    reused; the env rides on the pane-creation call (pane split / workspace
+    create --env). Reuse without env flags stays covered by h24."""
     tmp_path = mam_sandbox
     state = _load_mock_state(mock_herdr)
     state["panes"] = [{
@@ -538,17 +541,22 @@ def test_h25_new_session_idle_pane_preserves_env_and_ws_id(mam_sandbox, mock_her
     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}"
+    # the shim may prefix `--session <name>`; compare the subcommands themselves
+    new_calls = [c[2:] if c[:1] == ["--session"] else c for c in final_state.get("calls", [])[n_calls:]]
+    creates = [c for c in new_calls if c[:2] in (["pane", "split"], ["workspace", "create"])]
+    assert creates, f"expected a pane split / workspace create carrying the env: {new_calls}"
+    env_vals = [c[i + 1] for c in creates for i, t in enumerate(c[:-1]) if t == "--env"]
+    assert "MY_TEST_FLAG=active" in env_vals and "HERDR_WORKSPACE_ID=w1" in env_vals, env_vals
 
-    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}"
+    matched = [d for n, d in final_state.get("agents", {}).items() if "test-creator-claude" in n]
+    assert matched and matched[0].get("pane_id") != "w1:p_idle", matched
+
+    starts = [c for c in new_calls if c[:2] == ["agent", "start"]]
+    assert starts, new_calls
+    for c in starts:
+        args = c[c.index("--") + 1:] if "--" in c else []
+        assert "env" not in args, args
+        assert not any(a.startswith(("MY_TEST_FLAG=", "HERDR_WORKSPACE_ID=")) for a in args), args
 
 
 def test_h26_daemon_spawn_uses_process_group_detachment(mam_sandbox, mock_herdr, mock_agents):