From 2679ddfffd0a57104a2e2f0dfd877fc54429d20b Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 14 Sep 2026 15:24:10 +0000 Subject: [PATCH] feat(issue-21): resume only at the original cwd; persist the control-plane binding on the already-running path; fail loudly when a worktree binding cannot be computed - A11: resume refuses (exit 2) a --workspace other than the row's pane.cwd, and a pane.cwd that no longer exists (checked first: mam_abs_workspace returns a missing path unchanged). Before, claude/grok failed with a misleading "No saved session" and class A agents started a new conversation in the wrong directory. Rows without pane.cwd are not checked. - A12 (recommendation 1): create and resume no longer swallow a failed binding computation. A worktree (--workspace != control root) is refused with exit 2 and nothing spawned or recorded; the control root itself warns and records the row without control_plane. - A13 (recommendation 2): resume computes the binding before the herdr-alive check, so the already-running path records it too (update_yaml_resumed.sh fills a missing field only). Pane forwarding and the whitespace refusal stay on the spawn path. Tests W7-W9. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G --- .../scripts/create_session.sh | 15 +++- .../scripts/resume_session.sh | 68 ++++++++++++++---- tests/test_issue21_phase2_loop_a.py | 70 +++++++++++++++++++ 3 files changed, 138 insertions(+), 15 deletions(-) diff --git a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh index de2e809..05cbabd 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -208,7 +208,20 @@ fi # lib.sh splits those flags unquoted (and, with env flags, never reuses an # idle pane - `agent start --` would pass them to the agent as arguments), so a # binding with whitespace that would have to be forwarded is refused. -CP_JSON="$(mam_control_plane_json "$WORKSPACE" 2>/dev/null || true)" +# A binding that cannot be computed is not swallowed (Issue #21 phase 2, A12): +# for a worktree (--workspace != control root) that would silently put the +# session on the wrong control plane, so refuse; for the control root itself +# warn and record the row without control_plane. +cp_rc=0 +CP_JSON="$(mam_control_plane_json "$WORKSPACE")" || cp_rc=$? +if [ "$cp_rc" -ne 0 ] || [ -z "$CP_JSON" ]; then + if [ "$(mam_abs_workspace "$WORKSPACE")" != "$(mam_abs_workspace "$WORKSPACE_ROOT")" ]; then + echo "ERROR: cannot compute the control-plane binding (rc=$cp_rc) for --workspace '$WORKSPACE' (control root '$WORKSPACE_ROOT'); refusing to start a session that would use the wrong control plane" >&2 + exit 2 + fi + echo "WARNING: control-plane binding unavailable (rc=$cp_rc); the row is recorded without control_plane" >&2 + CP_JSON="" +fi CP_PREFIX="" if [ -n "$CP_JSON" ]; then cp_rc=0 diff --git a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh index 72fee21..c5a58ff 100755 --- a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh +++ b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh @@ -47,6 +47,57 @@ case "$AGENT" in esac [ -n "$SESSION_NAME" ] || { echo "ERROR: --session required" >&2; exit 2; } +# Issue #21 phase 2 (A11): a session resumes only in the directory it ran in +# (its conversation is keyed by that cwd; a class A agent would otherwise start +# a fresh conversation in the wrong tree). Existence first: mam_abs_workspace +# returns a missing path unchanged, which would read as "a different path". +ROW_CWD="$(MAM_STATE_JSON="$(load_state_json)" SESSION_NAME="$SESSION_NAME" python3 -c ' +import json, os +d = json.loads(os.environ.get("MAM_STATE_JSON") or "{}") +for s in d.get("herdr_sessions", []): + if s.get("name") == os.environ["SESSION_NAME"]: + print((s.get("pane") or {}).get("cwd", "") or ""); break +' 2>/dev/null || true)" +if [ -n "$ROW_CWD" ]; then + if [ ! -d "$ROW_CWD" ]; then + echo "ERROR: session '$SESSION_NAME' ran in '$ROW_CWD', which no longer exists (worktree removed?). It cannot be resumed; recreate the worktree at that path, or create a new session." >&2 + exit 2 + fi + if [ "$(mam_abs_workspace "$ROW_CWD")" != "$(mam_abs_workspace "$WORKSPACE")" ]; then + echo "ERROR: session '$SESSION_NAME' was created in '$ROW_CWD'; resume it with --workspace '$ROW_CWD' (its conversation is keyed by that directory)." >&2 + exit 2 + fi +fi + +# Issue #21 (item 3, D11): the control-plane binding recorded on the row wins +# over the caller's environment (a difference is reported on stderr); a legacy +# row without one uses the caller's binding, which update_yaml_resumed.sh then +# records - on the already-running path too (phase 2, A13). A binding that +# cannot be computed is refused for a worktree and warned about for the +# control root itself (A12), as in create. +ROW_CP="$(MAM_STATE_JSON="$(load_state_json)" SESSION_NAME="$SESSION_NAME" python3 -c " +import json, os +d = json.loads(os.environ.get('MAM_STATE_JSON') or '{}') +for s in d.get('herdr_sessions', []): + if s.get('name') == os.environ['SESSION_NAME'] and isinstance(s.get('control_plane'), dict): + print(json.dumps(s['control_plane'])) + break +" 2>/dev/null || true)" +CP_ROOT="$(printf '%s' "$ROW_CP" | python3 -c 'import json,sys +try: print(json.load(sys.stdin).get("workspace_root") or "") +except Exception: print("")' 2>/dev/null || true)" +CP_ROOT="${CP_ROOT:-$WORKSPACE_ROOT}" +cp_rc=0 +CP_JSON="$(mam_control_plane_json "$WORKSPACE" "$ROW_CP")" || cp_rc=$? +if [ "$cp_rc" -ne 0 ] || [ -z "$CP_JSON" ]; then + if [ "$(mam_abs_workspace "$WORKSPACE")" != "$(mam_abs_workspace "$CP_ROOT")" ]; then + echo "ERROR: cannot compute the control-plane binding (rc=$cp_rc) for --workspace '$WORKSPACE' (control root '$CP_ROOT'); refusing to resume a session that would use the wrong control plane" >&2 + exit 2 + fi + echo "WARNING: control-plane binding unavailable (rc=$cp_rc); the row is kept without control_plane" >&2 + CP_JSON="" +fi + # 1. Resolve the session id UUID=$(bash "$(dirname "${BASH_SOURCE[0]}")/resolve_session_id.sh" \ --workspace "$WORKSPACE" --agent "$AGENT" --session "$SESSION_NAME") @@ -91,7 +142,7 @@ if herdr has-session -t "$SESSION_NAME" 2>/dev/null; then fi echo "herdr '$SESSION_NAME' already running." # Just update YAML to make sure it's set to running - bash "$(dirname "${BASH_SOURCE[0]}")/update_yaml_resumed.sh" \ + CONTROL_PLANE_JSON="$CP_JSON" bash "$(dirname "${BASH_SOURCE[0]}")/update_yaml_resumed.sh" \ --session "$SESSION_NAME" --uuid "$UUID" --agent "$AGENT" --workspace "$WORKSPACE" \ --herdr-session "$HERDR_SESSION_NAME" \ ${HERDR_WORKSPACE_OPT:+--herdr-workspace "$HERDR_WORKSPACE_OPT"} @@ -150,19 +201,8 @@ else fi fi -# Issue #21 (item 3, D11): the control-plane binding recorded on the row wins -# over the caller's environment (a difference is reported on stderr); a legacy -# row without one uses the caller's binding, which update_yaml_resumed.sh then -# records. Forwarded to the pane only when it differs from --workspace's own. -ROW_CP="$(MAM_STATE_JSON="$(load_state_json)" SESSION_NAME="$SESSION_NAME" python3 -c " -import json, os -d = json.loads(os.environ.get('MAM_STATE_JSON') or '{}') -for s in d.get('herdr_sessions', []): - if s.get('name') == os.environ['SESSION_NAME'] and isinstance(s.get('control_plane'), dict): - print(json.dumps(s['control_plane'])) - break -" 2>/dev/null || true)" -CP_JSON="$(mam_control_plane_json "$WORKSPACE" "$ROW_CP" || true)" +# The binding (computed above) is forwarded to the pane only when it differs +# from --workspace's own; a forward with whitespace is refused (spawn path only). CP_PREFIX="" if [ -n "$CP_JSON" ]; then cp_rc=0 diff --git a/tests/test_issue21_phase2_loop_a.py b/tests/test_issue21_phase2_loop_a.py index e2f6cdd..144ee7e 100644 --- a/tests/test_issue21_phase2_loop_a.py +++ b/tests/test_issue21_phase2_loop_a.py @@ -176,3 +176,73 @@ def test_w11_reinstall_replaces_the_old_rule_and_keeps_user_lines(tmp_path): assert "user-one" in gi.splitlines() and "user-two" in gi.splitlines() for ledger in list(tgt.rglob("asset_hashes.txt")) + list(tgt.rglob("install_manifest.txt")): assert ".gitignore" not in ledger.read_text().split(), ledger + + +# --------------------------------------------------------------------------- +# resume: original cwd only (A11), binding failures (A12), already running (A13) +# --------------------------------------------------------------------------- + +def _resume(root, ws, name, agent="claude", env=None): + return _run(root, ("multi-agent-mux-resume", "scripts", "resume_session.sh"), + "--workspace", str(ws), "--agent", agent, "--session", name, env=env) + + +def test_w7_resume_only_at_the_original_cwd(mam_sandbox, mock_herdr, mock_agents): + root = mam_sandbox + here, there, gone = root / "here", root / "there", root / "gone" + here.mkdir() + there.mkdir() + _mutate(root, f"""d['herdr_sessions'] = [ + {{'name': 'x-creator-claude', 'status': 'stopped', 'pane': {{'cwd': {str(here)!r}, 'cmd': 'claude'}}, + 'claude_session_id_own': '11111111-2222-4333-8444-555555555555', 'session_id_source': 'assigned'}}, + {{'name': 'x-creator-agy', 'status': 'stopped', 'pane': {{'cwd': {str(here)!r}, 'cmd': 'agy'}}}}, + {{'name': 'y-creator-claude', 'status': 'stopped', 'pane': {{'cwd': {str(gone)!r}, 'cmd': 'claude'}}}}]""") + since = len(_calls(mock_herdr)) + res = _resume(root, there, "x-creator-claude") + assert res.returncode == 2 and f"--workspace '{here}'" in res.stderr + res = _resume(root, there, "x-creator-agy", agent="agy") + assert res.returncode == 2 + res = _resume(root, gone, "y-creator-claude") + assert res.returncode == 2 and "no longer exists" in res.stderr + assert _spawn_calls(mock_herdr, since) == [], "nothing was spawned in the wrong directory" + + +def test_w8_binding_failure_refused_for_a_worktree_warned_for_the_control_root(mam_sandbox, mock_herdr, mock_agents): + root = mam_sandbox + Path(_skill(root, "lib_py", "control_plane.py")).write_text("raise SystemExit(1)\n") + wt = root.parent / f"{root.name}-wt8" + wt.mkdir() + since = len(_calls(mock_herdr)) + create = ("multi-agent-mux-create", "scripts", "create_session.sh") + res = _run(root, create, "--workspace", str(wt), "--agent", "claude", "--role", "creator", "--no-onboard") + assert res.returncode == 2 and "cannot compute the control-plane binding" in res.stderr + assert _rows(root) == [] and _spawn_calls(mock_herdr, since) == [] + res = _run(root, create, "--workspace", str(root), "--agent", "claude", "--role", "creator", "--no-onboard") + assert res.returncode == 0 and "WARNING: control-plane binding unavailable" in res.stderr, res.stderr + (row,) = _rows(root) + assert "control_plane" not in row + + _mutate(root, f"""d['herdr_sessions'].append({{'name': 'w8-creator-claude', 'status': 'stopped', + 'pane': {{'cwd': {str(wt)!r}, 'cmd': 'claude'}}, + 'claude_session_id_own': '11111111-2222-4333-8444-555555555555', 'session_id_source': 'assigned'}})""") + res = _resume(root, wt, "w8-creator-claude") + assert res.returncode == 2 and "cannot compute the control-plane binding" in res.stderr + + +def test_w9_already_running_resume_persists_the_binding(mam_sandbox, mock_herdr, mock_agents): + root = mam_sandbox + create = ("multi-agent-mux-create", "scripts", "create_session.sh") + res = _run(root, create, "--workspace", str(root), "--agent", "claude", "--role", "creator", "--no-onboard") + assert res.returncode == 0, res.stdout + res.stderr + name = _rows(root)[0]["name"] + _mutate(root, "d['herdr_sessions'][0].pop('control_plane', None)") + res = _resume(root, root, name) + assert res.returncode == 0 and "already running" in res.stdout, res.stdout + res.stderr + row = _rows(root)[0] + assert set(row["control_plane"]) == set(CP_KEYS) + assert row["control_plane"]["workspace_root"] == os.path.realpath(root) + + custom = {**row["control_plane"], "mam_jobs_dir": str(root / "custom-jobs")} + _mutate(root, f"d['herdr_sessions'][0]['control_plane'] = {custom!r}") + assert _resume(root, root, name).returncode == 0 + assert _rows(root)[0]["control_plane"] == custom, "an existing row binding is kept"