fix(issue-23): a herdr call to another server drops the caller pane's workspace/pane/tab ids

A MAM script often runs inside one herdr server's pane (the orchestrator's)
while targeting another (the isolated MAM server). The pane exports
HERDR_WORKSPACE_ID, HERDR_PANE_ID and HERDR_TAB_ID, and those ids only mean
something on the server that issued them. They reached the scope filters as a
foreign workspace, whose fail-closed miss (ISSUE-3 / F-2b) reports a live
session as absent: has-session, kill-session, agent prompt and the delegate-job
preflight all answered "no such session". Reproduced on Linux and macOS in the
#23 comments, worked around until now with `env -u` at every call site.

mam_herdr drops the three ids for a call aimed at a named server other than the
caller's own. Scope then falls back to .mam/herdr_workspace_id and finally to
server-global, which is what _herdr_ws_scope already does. `default` is not a
crossing: it is the sentinel for "pass no session flag", so herdr answers on
the ambient server, which for a caller in a pane is that caller's own. A caller
outside any pane keeps an exported scope, so H-18/H-21/H-23 keep their meaning.

The fix lives in mam_herdr, not in the shim heredoc: that heredoc is
materialised into $WORKSPACE_ROOT/.mam/shim/herdr, one file shared by every
checkout bound to a control plane and rewritten on every herdr call, so editing
it would put unreviewed code in the control root and let two checkouts
overwrite each other. S11 fixes that invariant.

lib_py/loop/watcher.py builds its own herdr argv and is the one path that does
not go through mam_herdr, so it applies the same rule via scoped_env.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

.agents/skills/lib.sh

@@ -1373,9 +1373,50 @@ EOF
   fi
 }
 
