diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 9815408..f860596 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -727,33 +727,41 @@ except Exception: print(cmd) " "$final_cmd" "$kind" 2>/dev/null || echo "$final_cmd") - # W1: Resolve existing_ws by querying pane list for matching CWD (since WorkspaceInfo has no cwd key) - existing_ws=$(_real_herdr pane list 2>/dev/null | TARGET_CWD="${ws:-.}" python3 -c " + # W1: Resolve candidate workspaces by querying pane list for matching CWD + # (WorkspaceInfo has no cwd key). ISSUE-7: every workspace on this cwd is + # a candidate, in pane order; the first one with an idle pane or with + # room in its 2xK grid takes the agent, and a fresh workspace is created + # only when all of them are full. Taking just the first match found the + # full workspace every time and re-judged it overflow, so each agent past + # the fourth opened yet another workspace (4 + 1 + 1 instead of 4 + 2). + candidate_wss=$(_real_herdr pane list 2>/dev/null | TARGET_CWD="${ws:-.}" python3 -c " import sys, json, os target_ws = os.environ.get('TARGET_CWD', '') try: target_abs = os.path.realpath(target_ws) if target_ws else '' d = json.loads(sys.stdin.read()) panes = d.get('result', {}).get('panes', []) - matched_ws = '' + seen = [] for p in panes: p_cwd = os.path.realpath(p.get('cwd', '')) if p.get('cwd') else '' - if target_abs and p_cwd == target_abs and p.get('workspace_id'): - matched_ws = p.get('workspace_id') - break - print(matched_ws) + wid = p.get('workspace_id') + if target_abs and p_cwd == target_abs and wid and wid not in seen: + seen.append(wid) + print('\n'.join(seen)) except Exception: pass " 2>/dev/null || echo "") ws_id="" target_pane="" - if [ -n "$existing_ws" ]; then + existing_ws="" + for cand_ws in $candidate_wss; do + [ -n "$cand_ws" ] || continue # BUG-4: prefer an already-idle (agent-less) pane in the existing # workspace over splitting a new one — avoids unbounded pane # proliferation and 2x2 grid-layout churn when a prior agent's pane # was left behind idle. - idle_pane=$(_real_herdr pane list 2>/dev/null | TARGET_WS="$existing_ws" python3 -c " + idle_pane=$(_real_herdr pane list 2>/dev/null | TARGET_WS="$cand_ws" python3 -c " import sys, json, os target_ws = os.environ.get('TARGET_WS', '') try: @@ -768,15 +776,18 @@ except Exception: " 2>/dev/null || echo "") if [ -n "$idle_pane" ]; then + existing_ws="$cand_ws" target_pane="$idle_pane" if [ -n "$env_flags" ] && [ -n "$final_cmd" ]; then idle_env="${env_flags//--env /}" idle_env="${idle_env:+$idle_env }HERDR_WORKSPACE_ID=$existing_ws" final_cmd="env $idle_env $final_cmd" fi - else + break + fi + # W2a: Determine split direction policy via pane layout - sample_pane=$(_real_herdr pane list 2>/dev/null | TARGET_WS="$existing_ws" python3 -c " + sample_pane=$(_real_herdr pane list 2>/dev/null | TARGET_WS="$cand_ws" python3 -c " import sys, json, os target_ws = os.environ.get('TARGET_WS', '') try: @@ -798,9 +809,15 @@ except Exception: sample_pane="${split_target:-$sample_pane}" fi - if [ "$split_dir" = "right" ] || [ "$split_dir" = "down" ]; then - split_json=$(_real_herdr pane split --pane "$sample_pane" --direction "$split_dir" --cwd "${ws:-.}" $env_flags --env HERDR_WORKSPACE_ID="$existing_ws" --no-focus 2>/dev/null || echo "") - target_pane=$(echo "$split_json" | python3 -c " + if [ "$split_dir" = "overflow" ]; then + # W2b: this workspace's grid is full — try the next one on the same cwd + # (ISSUE-7); a fresh workspace is created below only if none has room. + continue + fi + existing_ws="$cand_ws" + [ "$split_dir" = "down" ] || split_dir="right" + split_json=$(_real_herdr pane split --pane "$sample_pane" --direction "$split_dir" --cwd "${ws:-.}" $env_flags --env HERDR_WORKSPACE_ID="$existing_ws" --no-focus 2>/dev/null || echo "") + target_pane=$(echo "$split_json" | python3 -c " import sys, json try: d = json.loads(sys.stdin.read()) @@ -809,23 +826,8 @@ try: except Exception: pass " 2>/dev/null || echo "") - elif [ "$split_dir" = "overflow" ]; then - # W2b: Overflow threshold reached — force create fresh workspace - existing_ws="" - else - split_json=$(_real_herdr pane split --pane "$sample_pane" --direction right --cwd "${ws:-.}" $env_flags --env HERDR_WORKSPACE_ID="$existing_ws" --no-focus 2>/dev/null || echo "") - target_pane=$(echo "$split_json" | python3 -c " -import sys, json -try: - d = json.loads(sys.stdin.read()) - res = d.get('result', {}) - print(res.get('pane', {}).get('pane_id') or res.get('pane_id', '')) -except Exception: - pass -" 2>/dev/null || echo "") - fi - fi - fi + break + done if [ -z "$existing_ws" ] || [ -z "$target_pane" ]; then ws_json=$(_real_herdr workspace create --cwd "${ws:-.}" ${MAM_WS_LABEL:+--label "$MAM_WS_LABEL"} $env_flags --no-focus 2>/dev/null || echo "") diff --git a/.agents/skills/lib_py/delegation/mqtt_common.py b/.agents/skills/lib_py/delegation/mqtt_common.py index 533a1d5..a413779 100644 --- a/.agents/skills/lib_py/delegation/mqtt_common.py +++ b/.agents/skills/lib_py/delegation/mqtt_common.py @@ -227,13 +227,19 @@ class BrokerConfig: return asdict(self) def to_registry_block(self) -> Dict[str, Any]: - """The subset that gets persisted into a job record's broker block.""" + """The subset that gets persisted into a job record's broker block. + + Never the password (Issue #12): every sub-agent can read the job + record, so a broker secret written there ends up in agent transcripts. + ``broker_config_from_job`` layers the block over the environment, so + the password is taken from ``MQTT_PASSWORD`` (``.mam.env``) at connect + time instead. + """ return { "host": self.host, "port": self.port, "tls": self.tls, "username": self.username, - "password": self.password, } @@ -393,12 +399,23 @@ def load_job(job_id: str, registry_dir: str = DEFAULT_REGISTRY_DIR) -> Dict[str, return json.load(fh) +def _scrub_record_secrets(record: Dict[str, Any]) -> None: + """Issue #12: a broker password never belongs in a job record (see + ``BrokerConfig.to_registry_block``). Records written before that rule are + cleaned the next time anything writes them; ``registry.py scrub-secrets`` + does it for a whole registry at once.""" + broker = record.get("broker") + if isinstance(broker, dict): + broker.pop("password", None) + + def _atomic_write_record(job_id: str, registry_dir: str, record: Dict[str, Any]) -> None: """Write a record atomically: temp file in the same dir + os.replace. The rename is atomic on POSIX, so readers never observe a half-written file. Callers MUST already hold ``registry_lock`` for read-modify-write correctness.""" + _scrub_record_secrets(record) Path(registry_dir).mkdir(parents=True, exist_ok=True) path = _job_path(job_id, registry_dir) fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=f".{job_id}.", suffix=".tmp") diff --git a/.agents/skills/lib_py/delegation/registry.py b/.agents/skills/lib_py/delegation/registry.py index d73bd15..a8c2706 100755 --- a/.agents/skills/lib_py/delegation/registry.py +++ b/.agents/skills/lib_py/delegation/registry.py @@ -333,9 +333,35 @@ def _build_parser() -> argparse.ArgumentParser: p_logs.add_argument("--json", action="store_true", help="emit raw JSON lines / records instead of a table") + p_scrub = sub.add_parser( + "scrub-secrets", + help="remove broker.password from every job record (Issue #12 migration)", + ) + p_scrub.add_argument("--dry-run", action="store_true", + help="only list the records that still carry a password") + return parser +def _cmd_scrub_secrets(args) -> int: + """Issue #12: strip broker.password from records written before + to_registry_block stopped persisting it. Every other field is kept.""" + rd = args.registry_dir + found = [] + with registry_lock(rd): + for record in list(_iter_records(rd)): + broker = record.get("broker") + if not (isinstance(broker, dict) and "password" in broker): + continue + found.append(record.get("job_id", "?")) + if not args.dry_run: + broker.pop("password", None) + _atomic_write_record(record["job_id"], rd, record) + verb = "would scrub" if args.dry_run else "scrubbed" + print(f"{verb} {len(found)} record(s) under {rd}" + (": " + ", ".join(found) if found else "")) + return 0 + + def _operator_write(job_id: str, registry_dir: str, **fields) -> int: """Apply an operator-authority write and report a refusal on the exit status. @@ -506,6 +532,9 @@ def main(argv: Optional[List[str]] = None) -> int: if args.command == "logs": return _cmd_logs(args) + if args.command == "scrub-secrets": + return _cmd_scrub_secrets(args) + return 1 diff --git a/.agents/skills/multi-agent-mux-delegate-job/registry.md b/.agents/skills/multi-agent-mux-delegate-job/registry.md index cd902df..cbdafef 100644 --- a/.agents/skills/multi-agent-mux-delegate-job/registry.md +++ b/.agents/skills/multi-agent-mux-delegate-job/registry.md @@ -42,8 +42,7 @@ Reference implementation: [`./scripts/registry.py`](./scripts/registry.py) "host": "broker.hivemq.com", "port": 1883, "tls": false, - "username": null, - "password": null + "username": null }, "topic_prefix": "python/mqtt/jobs/abc12345", "timeout_sec": 3600, @@ -55,7 +54,13 @@ Reference implementation: [`./scripts/registry.py`](./scripts/registry.py) ``` - `broker` lets `publish_event.py` connect from the record alone (env still - overrides toggles like `MQTT_TLS`). + overrides toggles like `MQTT_TLS`). It never carries the password (Issue #12): + every sub-agent can read the record, so `MQTT_PASSWORD` is layered back from + the environment (`.mam.env`) at connect time. Records written before that rule + are cleaned on their next write; `registry.py scrub-secrets [--dry-run]` cleans + a whole registry at once. `auth_token` and `dispatch_token` stay in the record + because the agent's own `publish_event` needs them to sign and to pin its + attempt. - `topic_prefix` → the events topic is `/events`. - `last_seq` backs the monotonic `seq` counter so it survives process restarts. - `expected_artifacts` is the hook a user `validate.sh` checks (existence/content). diff --git a/deploy/update.sh b/deploy/update.sh index 645da06..941c41c 100644 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -229,11 +229,23 @@ if [ $HAS_MAM -eq 1 ]; then mkdir -p .mam/delegate_job_logs cp -rf .mam.update-tmp/delegate_job_logs/* .mam/delegate_job_logs/ fi - for st in install_state asset_hashes.txt version.txt; do - if [ -f ".mam.update-tmp/$st" ]; then - cp -f ".mam.update-tmp/$st" .mam/ - fi - done + # Issue #8: asset_hashes.txt and version.txt describe the tree the + # installer just wrote, so the pre-update copies must NOT come back here - + # restoring them made version.txt name the previous commit and remove.sh + # treat every updated file as locally modified (spurious backups). Only + # install_state carries history worth keeping: merge in any key the fresh + # install did not write (e.g. gitignore_created from the original install). + # The failure path (restore_on_failure) still puts all three back verbatim. + if [ -f ".mam.update-tmp/install_state" ]; then + touch .mam/install_state + while IFS= read -r line || [ -n "$line" ]; do + key="${line%%=*}" + [ -n "$key" ] || continue + if ! grep -q "^${key}=" .mam/install_state 2>/dev/null; then + printf '%s\n' "$line" >> .mam/install_state + fi + done < .mam.update-tmp/install_state + fi if [ -d ".mam.update-tmp/skill-backups" ]; then mkdir -p .mam/skill-backups cp -rf .mam.update-tmp/skill-backups/* .mam/skill-backups/ 2>/dev/null || true diff --git a/tests/conftest.py b/tests/conftest.py index 5164733..93f144d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -334,15 +334,50 @@ elif cmd1 == "pane": focused = "w1:p1" panes_list = [] agents = state.get("agents", {}) - count = max(len(agents), 1) - w_per_pane = 184 // count - idx = 0 - for name, data in agents.items(): - panes_list.append({ - "pane_id": data.get("pane_id", f"w1:p{idx+1}"), - "rect": {"width": w_per_pane, "height": 78} - }) - idx += 1 + if state.get("layout_geometry") == "2x2": + # Issue #7 tests: real per-workspace geometry. Only the panes of + # the sampled pane's workspace are reported, laid out as the 2x2 + # grid herdr builds, so lib_py.layout judges a full workspace as + # overflow while a sibling workspace with room is offered a split. + sample = "" + i = 2 + while i < len(args): + if args[i] == "--pane" and i + 1 < len(args): + sample = args[i+1] + i += 2 + else: + i += 1 + ws_of = None + for data in agents.values(): + if data.get("pane_id") == sample: + ws_of = data.get("workspace_id") + break + if ws_of is None and ":" in sample: + ws_of = sample.split(":")[0] + members = [d for d in agents.values() if ws_of is None or d.get("workspace_id", "w1") == ws_of] + n = len(members) + for idx, data in enumerate(members): + if n == 1: + rect = {"x": 0, "y": 0, "width": 184, "height": 78} + elif n == 2: + rect = {"x": 92 * idx, "y": 0, "width": 92, "height": 78} + elif n == 3: + rect = [{"x": 0, "y": 0, "width": 92, "height": 39}, + {"x": 0, "y": 39, "width": 92, "height": 39}, + {"x": 92, "y": 0, "width": 92, "height": 78}][idx] + else: + rect = {"x": 92 * (idx % 2), "y": 39 * (idx // 2), "width": 92, "height": 39} + panes_list.append({"pane_id": data.get("pane_id", f"w1:p{idx+1}"), "rect": rect}) + else: + count = max(len(agents), 1) + w_per_pane = 184 // count + idx = 0 + for name, data in agents.items(): + panes_list.append({ + "pane_id": data.get("pane_id", f"w1:p{idx+1}"), + "rect": {"width": w_per_pane, "height": 78} + }) + idx += 1 if not panes_list: panes_list = [{"pane_id": "w1:p1", "rect": {"width": 184, "height": 78}}] print(json.dumps({ diff --git a/tests/fixtures/issue_5/phase1/help-registry.txt b/tests/fixtures/issue_5/phase1/help-registry.txt index 3d63ce0..6a660e8 100644 --- a/tests/fixtures/issue_5/phase1/help-registry.txt +++ b/tests/fixtures/issue_5/phase1/help-registry.txt @@ -1,11 +1,11 @@ usage: registry.py [-h] [--registry-dir REGISTRY_DIR] - {register,list,get,status,update,close,get-feedback,pick,logs} + {register,list,get,status,update,close,get-feedback,pick,logs,scrub-secrets} ... multi-agent-mux-delegate-job registry CLI positional arguments: - {register,list,get,status,update,close,get-feedback,pick,logs} + {register,list,get,status,update,close,get-feedback,pick,logs,scrub-secrets} register create a pending job; prints the job id list list jobs (optionally by status) get print one job record as JSON @@ -17,6 +17,8 @@ positional arguments: pick claim a pending job for a session; prints id logs show the persistent audit log for a job, or --list every logged job + scrub-secrets remove broker.password from every job record (Issue + #12 migration) optional arguments: -h, --help show this help message and exit diff --git a/tests/test_issue12_registry_secrets.py b/tests/test_issue12_registry_secrets.py new file mode 100644 index 0000000..7bb5cdc --- /dev/null +++ b/tests/test_issue12_registry_secrets.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +"""Issue #12 — the broker password must not be persisted in job records. + +Every sub-agent can read `.mam/jobs/.json`, so `MQTT_PASSWORD` copied +there ends up in agent transcripts. The broker block now carries only +host/port/tls/username; the password is layered back from the environment +(`.mam.env`) at connect time. Records written before this rule are cleaned on +their next write and by `registry.py scrub-secrets`. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS = REPO_ROOT / ".agents" / "skills" +REGISTRY_PY = SKILLS / "multi-agent-mux-delegate-job" / "scripts" / "registry.py" + +sys.path.insert(0, str(SKILLS)) +from lib_py.delegation import mqtt_common, registry # noqa: E402 + + +def _secure_env(monkeypatch): + monkeypatch.setenv("MQTT_BROKER", "broker.example.test") + monkeypatch.setenv("MQTT_USERNAME", "mam-user") + monkeypatch.setenv("MQTT_PASSWORD", "s3cret-broker-pass") + + +def test_registry_block_carries_no_password(monkeypatch): + _secure_env(monkeypatch) + block = mqtt_common.broker_config_from_env().to_registry_block() + assert "password" not in block + assert block["username"] == "mam-user" and block["host"] == "broker.example.test" + + +def test_registered_record_has_no_password_but_connects_with_it(mam_sandbox, monkeypatch): + _secure_env(monkeypatch) + rd = str(mam_sandbox / ".mam" / "jobs") + job_id = registry.register_job("issue 12", "claude-code", "herdr:claude", registry_dir=rd) + + record = json.loads((Path(rd) / f"{job_id}.json").read_text()) + assert "password" not in record["broker"], record["broker"] + assert "s3cret-broker-pass" not in json.dumps(record) + # The secure broker still gets its per-job HMAC key (needed by the agent's + # own publish_event to sign; documented as remaining in the record). + assert record["auth_token"] + + cfg = mqtt_common.broker_config_from_job(record) + assert cfg.password == "s3cret-broker-pass", "password must come back from the environment" + assert cfg.username == "mam-user" + + +def test_legacy_record_is_scrubbed_on_next_write(mam_sandbox, monkeypatch): + _secure_env(monkeypatch) + rd = str(mam_sandbox / ".mam" / "jobs") + job_id = registry.register_job("legacy", "claude-code", "herdr:claude", registry_dir=rd) + path = Path(rd) / f"{job_id}.json" + record = json.loads(path.read_text()) + record["broker"]["password"] = "leaked-earlier" + path.write_text(json.dumps(record)) + + registry.update_status(job_id, rd, "running") + + after = json.loads(path.read_text()) + assert after["status"] == "running" + assert "password" not in after["broker"] + + +def test_scrub_secrets_cli_migrates_a_registry(mam_sandbox, monkeypatch): + _secure_env(monkeypatch) + rd = str(mam_sandbox / ".mam" / "jobs") + dirty = registry.register_job("dirty", "claude-code", "herdr:claude", registry_dir=rd) + clean = registry.register_job("clean", "claude-code", "herdr:claude", registry_dir=rd) + dirty_path = Path(rd) / f"{dirty}.json" + rec = json.loads(dirty_path.read_text()) + rec["broker"]["password"] = "leaked-earlier" + dirty_path.write_text(json.dumps(rec)) + clean_before = (Path(rd) / f"{clean}.json").read_text() + + env = dict(os.environ, PYTHONPATH=f"{SKILLS}:{os.environ.get('PYTHONPATH', '')}") + dry = subprocess.run([sys.executable, str(REGISTRY_PY), "--registry-dir", rd, "scrub-secrets", "--dry-run"], + capture_output=True, text=True, env=env, cwd=str(mam_sandbox)) + assert dry.returncode == 0, dry.stderr + assert f"would scrub 1 record(s)" in dry.stdout and dirty in dry.stdout + assert "leaked-earlier" in dirty_path.read_text(), "--dry-run must not write" + + res = subprocess.run([sys.executable, str(REGISTRY_PY), "--registry-dir", rd, "scrub-secrets"], + capture_output=True, text=True, env=env, cwd=str(mam_sandbox)) + assert res.returncode == 0, res.stderr + assert "scrubbed 1 record(s)" in res.stdout + scrubbed = json.loads(dirty_path.read_text()) + assert "password" not in scrubbed["broker"] + assert scrubbed["prompt"] == "dirty" and scrubbed["auth_token"] == rec["auth_token"] + assert (Path(rd) / f"{clean}.json").read_text() == clean_before, "untouched records are not rewritten" diff --git a/tests/test_issue5_phase1_delegation.py b/tests/test_issue5_phase1_delegation.py index 32c851a..dd08c8e 100644 --- a/tests/test_issue5_phase1_delegation.py +++ b/tests/test_issue5_phase1_delegation.py @@ -330,6 +330,7 @@ def test_registry_cli_contract_snapshot(mam_sandbox): commands = set(subparsers[0].choices) assert commands == { "register", "list", "get", "status", "update", "close", "get-feedback", "pick", "logs", + "scrub-secrets", # Issue #12 migration }, commands reg = _parser_snapshot(subparsers[0].choices["register"]) assert reg["--prompt"][2] is None diff --git a/tests/test_issue5_phase2a_loop.py b/tests/test_issue5_phase2a_loop.py index 3149f61..1837849 100644 --- a/tests/test_issue5_phase2a_loop.py +++ b/tests/test_issue5_phase2a_loop.py @@ -339,6 +339,8 @@ def test_g6_every_record_writer_is_the_engine_or_a_declared_exception(): ("mqtt_common.py", "_update_job_status_legacy"), ("registry.py", "_pick_pending_legacy"), ("engine.py", "transition"), + # Issue #12 migration: drops broker.password only, never status + ("registry.py", "_cmd_scrub_secrets"), } found = set() for path in list((SKILLS / "lib_py" / "delegation").glob("*.py")) + \ diff --git a/tests/test_issue7_workspace_packing.py b/tests/test_issue7_workspace_packing.py new file mode 100644 index 0000000..088f50c --- /dev/null +++ b/tests/test_issue7_workspace_packing.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +"""Issue #7 — a new agent must be packed into any same-cwd workspace that still +has room in its 2xK grid; a fresh workspace is created only when all of them +are full. + +The shim's `new-session` used to take the first pane whose cwd matched, which +was always the (full) first workspace, judged it overflow and opened yet +another workspace: six agents came out as 4 + 1 + 1 instead of 4 + 2. + +The mock herdr reports real per-workspace geometry when the state carries +``layout_geometry: "2x2"``, so lib_py.layout sees a full workspace as +overflow and a half-empty sibling as splittable. +""" +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +CWD_AGENTS = { + # workspace w1: full 2x2 grid + "a1": ("w1", "w1:p1"), "a2": ("w1", "w1:p2"), "a3": ("w1", "w1:p3"), "a4": ("w1", "w1:p4"), +} + + +def _seed(state_file: Path, sandbox: Path, layout: dict) -> None: + """layout: workspace id -> number of agents already in it (same cwd).""" + state = json.loads(state_file.read_text()) + state["layout_geometry"] = "2x2" + agents = {} + workspaces = [] + for ws_id, count in layout.items(): + workspaces.append({"workspace_id": ws_id, "label": "default", "cwd": str(sandbox)}) + for i in range(1, count + 1): + agents[f"{ws_id}-agent-{i}"] = { + "agent": "claude", + "agent_status": "idle", + "status": "running", + "workspace_id": ws_id, + "pane_id": f"{ws_id}:p{i}", + "cwd": str(sandbox), + "buffer": "Ready", + } + state["workspaces"] = workspaces + state["agents"] = agents + state_file.write_text(json.dumps(state, indent=2)) + + +def _new_session(sandbox: Path, name: str) -> subprocess.CompletedProcess: + lib = sandbox / ".agents" / "skills" / "lib.sh" + script = f''' + source "{lib}" + _init_herdr_isolation + herdr new-session -d -s "{name}" -c "{sandbox}" "claude --dangerously-skip-permissions" + ''' + return subprocess.run(["bash", "-c", script], capture_output=True, text=True, cwd=str(sandbox), timeout=180) + + +def _calls(state_file: Path, *prefix: str) -> list: + calls = json.loads(state_file.read_text()).get("calls", []) + return [c for c in calls if c[:len(prefix)] == list(prefix)] + + +def test_agent_packs_into_sibling_workspace_with_room(mam_sandbox, mock_herdr, mock_agents): + """w1 holds 4 agents (full), w2 holds 1: the sixth agent must split w2.""" + state_file = Path(mock_herdr) + _seed(state_file, mam_sandbox, {"w1": 4, "w2": 1}) + + res = _new_session(mam_sandbox, "sixth-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + + splits = _calls(state_file, "pane", "split") + assert splits, "the sixth agent must be placed by splitting a pane" + assert all("HERDR_WORKSPACE_ID=w2" in c and "--pane" in c and c[c.index("--pane") + 1].startswith("w2:") + for c in splits), splits + assert not _calls(state_file, "workspace", "create"), "no third workspace while w2 has room" + + +def test_new_workspace_only_when_every_candidate_is_full(mam_sandbox, mock_herdr, mock_agents): + """w1 and w2 both hold 4 agents: only then is a third workspace created.""" + state_file = Path(mock_herdr) + _seed(state_file, mam_sandbox, {"w1": 4, "w2": 4}) + + res = _new_session(mam_sandbox, "ninth-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + + assert not _calls(state_file, "pane", "split"), "a full grid must not be split" + assert len(_calls(state_file, "workspace", "create")) == 1 + + +def test_first_workspace_with_room_still_wins(mam_sandbox, mock_herdr, mock_agents): + """Regression guard: with a single half-empty workspace the agent lands + there, exactly as before.""" + state_file = Path(mock_herdr) + _seed(state_file, mam_sandbox, {"w1": 2}) + + res = _new_session(mam_sandbox, "third-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + + splits = _calls(state_file, "pane", "split") + assert len(splits) == 1 and "HERDR_WORKSPACE_ID=w1" in splits[0], splits + assert not _calls(state_file, "workspace", "create") diff --git a/tests/test_issue8_update_state.py b/tests/test_issue8_update_state.py new file mode 100644 index 0000000..e572e01 --- /dev/null +++ b/tests/test_issue8_update_state.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Issue #8 — update.sh must keep the installer's fresh version.txt and +asset_hashes.txt instead of restoring the pre-update copies. + +Restoring them left version.txt naming the previous commit and made remove.sh +compare against stale hashes, so every updated-but-pristine file was backed up +as "locally modified". install_state is the one file whose old keys are +merged back (gitignore_created from the original install). +""" +from __future__ import annotations + +import os +import shutil +import subprocess +import tempfile +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent + + +@pytest.fixture +def installed_workspace(): + temp = tempfile.mkdtemp(prefix="mam_issue8_") + ws = Path(temp) / "workspace" + ws.mkdir() + env = os.environ.copy() + env["MAM_REPO_URL"] = str(REPO_ROOT) + env["MAM_SKIP_VENV"] = "1" + env["MAM_INSTALLER_URL"] = "file://" + str(REPO_ROOT / "deploy" / "install.sh") + res = subprocess.run(["bash", str(REPO_ROOT / "deploy" / "install.sh"), str(ws)], + env=env, capture_output=True, text=True, timeout=600) + assert res.returncode == 0, res.stderr + try: + yield ws, env + finally: + shutil.rmtree(temp, ignore_errors=True) + + +def test_update_keeps_fresh_version_and_hashes_and_merges_install_state(installed_workspace): + ws, env = installed_workspace + mam = ws / ".mam" + fresh_version = (mam / "version.txt").read_text() + assert "commit=" in fresh_version + + # Simulate the previous install: stale stamps and an install_state key the + # re-install will not write again. + (mam / "version.txt").write_text("source=old\ncommit=stale0000\nfetched_at=then\nmethod=git\n") + (mam / "asset_hashes.txt").write_text("stale-hash-db\n") + with open(mam / "install_state", "a") as fh: + fh.write("custom_marker=kept\n") + + res = subprocess.run(["bash", str(ws / ".mam_deploy" / "update.sh"), "--force"], + cwd=str(ws), env=env, capture_output=True, text=True, timeout=600) + assert res.returncode == 0, res.stderr + res.stdout + + version = (mam / "version.txt").read_text() + assert "stale0000" not in version, version + assert "commit=" in version and version.strip() != "" + hashes = (mam / "asset_hashes.txt").read_text() + assert "stale-hash-db" not in hashes and hashes.strip(), "asset_hashes.txt must be the installer's fresh ledger" + + state = (mam / "install_state").read_text() + assert "custom_marker=kept" in state, state + assert state.count("gitignore_created=1") == 1, "merged keys must not be duplicated:\n" + state