From ea911d7842161fec0055f2b496bb7a672587c1e1 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 12:15:26 +0900 Subject: [PATCH] fix(issue-8,7): harden install_state merge and pin sibling-workspace edge cases Round-1 review of 4176d75 (planner-reviewer-claude-01, non-blocking): - update.sh: merge install_state by exact key match (awk, no regex interpretation of the key), skip lines without '=', and terminate a fresh install_state that lacks a final newline before appending - tests (#7): an agent-less pane left behind in a later same-cwd workspace is reused instead of splitting or creating; MAM_WS_LABEL renames only the workspace that was chosen Refs #8 #7 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- deploy/update.sh | 12 ++++++++- tests/test_issue7_workspace_packing.py | 34 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/deploy/update.sh b/deploy/update.sh index 941c41c..48b1836 100644 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -238,10 +238,20 @@ if [ $HAS_MAM -eq 1 ]; then # The failure path (restore_on_failure) still puts all three back verbatim. if [ -f ".mam.update-tmp/install_state" ]; then touch .mam/install_state + # A fresh install_state without a final newline would glue the first + # merged key onto its last line. + if [ -s .mam/install_state ] && [ -n "$(tail -c1 .mam/install_state)" ]; then + printf '\n' >> .mam/install_state + fi while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + *=*) ;; + *) continue ;; # not a key=value line + esac key="${line%%=*}" [ -n "$key" ] || continue - if ! grep -q "^${key}=" .mam/install_state 2>/dev/null; then + # exact key match, no regex interpretation of the key + if ! awk -F= -v k="$key" '$1 == k { found = 1 } END { exit !found }' .mam/install_state; then printf '%s\n' "$line" >> .mam/install_state fi done < .mam.update-tmp/install_state diff --git a/tests/test_issue7_workspace_packing.py b/tests/test_issue7_workspace_packing.py index 088f50c..0c49f50 100644 --- a/tests/test_issue7_workspace_packing.py +++ b/tests/test_issue7_workspace_packing.py @@ -88,6 +88,40 @@ def test_new_workspace_only_when_every_candidate_is_full(mam_sandbox, mock_herdr assert len(_calls(state_file, "workspace", "create")) == 1 +def test_idle_pane_in_a_later_workspace_is_reused(mam_sandbox, mock_herdr, mock_agents): + """w1 is full and w2 keeps an agent-less pane behind (BUG-4 reuse): the + candidate walk must land on that idle pane - no split, no new workspace.""" + state_file = Path(mock_herdr) + _seed(state_file, mam_sandbox, {"w1": 4, "w2": 1}) + state = json.loads(state_file.read_text()) + state.setdefault("panes", []).append({"pane_id": "w2:p_idle", "workspace_id": "w2", "cwd": str(mam_sandbox), "tab_id": "w2:t1"}) + state_file.write_text(json.dumps(state, indent=2)) + + res = _new_session(mam_sandbox, "seventh-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + + assert not _calls(state_file, "pane", "split"), "an idle pane must be reused, not split around" + assert not _calls(state_file, "workspace", "create") + starts = _calls(state_file, "agent", "start") + assert starts and "w2:p_idle" in starts[-1], starts + + +def test_ws_label_rename_targets_only_the_chosen_workspace(mam_sandbox, mock_herdr, mock_agents): + state_file = Path(mock_herdr) + _seed(state_file, mam_sandbox, {"w1": 4, "w2": 1}) + lib = mam_sandbox / ".agents" / "skills" / "lib.sh" + script = f''' + source "{lib}" + _init_herdr_isolation + MAM_WS_LABEL="team-b" herdr new-session -d -s "eighth-creator-claude" -c "{mam_sandbox}" "claude --dangerously-skip-permissions" + ''' + res = subprocess.run(["bash", "-c", script], capture_output=True, text=True, cwd=str(mam_sandbox), timeout=180) + assert res.returncode == 0, res.stderr + res.stdout + + renames = _calls(state_file, "workspace", "rename") + assert renames == [["workspace", "rename", "w2", "team-b"]], renames + + 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."""