fix(issue-23): the caller's server comes from HERDR_SOCKET_PATH, not only HERDR_SESSION
The live trial of this branch found the cross-server verdict inert on this host: _herdr_call_is_cross_server read the caller's server from HERDR_SESSION, and herdr 0.8.2 does not export that variable to a pane of the default server - the orchestrator's usual home, and exactly the pane the #23 leak comes from. Every call was judged "not crossing", the three ids were never dropped, and all four trial onboardings ended "no started within 180s". My own pane sits in a named server and does carry HERDR_SESSION, which is how the assumption got past me. HERDR_SOCKET_PATH is exported to every pane and its layout names the server: .../.config/herdr/herdr.sock is the default server and .../.config/herdr/sessions/<name>/herdr.sock a named one - the convention `herdr session list --json` reports and the shim already uses to find a named server's socket. _herdr_caller_server prefers HERDR_SESSION when a herdr does export it, otherwise reads the socket's shape (suffix-matched, not $HOME- anchored), and yields "" for anything unrecognised so an exported scope is respected rather than guessed away. The rest of the rule is unchanged from3bcaa39: a `default` or empty target is the ambient server, no caller server means no pane, caller == target is the same server. watcher.caller_server applies the same rule for the loop's own herdr calls. Reproduced against the live server in the trial's exact env:10a678canswers "NOT crossing / has-session NOT FOUND"; this commit answers "caller server default / crossing / FOUND". S1c pins that env end to end with the recorder asserting the ids never reach herdr; S1d-S1h pin the verdict, the named-socket same-server and cross-server cases, HERDR_SESSION precedence, and the unrecognised layout. The3bcaa39tests are unchanged and still pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
10a678c010
commit
f7cee00dec
3 files changed
+176
-7
No files matched your search
+47
-4
@@ -1384,19 +1384,62 @@ EOF
|
|||||||
# live session as absent: has-session, kill-session, agent prompt and the
|
# live session as absent: has-session, kill-session, agent prompt and the
|
||||||
# delegate-job preflight all answer "no such session".
|
# delegate-job preflight all answer "no such session".
|
||||||
#
|
#
|
||||||
|
# _herdr_caller_server — the name of the server whose pane this process runs
|
||||||
|
# in; empty when it is not in a pane at all.
|
||||||
|
#
|
||||||
|
# HERDR_SESSION is used when present, but herdr 0.8.2 does not export it to a
|
||||||
|
# pane of the *default* server (the orchestrator's usual home), which is
|
||||||
|
# exactly the pane the #23 leak comes from - the live trial of this branch
|
||||||
|
# found every call judged "not crossing" for that reason. HERDR_SOCKET_PATH is
|
||||||
|
# exported to every pane, and its layout names the server:
|
||||||
|
#
|
||||||
|
# $HOME/.config/herdr/herdr.sock -> default
|
||||||
|
# $HOME/.config/herdr/sessions/<name>/herdr.sock -> <name>
|
||||||
|
#
|
||||||
|
# (the same convention lib.sh's shim already relies on to find a named
|
||||||
|
# server's socket). Matched by shape, not by $HOME, so a redirected HOME does
|
||||||
|
# not break it. An unrecognised layout yields "" - treated as "not in a pane",
|
||||||
|
# which keeps today's behaviour rather than guessing.
|
||||||
|
_herdr_caller_server() {
|
||||||
|
if [ -n "${HERDR_SESSION:-}" ]; then
|
||||||
|
printf '%s\n' "$HERDR_SESSION"
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
local sock="${HERDR_SOCKET_PATH:-}"
|
||||||
|
case "$sock" in
|
||||||
|
*/.config/herdr/herdr.sock)
|
||||||
|
printf 'default\n'
|
||||||
|
;;
|
||||||
|
*/.config/herdr/sessions/*/herdr.sock)
|
||||||
|
local d="${sock%/herdr.sock}"
|
||||||
|
printf '%s\n' "${d##*/}"
|
||||||
|
;;
|
||||||
|
*)
|
||||||
|
printf '\n'
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
# True only for a call aimed at a *named* server other than the caller's own:
|
# 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
|
# * 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 "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;
|
# 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
|
# * no caller server (_herdr_caller_server empty) means the caller is not in
|
||||||
# HERDR_WORKSPACE_ID is a deliberate scope, not a leak (H-18/H-21/H-23);
|
# 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.
|
# * caller == target is the same server.
|
||||||
|
#
|
||||||
|
# A caller in the default server's pane targeting a named server IS a crossing
|
||||||
|
# (the reported #23 scenario): `default` is a real server name on the caller's
|
||||||
|
# side, only as a *target* is it the "ambient" sentinel.
|
||||||
_herdr_call_is_cross_server() {
|
_herdr_call_is_cross_server() {
|
||||||
local caller="${HERDR_SESSION:-}" target="${HERDR_SESSION_NAME:-}"
|
local caller target="${HERDR_SESSION_NAME:-}"
|
||||||
case "$target" in
|
case "$target" in
|
||||||
""|default) return 1 ;;
|
""|default) return 1 ;;
|
||||||
esac
|
esac
|
||||||
|
caller="$(_herdr_caller_server)"
|
||||||
if [ -n "$caller" ] && [ "$caller" != "$target" ]; then
|
if [ -n "$caller" ] && [ "$caller" != "$target" ]; then
|
||||||
return 0
|
return 0
|
||||||
fi
|
fi
|
||||||
@@ -1412,7 +1455,7 @@ mam_herdr() {
|
|||||||
if _herdr_call_is_cross_server; then
|
if _herdr_call_is_cross_server; then
|
||||||
_scope_strip=(env -u HERDR_WORKSPACE_ID -u HERDR_PANE_ID -u HERDR_TAB_ID)
|
_scope_strip=(env -u HERDR_WORKSPACE_ID -u HERDR_PANE_ID -u HERDR_TAB_ID)
|
||||||
if [ "${MAM_SHIM_DEBUG:-0}" = "1" ]; then
|
if [ "${MAM_SHIM_DEBUG:-0}" = "1" ]; then
|
||||||
echo "mam_herdr: dropping caller pane scope ids (caller server '${HERDR_SESSION:-}' != target '${HERDR_SESSION_NAME:-}')" >&2
|
echo "mam_herdr: dropping caller pane scope ids (caller server '$(_herdr_caller_server)' != target '${HERDR_SESSION_NAME:-}')" >&2
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
${_scope_strip[@]+"${_scope_strip[@]}"} "$WORKSPACE_ROOT/.mam/shim/herdr" "$@"
|
${_scope_strip[@]+"${_scope_strip[@]}"} "$WORKSPACE_ROOT/.mam/shim/herdr" "$@"
|
||||||
|
|||||||
@@ -47,15 +47,42 @@ BACKOFF_MAX_S = 16.0
|
|||||||
CALLER_SCOPE_VARS = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID")
|
CALLER_SCOPE_VARS = ("HERDR_WORKSPACE_ID", "HERDR_PANE_ID", "HERDR_TAB_ID")
|
||||||
|
|
||||||
|
|
||||||
|
def caller_server(environ=None):
|
||||||
|
"""The server whose pane this process runs in, "" when not in a pane.
|
||||||
|
|
||||||
|
HERDR_SESSION when present; otherwise the shape of HERDR_SOCKET_PATH, which
|
||||||
|
herdr exports to every pane (HERDR_SESSION is absent in a default-server
|
||||||
|
pane on herdr 0.8.2 - the orchestrator's usual home, and where the #23 leak
|
||||||
|
comes from). Same convention as lib.sh's _herdr_caller_server:
|
||||||
|
|
||||||
|
.../.config/herdr/herdr.sock -> "default"
|
||||||
|
.../.config/herdr/sessions/<name>/herdr.sock -> <name>
|
||||||
|
|
||||||
|
Matched by shape, not by $HOME. Anything else is "" (not in a pane).
|
||||||
|
"""
|
||||||
|
src = os.environ if environ is None else environ
|
||||||
|
sess = (src.get("HERDR_SESSION") or "").strip()
|
||||||
|
if sess:
|
||||||
|
return sess
|
||||||
|
sock = (src.get("HERDR_SOCKET_PATH") or "").strip()
|
||||||
|
parts = sock.split("/")
|
||||||
|
if len(parts) >= 4 and parts[-1] == "herdr.sock":
|
||||||
|
if parts[-3:-1] == [".config", "herdr"]:
|
||||||
|
return "default"
|
||||||
|
if len(parts) >= 5 and parts[-4:-2] == ["herdr", "sessions"] and parts[-5] == ".config":
|
||||||
|
return parts[-2]
|
||||||
|
return ""
|
||||||
|
|
||||||
|
|
||||||
def scoped_env(session=None, environ=None):
|
def scoped_env(session=None, environ=None):
|
||||||
"""The environment for a wait aimed at `session`.
|
"""The environment for a wait aimed at `session`.
|
||||||
|
|
||||||
Unchanged unless this process runs inside a pane of another server: only
|
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
|
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.
|
and dropped. No pane and same-server keep every value.
|
||||||
"""
|
"""
|
||||||
src = os.environ if environ is None else environ
|
src = os.environ if environ is None else environ
|
||||||
caller = (src.get("HERDR_SESSION") or "").strip()
|
caller = caller_server(src)
|
||||||
target = (session or "").strip()
|
target = (session or "").strip()
|
||||||
# `default` is the same sentinel build_argv reads: no --session flag, so
|
# `default` is the same sentinel build_argv reads: no --session flag, so
|
||||||
# herdr answers on the ambient server - the caller's own.
|
# herdr answers on the ambient server - the caller's own.
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ def _run_lib(tmp_path, body, env=None):
|
|||||||
script = 'source "%s"\n_init_herdr_isolation\n%s\n' % (lib_path, body)
|
script = 'source "%s"\n_init_herdr_isolation\n%s\n' % (lib_path, body)
|
||||||
run_env = dict(os.environ)
|
run_env = dict(os.environ)
|
||||||
for var in ("HERDR_SESSION", "HERDR_SESSION_NAME", "HERDR_WORKSPACE_ID",
|
for var in ("HERDR_SESSION", "HERDR_SESSION_NAME", "HERDR_WORKSPACE_ID",
|
||||||
"HERDR_PANE_ID", "HERDR_TAB_ID"):
|
"HERDR_PANE_ID", "HERDR_TAB_ID", "HERDR_SOCKET_PATH"):
|
||||||
run_env.pop(var, None)
|
run_env.pop(var, None)
|
||||||
run_env.update(env or {})
|
run_env.update(env or {})
|
||||||
return subprocess.run(["bash", "-c", script], capture_output=True,
|
return subprocess.run(["bash", "-c", script], capture_output=True,
|
||||||
@@ -110,6 +110,16 @@ CROSS = {"HERDR_SESSION": "default", "HERDR_SESSION_NAME": "mam-isolated",
|
|||||||
"HERDR_WORKSPACE_ID": "w5", "HERDR_PANE_ID": "w5:p2",
|
"HERDR_WORKSPACE_ID": "w5", "HERDR_PANE_ID": "w5:p2",
|
||||||
"HERDR_TAB_ID": "w5:t1"}
|
"HERDR_TAB_ID": "w5:t1"}
|
||||||
|
|
||||||
|
# The orchestrator pane as the live trial of this branch measured it (herdr
|
||||||
|
# 0.8.2): no HERDR_SESSION at all, the default server's socket, and that
|
||||||
|
# server's own ids. This is the env in which the HERDR_SESSION-based verdict
|
||||||
|
# was always "not crossing" and #23 reproduced unchanged.
|
||||||
|
DEFAULT_SOCK = "/home/someone/.config/herdr/herdr.sock"
|
||||||
|
NAMED_SOCK = "/home/someone/.config/herdr/sessions/%s/herdr.sock"
|
||||||
|
TRIAL_PANE = {"HERDR_SOCKET_PATH": DEFAULT_SOCK, "HERDR_WORKSPACE_ID": "w6",
|
||||||
|
"HERDR_PANE_ID": "w6:p1", "HERDR_TAB_ID": "w6:t1",
|
||||||
|
"HERDR_SESSION_NAME": "multi-agent-mux"}
|
||||||
|
|
||||||
|
|
||||||
def test_s1_cross_server_call_drops_the_caller_panes_scope_ids(
|
def test_s1_cross_server_call_drops_the_caller_panes_scope_ids(
|
||||||
mam_sandbox, mock_herdr, mock_agents, monkeypatch):
|
mam_sandbox, mock_herdr, mock_agents, monkeypatch):
|
||||||
@@ -190,6 +200,83 @@ def test_s2b_a_caller_outside_any_pane_keeps_the_explicit_scope(
|
|||||||
assert res.returncode == 0, res.stdout + res.stderr
|
assert res.returncode == 0, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1c_the_real_default_server_pane_is_a_crossing(
|
||||||
|
mam_sandbox, mock_herdr, mock_agents, monkeypatch):
|
||||||
|
"""S1c (trial finding, req. 4): the caller's server comes from the socket
|
||||||
|
path when HERDR_SESSION is absent. In the measured orchestrator pane the
|
||||||
|
three ids must be dropped and the agent on the named server found."""
|
||||||
|
tmp_path = mam_sandbox
|
||||||
|
env_log = _install_env_recorder(tmp_path, monkeypatch)
|
||||||
|
_seed_agent(mock_herdr, tmp_path, "trial-agy", "w1", "w1:p5")
|
||||||
|
|
||||||
|
res = _run_lib(tmp_path, 'herdr has-session -t trial-agy', env=TRIAL_PANE)
|
||||||
|
assert res.returncode == 0, "the trial's exact failure: " + 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
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1d_the_verdict_itself_on_the_trial_pane(mam_sandbox):
|
||||||
|
"""S1d: the function the trial printed, in the trial's env: crossing."""
|
||||||
|
res = _run_lib(mam_sandbox,
|
||||||
|
'echo "caller=$(_herdr_caller_server)"\n'
|
||||||
|
'if _herdr_call_is_cross_server; then echo VERDICT=crossing; '
|
||||||
|
'else echo VERDICT=not-crossing; fi',
|
||||||
|
env=TRIAL_PANE)
|
||||||
|
assert "caller=default" in res.stdout, res.stdout + res.stderr
|
||||||
|
assert "VERDICT=crossing" in res.stdout, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1e_a_named_server_pane_calling_its_own_server_keeps_its_scope(
|
||||||
|
mam_sandbox, mock_herdr, mock_agents):
|
||||||
|
"""S1e (req. 4): a caller in a named server's pane (socket under
|
||||||
|
sessions/<name>/) targeting that same server is not a crossing - the
|
||||||
|
exported workspace id still scopes, as in S2."""
|
||||||
|
tmp_path = mam_sandbox
|
||||||
|
_seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
|
||||||
|
same = {"HERDR_SOCKET_PATH": NAMED_SOCK % "mam-isolated",
|
||||||
|
"HERDR_SESSION_NAME": "mam-isolated", "HERDR_WORKSPACE_ID": "w2"}
|
||||||
|
res = _run_lib(tmp_path, 'echo "caller=$(_herdr_caller_server)"\nherdr has-session -t creator-agy-01',
|
||||||
|
env=same)
|
||||||
|
assert "caller=mam-isolated" in res.stdout, res.stdout + res.stderr
|
||||||
|
assert res.returncode == 1, "same server: the explicit w2 scope must still hide a w1 agent"
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1f_a_named_server_pane_calling_another_named_server_is_a_crossing(
|
||||||
|
mam_sandbox, mock_herdr, mock_agents):
|
||||||
|
"""S1f: named socket, different named target, no HERDR_SESSION - crossing."""
|
||||||
|
tmp_path = mam_sandbox
|
||||||
|
_seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
|
||||||
|
other = {"HERDR_SOCKET_PATH": NAMED_SOCK % "mam-other",
|
||||||
|
"HERDR_SESSION_NAME": "mam-isolated", "HERDR_WORKSPACE_ID": "w2"}
|
||||||
|
res = _run_lib(tmp_path, 'herdr has-session -t creator-agy-01', env=other)
|
||||||
|
assert res.returncode == 0, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1g_herdr_session_wins_over_the_socket_when_both_are_present(mam_sandbox):
|
||||||
|
"""S1g (req. 1): HERDR_SESSION, when a herdr exports it, is authoritative
|
||||||
|
over the socket-derived name."""
|
||||||
|
res = _run_lib(mam_sandbox, 'echo "caller=$(_herdr_caller_server)"',
|
||||||
|
env={"HERDR_SESSION": "from-herdr", "HERDR_SOCKET_PATH": DEFAULT_SOCK})
|
||||||
|
assert "caller=from-herdr" in res.stdout, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1h_an_unrecognised_socket_layout_is_not_in_a_pane(mam_sandbox, mock_herdr, mock_agents):
|
||||||
|
"""S1h: a socket path that is neither layout yields no caller server, so
|
||||||
|
an exported scope is respected (today's behaviour), never guessed away."""
|
||||||
|
tmp_path = mam_sandbox
|
||||||
|
_seed_agent(mock_herdr, tmp_path, "creator-agy-01", "w1", "w1:p5")
|
||||||
|
res = _run_lib(tmp_path, 'echo "caller=[$(_herdr_caller_server)]"\nherdr has-session -t creator-agy-01',
|
||||||
|
env={"HERDR_SOCKET_PATH": "/tmp/odd/herdr.sock",
|
||||||
|
"HERDR_SESSION_NAME": "mam-isolated", "HERDR_WORKSPACE_ID": "w2"})
|
||||||
|
assert "caller=[]" in res.stdout, res.stdout + res.stderr
|
||||||
|
assert res.returncode == 1, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
def test_s3_shim_contract_regressions_still_pass(mam_sandbox):
|
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.
|
"""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."""
|
Run them explicitly as this change's regression guard."""
|
||||||
@@ -291,6 +378,18 @@ def test_s13_the_loop_watcher_applies_the_same_rule(mam_sandbox):
|
|||||||
none_target = watcher.scoped_env(None, environ=caller_pane)
|
none_target = watcher.scoped_env(None, environ=caller_pane)
|
||||||
assert none_target["HERDR_WORKSPACE_ID"] == "w5"
|
assert none_target["HERDR_WORKSPACE_ID"] == "w5"
|
||||||
|
|
||||||
|
# trial finding: no HERDR_SESSION, the caller's server comes from the socket
|
||||||
|
trial = {"HERDR_SOCKET_PATH": DEFAULT_SOCK, "HERDR_WORKSPACE_ID": "w6",
|
||||||
|
"HERDR_PANE_ID": "w6:p1", "HERDR_TAB_ID": "w6:t1"}
|
||||||
|
assert watcher.caller_server(trial) == "default"
|
||||||
|
assert "HERDR_WORKSPACE_ID" not in watcher.scoped_env("multi-agent-mux", environ=trial)
|
||||||
|
named_same = {"HERDR_SOCKET_PATH": NAMED_SOCK % "iso-x", "HERDR_WORKSPACE_ID": "w2"}
|
||||||
|
assert watcher.caller_server(named_same) == "iso-x"
|
||||||
|
assert watcher.scoped_env("iso-x", environ=named_same)["HERDR_WORKSPACE_ID"] == "w2"
|
||||||
|
assert "HERDR_WORKSPACE_ID" not in watcher.scoped_env("iso-y", environ=named_same)
|
||||||
|
assert watcher.caller_server({"HERDR_SOCKET_PATH": "/tmp/odd/herdr.sock"}) == ""
|
||||||
|
assert watcher.caller_server({"HERDR_SESSION": "s", "HERDR_SOCKET_PATH": DEFAULT_SOCK}) == "s"
|
||||||
|
|
||||||
seen = {}
|
seen = {}
|
||||||
|
|
||||||
def _run(argv, **kw):
|
def _run(argv, **kw):
|
||||||
|
|||||||
Reference in new issue
Block a user