fix(issue-12,8,7): keep broker password out of records, keep fresh install stamps, pack sibling workspaces

Issue #12 - MQTT_PASSWORD was copied into every job record, which any
sub-agent can read. BrokerConfig.to_registry_block no longer persists it;
broker_config_from_job layers the password back from the environment
(.mam.env) at connect time, so publish_event still connects from the record.
_atomic_write_record scrubs broker.password from any record it writes, and
`registry.py scrub-secrets [--dry-run]` migrates an existing registry.
auth_token/dispatch_token stay: the agent's own publish_event needs them.

Issue #8 - update.sh restored the pre-update asset_hashes.txt and version.txt
over the installer's fresh ones, so version.txt named the previous commit and
remove.sh backed up every pristine updated file as "locally modified". The
success path now keeps the installer's stamps and only merges install_state
keys the re-install did not write (gitignore_created); the failure path
still restores everything verbatim.

Issue #7 - the shim's new-session took the first same-cwd pane, always found
the full first workspace, judged it overflow and opened another workspace per
agent (4 + 1 + 1). It now walks every same-cwd workspace in pane order and
takes the first with an idle pane or room in its 2xK grid; a workspace is
created only when all are full (4 + 2).

Tests: test_issue7_workspace_packing.py (3; the mock herdr gains an opt-in
per-workspace 2x2 geometry), test_issue8_update_state.py (1, real
install+update cycle), test_issue12_registry_secrets.py (4); registry CLI
snapshot and help capture refreshed for the intentional scrub-secrets
subcommand; registry.md documents the broker block.

Refs #12 #8 #7

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
GodopuandClaude Fable 5.1 committed 2026-09-13 11:23:19 +09:00
1 parent 8a09e022a6
commit e629cc137f
12 files changed
+422 -52

No files matched your search

+44 -9
View File
@@ -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({
+4 -2
View File
@@ -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
+97
View File
@@ -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/<id>.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"
+1
View File
@@ -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
+2
View File
@@ -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")) + \
+102
View File
@@ -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")
+66
View File
@@ -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