multi-agent-mux/.agents/reports/planner-reviewer-claude-01/plan-c38ddfcf.md

🛠️ Implementation Plan: Agent-Creation Reliability Fixes (Job c38ddfcf)

• Author: planner-reviewer-claude-01 (Planner)
• Source: mam-agent-creation-fix-report.md (untracked, root of repo) — a field report describing 4 issues observed while creating reviewer-opencode-01 and other sessions. At the time this plan was written, none of the 4 fixes it describes were actually present in the code (verified directly: .agents/skills/lib_py/agents/adapters/opencode.py's ready_tokens was still 'OpenCode|Chat', and none of the lib.sh diffs it shows existed on disk) — the report documents intended fixes, not landed ones.
• Branch: fix/agent-creation-issues
• Status: Plan executed in this same job — code changed, tests updated/passing, version bumped (see §5).

---

1. Problem 1 — OpenCode TUI ready-token mismatch

• File: .agents/skills/lib_py/agents/adapters/opencode.py
• Root cause: ready_tokens was 'OpenCode|Chat', which never appears in OpenCode's actual TUI (the banner renders as an ASCII block, not text matching those words). wait_for_tui_ready() (.agents/skills/lib.sh:1830) therefore ran the full 30s timeout loop before declaring readiness (or failing outright), even though the process was healthy.
• Fix: extend ready_tokens to include real, observed TUI surface text: 'OpenCode|Chat|Ask anything|tab agents|ctrl\+p|Build auto|commands'. The \+ escaping is required because wait_for_tui_ready matches this string with grep -E (confirmed at lib.sh:1901); an unescaped + would be interpreted as an ERE quantifier.
• Why safe: OpenCodeAgentAdapter does not override strong_ready_tokens/weak_ready_tokens, so the base class's default (strong = ready_tokens, weak = '') still applies — the 2-tier S ∨ (W ∧ C) decision in wait_for_tui_ready degrades to plain S, matching every other single-tier adapter (agy, hermes, grok).
• Test impact: tests/test_a4_adapter_contract.py::test_adapter_required_properties hardcodes the exact ready_tokens string per agent — updated in lockstep. tests/test_c1_tui_readiness.py::test_adapter_strong_weak_partition is schema-agnostic (splits on | and checks set algebra) — unaffected.

2. Problem 2 — Missing PYTHONPATH in the herdr shim

• File: .agents/skills/lib.sh (the <<'EOF' heredoc that _init_herdr_isolation() writes to $WORKSPACE_ROOT/.mam/shim/herdr)
• Root cause: the shim script (once written to disk) directly calls python3 -m lib_py.layout (line ~682 in the heredoc) and imports from lib_py.agents.sanitize import ... (line ~528). These only resolve if PYTHONPATH includes .agents/skills. lib.sh itself exports PYTHONPATH when sourced (line 25), but the shim is a separate executable invoked directly via $PATH by herdr/other processes — it does not always inherit that export (e.g. a freshly spawned pane's shell never sourced lib.sh).
• Fix: added a self-locating PYTHONPATH export at the very top of the heredoc, right after set -euo pipefail:

  _SHIM_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
  _SKILL_DIR="$(cd "$_SHIM_DIR/../../.agents/skills" 2>/dev/null && pwd || true)"
  [ -n "$_SKILL_DIR" ] && export PYTHONPATH="$_SKILL_DIR:${PYTHONPATH:-}"
  

  Because the heredoc is single-quoted (<<'EOF'), ${BASH_SOURCE[0]} is not expanded at write time — it's copied verbatim and resolves at the shim's own runtime, correctly pointing at wherever .mam/shim/herdr actually lives ($WORKSPACE_ROOT/.mam/shim/../../.agents/skills = $WORKSPACE_ROOT/.agents/skills).
• Test impact: tests/test_herdr_shim_contract.py::test_h19_single_resolver_helper_used_by_all_branches already asserts bash -n on both the generated shim and lib.sh — passes. No test previously asserted PYTHONPATH presence in the shim; none needed updating.

3. Problem 3 — herdr agent start timeout treated as unconditional failure

• File: .agents/skills/lib.sh, the agent) start) case arm (~line 756–end of the W5/W6 backoff loop)
• Root cause: on a slow TUI render, herdr can return "timed out waiting for agent startup". The existing loop deliberately does not treat that string as success (see the pre-existing comment: "Do NOT treat 'timed out waiting for agent startup' as success: herdr returns that for a dead process too") — a correct design constraint I preserved rather than overrode, because herdr genuinely returns identical text for a live-but-slow process and a dead one.
• Fix: rather than trusting the ambiguous text, added a post-loop branch that re-queries herdr directly for ground truth:
  1. If _herdr_agent_get_scoped "$agent_name" now resolves a pane_id (i.e. herdr's own registry shows the agent alive), treat as success.
  2. Otherwise, fall back to injecting the launch command directly into the pane (pane send-text + pane send-keys ... Enter, mirroring the existing paste-buffer pattern at lib.sh:1040), wait 1s, and re-check liveness once more.
  3. Only if still unresolved does the existing exit 1 failure path trigger.
• Why this doesn't reintroduce the bug the comment warns about: the original concern was blindly promoting the timeout string to success. This fix never inspects the string as a truth signal for liveness — it always asks herdr's live registry, which is authoritative for both the dead-process and slow-render cases.
• Test impact: tests/test_b19_headless_reconcile_fixes.py::test_agent_start_success_tokens_exclude_startup_timeout (D-1) asserts the original success-token grep line still excludes the timeout string — untouched, still passes, since the new logic is a separate branch appended after the loop, not a change to that line.

4. Problem 4 — _resolve_herdr_pane_id lacks a pane_id fast-path and mismatches on workspace label

• File: .agents/skills/lib.sh, _resolve_herdr_pane_id() (inside the shim heredoc)
• Root cause (fast-path): every call went through agent get + pane list resolution even when the caller already had a real pane_id (wN:pM) in hand — wasted round-trips, and a pointless second point of failure.
• Root cause (label matching): the pane-list-based workspace filter compared target_ws directly against each pane's workspace_id. If a caller passed a workspace label (e.g. $MAM_WS_LABEL, such as "mam-agents") instead of the raw id, the filter always failed to match, since panes never carry a label field — only herdr workspace list returns {workspace_id, label} pairs (confirmed against tests/conftest.py's mock herdr, which is the closest ground truth available for herdr's real JSON schema in this repo).
• Fix:
  1. Added a fast-path at the top of the function: if $target already matches ^w[A-Za-z0-9]+:p[A-Za-z0-9]+$, return it immediately.
  2. Added a target_ws normalization step: call herdr workspace list, and if target_ws isn't already a live workspace_id, look it up as a workspace label and substitute the resolved id. Falls back permissively to the original value on any lookup failure or no-match (preserves all prior behavior when target_ws is already a raw id, which is the common case).
• Why the fallback is safe under set -e: mirrors the exact pattern already used by _herdr_agent_get_scoped a few lines above it — the inner Python explicitly sys.exit(1) (printing nothing) on any non-match/exception, and the outer || echo "$target_ws" supplies the single fallback line. (An earlier draft of this fix had the Python script itself print-then-succeed in the fallback case, which double-emitted the value once pipefail propagated the upstream _real_herdr failure — corrected before landing, verified via test_resolve_pane_id_fails_cleanly_under_set_e.)
• Test impact: tests/test_b19_headless_reconcile_fixes.py::test_no_substring_matching_remains_in_lib_sh (D-6) asserts the pane_id regex literal exists inside the function body — still true (now appears twice: once in the new fast-path, once in the pre-existing final validation). tests/test_b19_headless_reconcile_fixes.py::test_resolve_pane_id_fails_cleanly_under_set_e (D-4) and tests/test_herdr_shim_contract.py::test_h20_pane_id_regex_accepts_alphanumeric_workspace (H-20) both exercise this function with an all-failing mocked _real_herdr — both still pass, confirming the new lookup degrades gracefully rather than aborting the shim under set -euo pipefail.

5. Verification & Versioning

• Targeted tests (test_a4_adapter_contract.py, test_c1_tui_readiness.py, test_b19_headless_reconcile_fixes.py, test_herdr_shim_contract.py, test_version_consistency.py): 78 passed.
• Full suite (pytest tests/ -q): run and confirmed green before the version bump landed (see job report for the exact count/duration).
• SemVer classification: PATCH (v4.1.0 → v4.1.1). None of the 4 fixes add or change any public CLI flag, YAML schema key, or agent-facing contract — they are internal reliability/robustness fixes to code already shipped in v4.1.0 (TUI-detection tuning, shim environment robustness, a startup-race fallback, and a pane-resolution robustness improvement). This matches the v3.0.1 precedent ("Herdr Shim Routing Contract Refactor, Multi-Workspace Isolation") — the closest prior release of the same shape.
• 3-way lockstep bump applied: lib.sh:MAM_VERSION, VERSIONS.md (header + line-24 prose + 8-row matrix + new changelog section), 8× SKILL.md frontmatters.