+# _herdr_call_is_cross_server
+#
+# ISSUE-23: a herdr pane exports HERDR_WORKSPACE_ID / HERDR_PANE_ID /
+# HERDR_TAB_ID, and those ids only mean something on the server that issued
+# them. When a MAM script runs inside one server's pane (the orchestrator's)
+# but targets another (the isolated MAM server named by HERDR_SESSION_NAME),
+# the inherited ids reach the scope filters (_herdr_ws_scope,
+# _herdr_agent_get_scoped, _resolve_herdr_pane_id) as a foreign workspace, and
+# their fail-closed miss (lib.sh explicit-env miss, ISSUE-3 / F-2b) reports a
+# live session as absent: has-session, kill-session, agent prompt and the
+# delegate-job preflight all answer "no such session".
+#
+# True only for a call aimed at a *named* server other than the caller's own:
+#
+#   * target unset or `default` (the lib.sh:97 fallback) is the shim's sentinel
+#     for "pass no session flag", so herdr answers on the ambient server, which
+#     for a caller inside a pane is that same caller's server - not a crossing;
+#   * no HERDR_SESSION means the caller is not in a pane at all, so an exported
+#     HERDR_WORKSPACE_ID is a deliberate scope, not a leak (H-18/H-21/H-23);
+#   * caller == target is the same server.
+_herdr_call_is_cross_server() {
+  local caller="${HERDR_SESSION:-}" target="${HERDR_SESSION_NAME:-}"
+  case "$target" in
+    ""|default) return 1 ;;
+  esac
+  if [ -n "$caller" ] && [ "$caller" != "$target" ]; then
+    return 0
+  fi
+  return 1
+}
+
 mam_herdr() {
   _init_herdr_isolation
-  "$WORKSPACE_ROOT/.mam/shim/herdr" "$@"
+  # Built as an argv array and decided inside `if`: this runs under
+  # `set -euo pipefail` in most callers, where a bare failing test would abort
+  # the whole script instead of this one call.
+  local -a _scope_strip=()
+  if _herdr_call_is_cross_server; then
+    _scope_strip=(env -u HERDR_WORKSPACE_ID -u HERDR_PANE_ID -u HERDR_TAB_ID)
+    if [ "${MAM_SHIM_DEBUG:-0}" = "1" ]; then
+      echo "mam_herdr: dropping caller pane scope ids (caller server '${HERDR_SESSION:-}' != target '${HERDR_SESSION_NAME:-}')" >&2
+    fi
+  fi
+  ${_scope_strip[@]+"${_scope_strip[@]}"} "$WORKSPACE_ROOT/.mam/shim/herdr" "$@"
 }
 
 _herdr() {

.agents/skills/lib_py/loop/watcher.py

@@ -19,6 +19,7 @@ from __future__ import annotations
 
 import json
 import logging
+import os
 import subprocess
 from typing import Any, Dict, Optional, Sequence
 
@@ -38,6 +39,30 @@ TRANSPORT_ERROR = "transport_error"
 BACKOFF_START_S = 1.0
 BACKOFF_MAX_S = 16.0
 
+# ISSUE-23: ids a herdr pane exports about ITS OWN server. Passing them to a
+# call aimed at a different server makes herdr scope the lookup to a workspace
+# that does not exist there, and the target is reported as missing. The shell
+# side drops them in mam_herdr; this is the one python path that runs herdr
+# without going through it.
+CALLER_SCOPE_VARS = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID")
+
+
+def scoped_env(session=None, environ=None):
+    """The environment for a wait aimed at `session`.
+
+    Unchanged unless this process runs inside a pane of another server: only
+    then are the caller's own workspace/pane/tab ids meaningless for the call
+    and dropped. No pane (no HERDR_SESSION) and same-server keep every value.
+    """
+    src = os.environ if environ is None else environ
+    caller = (src.get("HERDR_SESSION") or "").strip()
+    target = (session or "").strip()
+    # `default` is the same sentinel build_argv reads: no --session flag, so
+    # herdr answers on the ambient server - the caller's own.
+    if not caller or not target or target == "default" or caller == target:
+        return dict(src)
+    return dict((k, v) for k, v in src.items() if k not in CALLER_SCOPE_VARS)
+
 
 class WaitOutcome(object):
     __slots__ = ("kind", "agent", "error_code", "detail", "raw")
@@ -92,7 +117,7 @@ def wait_once(target: str, until: Sequence[str] = DEFAULT_UNTIL,
     """One barrier call. `run` is injectable so tests never touch a real herdr."""
     argv = build_argv(target, until, timeout_ms, session=session)
     try:
-        proc = run(argv, capture_output=True, text=True)
+        proc = run(argv, capture_output=True, text=True, env=scoped_env(session))
     except Exception as exc:
         return WaitOutcome(TRANSPORT_ERROR, detail="herdr could not be executed: {}".format(exc))
 

tests/test_issue23_24_startup_path.py

@@ -0,0 +1,304 @@
+"""Issue #23 + #24 — the create_session start path.
+
+Four regressions live here, in the order of the issues:
+
+1. a herdr call aimed at another server must not carry the caller pane's own
+   workspace/pane/tab ids (#23 comments: reproduced on Linux and macOS);
+2. blocking start-up modals are agent-adapter knowledge and are answered while
+   waiting for the TUI (#24);
+3. onboarding delivery is judged by the agent's own `started` event (#23);
+4. a rollback verifies the kill and leaves no `running` row or dangling job (#23).
+
+What is mocked and what is real is stated per test. The dialog tests fix OUR
+contract - which pane string gets which keys - not that the keys drive a real
+agy/grok/codex/opencode TUI; no such TUI can be started here.
+"""
+
+import json
+import os
+import subprocess
+import textwrap
+
+import pytest
+
+
+# --------------------------------------------------------------------------
+# helpers
+# --------------------------------------------------------------------------
+
+def _run_lib(tmp_path, body, env=None):
+    """Source the sandbox lib.sh and run `body`. Unlike the shim-contract
+    helper this keeps HERDR_* exactly as `env` sets it: these tests are about
+    what the caller's pane leaks in."""
+    lib_path = tmp_path / ".agents" / "skills" / "lib.sh"
+    script = 'source "%s"\n_init_herdr_isolation\n%s\n' % (lib_path, body)
+    run_env = dict(os.environ)
+    for var in ("HERDR_SESSION", "HERDR_SESSION_NAME", "HERDR_WORKSPACE_ID",
+                "HERDR_PANE_ID", "HERDR_TAB_ID"):
+        run_env.pop(var, None)
+    run_env.update(env or {})
+    return subprocess.run(["bash", "-c", script], capture_output=True,
+                          text=True, cwd=str(tmp_path), env=run_env)
+
+
+def _mock_state(mock_herdr):
+    with open(mock_herdr) as f:
+        return json.load(f)
+
+
+def _write_mock_state(mock_herdr, state):
+    with open(mock_herdr, "w") as f:
+        json.dump(state, f, indent=2)
+
+
+def _seed_agent(mock_herdr, tmp_path, name, workspace_id, pane_id):
+    state = _mock_state(mock_herdr)
+    state.setdefault("workspaces", [])
+    if not any(w.get("workspace_id") == workspace_id for w in state["workspaces"]):
+        state["workspaces"].append(
+            {"workspace_id": workspace_id, "label": workspace_id, "cwd": str(tmp_path)})
+    state.setdefault("agents", {})[name] = {
+        "name": name, "agent": "agy", "status": "running", "cwd": str(tmp_path),
+        "workspace_id": workspace_id, "pane_id": pane_id, "pid": 9999,
+        "command": "agy", "buffer": "Ready",
+    }
+    state.setdefault("panes", []).append(
+        {"pane_id": pane_id, "workspace_id": workspace_id, "label": name, "buffer": "Ready"})
+    _write_mock_state(mock_herdr, state)
+
+
+def _install_env_recorder(tmp_path, monkeypatch):
+    """A herdr earlier in PATH than the mock that dumps the env it was called
+    with, then execs the mock. The shim resolves the first non-shim herdr on
+    PATH, so this observes exactly what reached the real binary."""
+    rec_dir = tmp_path / "recorder"
+    rec_dir.mkdir(parents=True, exist_ok=True)
+    env_log = tmp_path / "herdr_env.ndjson"
+    real = tmp_path / "bin" / "herdr"
+    script = rec_dir / "herdr"
+    script.write_text(textwrap.dedent('''\
+        #!/usr/bin/env python3
+        import json, os, sys
+        with open(%r, "a") as f:
+            f.write(json.dumps({
+                "argv": sys.argv[1:],
+                "HERDR_WORKSPACE_ID": os.environ.get("HERDR_WORKSPACE_ID"),
+                "HERDR_PANE_ID": os.environ.get("HERDR_PANE_ID"),
+                "HERDR_TAB_ID": os.environ.get("HERDR_TAB_ID"),
+                "HERDR_SESSION": os.environ.get("HERDR_SESSION"),
+                "HERDR_SESSION_NAME": os.environ.get("HERDR_SESSION_NAME"),
+            }) + "\\n")
+        os.execv(%r, [%r] + sys.argv[1:])
+        ''') % (str(env_log), str(real), str(real)))
+    script.chmod(0o755)
+    monkeypatch.setenv("PATH", "%s:%s" % (rec_dir, os.environ["PATH"]))
+    return env_log
+
+
+def _recorded(env_log):
+    if not os.path.exists(env_log):
+        return []
+    return [json.loads(l) for l in open(env_log) if l.strip()]
+
+
+# --------------------------------------------------------------------------
+# item 1 — caller pane scope must not leak across servers
+# --------------------------------------------------------------------------
+
+CROSS = {"HERDR_SESSION": "default", "HERDR_SESSION_NAME": "mam-isolated",
+         "HERDR_WORKSPACE_ID": "w5", "HERDR_PANE_ID": "w5:p2",
+         "HERDR_TAB_ID": "w5:t1"}
+
+
+def test_s1_cross_server_call_drops_the_caller_panes_scope_ids(
+        mam_sandbox, mock_herdr, mock_agents, monkeypatch):
+    """S1 (mock herdr): the caller sits in another server's pane. has-session,
+    kill-session and agent prompt must find the agent, and the three ids must
+    not reach herdr."""
+    tmp_path = mam_sandbox
+    env_log = _install_env_recorder(tmp_path, monkeypatch)
+    _seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
+
+    res = _run_lib(tmp_path, 'herdr has-session -t creator-agy-01', env=CROSS)
+    assert res.returncode == 0, res.stdout + res.stderr
+
+    res = _run_lib(tmp_path, 'herdr agent prompt creator-agy-01 "hello"', env=CROSS)
+    assert res.returncode == 0, res.stdout + res.stderr
+
+    res = _run_lib(tmp_path, 'herdr kill-session -t creator-agy-01', env=CROSS)
+    assert res.returncode == 0, res.stdout + res.stderr
+
+    calls = _recorded(env_log)
+    assert calls, "the recorder saw no herdr call"
+    for call in calls:
+        assert call["HERDR_WORKSPACE_ID"] is None, call
+        assert call["HERDR_PANE_ID"] is None, call
+        assert call["HERDR_TAB_ID"] is None, call
+
+    # the kill really happened, i.e. this is not a silent no-op
+    assert "creator-agy-01" not in _mock_state(mock_herdr).get("agents", {})
+
+
+def test_s1b_the_delegate_job_wrappers_bare_herdr_is_the_lib_sh_function(
+        mam_sandbox, mock_herdr, mock_agents, monkeypatch):
+    """S1b (mock herdr): the wrapper sources lib.sh and then calls a bare
+    `herdr` (wrapper :918 preflight). A shell function beats PATH, so that call
+    is mam_herdr and is fixed by item 1 too - this is why #23's preflight
+    failure is cured by the same change."""
+    tmp_path = mam_sandbox
+    _install_env_recorder(tmp_path, monkeypatch)
+    _seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
+
+    res = _run_lib(tmp_path, 'echo "kind=$(type -t herdr)"\nherdr has-session -t creator-agy-01',
+                   env=CROSS)
+    assert "kind=function" in res.stdout, res.stdout + res.stderr
+    assert res.returncode == 0, res.stdout + res.stderr
+
+
+def test_s2_same_server_keeps_the_explicit_workspace_scope(
+        mam_sandbox, mock_herdr, mock_agents, monkeypatch):
+    """S2 (mock herdr): on the target's own server the exported scope still
+    scopes - a same-named agent in another workspace is not addressed (H-22)."""
+    tmp_path = mam_sandbox
+    _seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
+    same = {"HERDR_SESSION": "mam-isolated", "HERDR_SESSION_NAME": "mam-isolated",
+            "HERDR_WORKSPACE_ID": "w2"}
+
+    before = len(_mock_state(mock_herdr).get("calls", []))
+    res = _run_lib(tmp_path, 'herdr agent prompt creator-agy-01 "hello from w2"', env=same)
+    assert res.returncode != 0, res.stdout + res.stderr
+    new = _mock_state(mock_herdr).get("calls", [])[before:]
+    prompts = [c for c in new if len(c) >= 2 and c[0] == "agent" and c[1] == "prompt"]
+    assert prompts == [], new
+
+
+def test_s2b_a_caller_outside_any_pane_keeps_the_explicit_scope(
+        mam_sandbox, mock_herdr, mock_agents):
+    """S2b (mock herdr): no HERDR_SESSION means the caller is not in a pane at
+    all, so an exported HERDR_WORKSPACE_ID is a deliberate scope, not a leak.
+    This is the case H-18/H-21/H-23 fix."""
+    tmp_path = mam_sandbox
+    _seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
+
+    res = _run_lib(tmp_path, 'herdr has-session -t creator-agy-01',
+                   env={"HERDR_SESSION_NAME": "mam-isolated", "HERDR_WORKSPACE_ID": "w2"})
+    assert res.returncode == 1, "an explicit out-of-scope id must still hide the agent"
+
+    res = _run_lib(tmp_path, 'herdr has-session -t creator-agy-01',
+                   env={"HERDR_SESSION_NAME": "mam-isolated", "HERDR_WORKSPACE_ID": "w1"})
+    assert res.returncode == 0, res.stdout + res.stderr
+
+
+def test_s3_shim_contract_regressions_still_pass(mam_sandbox):
+    """S3: H-18 / H-21 / H-23 are the tests that fix today's scoping meaning.
+    Run them explicitly as this change's regression guard."""
+    repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    res = subprocess.run(
+        [os.path.join(repo, ".venv", "bin", "python"), "-m", "pytest", "-q",
+         "-p", "no:cacheprovider",
+         "tests/test_herdr_shim_contract.py", "-k",
+         "h18 or h21 or h22 or h23"],
+        capture_output=True, text=True, cwd=repo,
+        env=dict(os.environ, PYTHONDONTWRITEBYTECODE="1"))
+    assert res.returncode == 0, res.stdout[-4000:] + res.stderr[-2000:]
+
+
+def test_s11_this_branch_does_not_change_the_generated_shim(mam_sandbox):
+    """S11: the item-1 fix lives in mam_herdr, never in the heredoc that
+    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."""
+    repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    base = os.environ.get("MAM_BASE_SHA", "1e78dae")
+    show = subprocess.run(["git", "show", "%s:.agents/skills/lib.sh" % base],
+                          capture_output=True, text=True, cwd=repo)
+    if show.returncode != 0:
+        pytest.skip("base %s not available in this checkout" % base)
+    with open(os.path.join(repo, ".agents", "skills", "lib.sh")) as f:
+        head = f.read()
+    assert _heredoc_body(head) == _heredoc_body(show.stdout), (
+        "the shim heredoc changed: the generated .mam/shim/herdr of this branch "
+        "would differ from the control root's, and the two would overwrite "
+        "each other on every herdr call")
+
+
+def _heredoc_body(text):
+    lines = text.splitlines(True)
+    start = None
+    for i, line in enumerate(lines):
+        if line.strip() == "cat <<'EOF' > \"$tmp_file\"":
+            start = i + 1
+            break
+    assert start is not None, "shim heredoc not found in lib.sh"
+    for j in range(start, len(lines)):
+        if lines[j].rstrip("\n") == "EOF":
+            return "".join(lines[start:j])
+    raise AssertionError("unterminated shim heredoc")
+
+
+@pytest.mark.parametrize("env,expected_rc", [(CROSS, 1), ({"HERDR_SESSION_NAME": "mam-isolated"}, 1)])
+def test_s12_mam_herdr_never_aborts_its_caller_under_set_e(
+        mam_sandbox, mock_herdr, mock_agents, env, expected_rc):
+    """S12 (mock herdr): mam_herdr runs inside scripts using `set -euo
+    pipefail`. Neither branch of the cross-server test may abort the shell,
+    and the child's rc must be the function's rc."""
+    tmp_path = mam_sandbox
+    body = ('set -euo pipefail\n'
+            'rc=0\n'
+            'mam_herdr has-session -t nobody-here || rc=$?\n'
+            'echo "missing_rc=$rc"\n'
+            '_seen=0\n'
+            'mam_herdr has-session -t creator-agy-01 || _seen=$?\n'
+            'echo "present_rc=$_seen"\n'
+            'echo REACHED_END\n')
+    _seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
+    lib_path = tmp_path / ".agents" / "skills" / "lib.sh"
+    script = 'source "%s"\n%s' % (lib_path, body)
+    run_env = dict(os.environ)
+    for var in ("HERDR_SESSION", "HERDR_SESSION_NAME", "HERDR_WORKSPACE_ID",
+                "HERDR_PANE_ID", "HERDR_TAB_ID"):
+        run_env.pop(var, None)
+    run_env.update(env)
+    res = subprocess.run(["bash", "-c", script], capture_output=True, text=True,
+                         cwd=str(tmp_path), env=run_env)
+    assert "REACHED_END" in res.stdout, res.stdout + res.stderr
+    assert "missing_rc=%d" % expected_rc in res.stdout, res.stdout + res.stderr
+    assert "present_rc=0" in res.stdout, res.stdout + res.stderr
+
+
+def test_s13_the_loop_watcher_applies_the_same_rule(mam_sandbox):
+    """S13 (pure unit): watcher.py builds its own herdr argv and never goes
+    through mam_herdr, so it needs the same rule."""
+    import sys
+    repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    sys.path.insert(0, os.path.join(repo, ".agents", "skills"))
+    from lib_py.loop import watcher
+
+    caller_pane = {"HERDR_SESSION": "default", "HERDR_WORKSPACE_ID": "w5",
+                   "HERDR_PANE_ID": "w5:p2", "HERDR_TAB_ID": "w5:t1", "PATH": "/usr/bin"}
+
+    cross = watcher.scoped_env("mam-isolated", environ=caller_pane)
+    assert "HERDR_WORKSPACE_ID" not in cross and "HERDR_PANE_ID" not in cross
+    assert "HERDR_TAB_ID" not in cross and cross["PATH"] == "/usr/bin"
+
+    same = watcher.scoped_env("default", environ=caller_pane)
+    assert same["HERDR_WORKSPACE_ID"] == "w5"
+
+    outside = watcher.scoped_env("mam-isolated", environ={"HERDR_WORKSPACE_ID": "w5"})
+    assert outside["HERDR_WORKSPACE_ID"] == "w5"
+
+    none_target = watcher.scoped_env(None, environ=caller_pane)
+    assert none_target["HERDR_WORKSPACE_ID"] == "w5"
+
+    seen = {}
+
+    def _run(argv, **kw):
+        seen["env"] = kw.get("env")
+
+        class P:
+            returncode, stdout, stderr = 0, "{}", ""
+        return P()
+
+    watcher.wait_once("agt", session="iso-x", run=_run)
+    assert seen["env"] is not None and "HERDR_SESSION" not in ("",)