multi-agent-mux/.agents/reports/canary-projects-multi-agent-mux-creator-cline/report-c16bed83.md

# Cross-Code Review — Job c16bed83 (revised bug-fix changeset)

- **Job ID**: c16bed83
- **Reviewer**: cline
- **Date**: 2026-08-23
- **Subject**: Revised changeset fixing Bugs 2, 3, 4 of the 5 reported multi-agent-mux bugs; cross-review of lint, behavior, and loss.
- **Prior review**: Job `c197a005` reviewed the initial Bug 4 implementation and returned a NOT-PASS verdict due to a **duplicate-input regression** (agent-prompt success + evidence-grep failure fell through to paste-buffer, re-sending the text) plus an overly-broad evidence pattern (`[A-Za-z]+ing`) and a test-coverage gap. This changeset is the revised implementation intended to address those findings.

## §1. Changeset Summary

Working tree (`git status`): `lib.sh` + `reconcile.sh` modified; `tests/test_bug_fixes_565255de.py` added (untracked). HEAD `f133e52`.

Diff stat: `lib.sh` 20 +/- (−10/+10 net structure), `reconcile.sh` 9 +. Compared to the prior revision (`c197a005`), `lib.sh` shrank (the evidence-grep + `_pane_capture` block was removed), confirming the Bug 4 simplification; `reconcile.sh` is byte-identical to the prior-approved version (`171086f..9007030`).

Bug disposition vs. the original 5-bug brief:
- **Bug 1** (top-level `lib.sh` sourcing before arg parse → daemon/socket collision): NOT addressed — correctly, per prior cross-review (`b4e8eaee`) which assessed Bug 1 as refuted (the top-level guard prevents the collision).
- **Bug 2** (headless 0×0 pane → forced `overflow` workspace): FIXED.
- **Bug 3** (`reconcile.sh` missing `SKILLS_DIR` in `env_python`/`atomic_dump_yaml`): FIXED.
- **Bug 4** (`send_keys_safe` fast-path `agent prompt` returning 0 prematurely, dropping onboarding prompts): FIXED (revised).
- **Bug 5** (non-Claude deferred artifact materialization → unverified UUID): NOT addressed — correctly, per prior cross-review (`b4e8eaee`) which assessed Bug 5 as by-design (modeled as `unverified`/`pending-discovery` initial state, self-heals via periodic re-discovery).

## §2. Bug 2 Fix — headless layout guard — APPROVED

`lib.sh:449-451` (inside the pane-layout Python snippet) inserts, before the `w // 2 >= min_cols` cascade:
```python
if w <= 0 or h <= 0:
    # Headless or detached session with unmeasured/zero dimensions
    print('right')
```
This routes headless/detached panes (which report width/height 0 because there is no measured TTY) to an in-workspace `'right'` split instead of falling through to `print('overflow')`, which previously forced a fresh workspace and caused the workspace-proliferation symptom. The genuine-small-pane case (e.g. 50×30, both dims positive but below thresholds) still correctly yields `'overflow'`. Tested by `test_bug2_headless_layout_does_not_overflow` (4 layout cases: 0×0→right, small→overflow, wide→right, tall→down). No regression. ✓

## §3. Bug 3 Fix — SKILLS_DIR propagation + fallback — APPROVED

Two coordinated changes in `reconcile.sh`, identical to the prior-approved revision:

1. **Env pass** (`reconcile.sh:814, 816`): both `env_python` (dry-run) and `atomic_dump_yaml` (write) invocations now receive `SKILLS_DIR="$SKILLS_DIR" LIB_SH="$LIB_SH"` explicitly, so the embedded Python `RECON_SRC` heredoc sees `SKILLS_DIR` via `os.environ` instead of reading `''`.
2. **Fallback** (`reconcile.sh:328-332`), mirroring the existing `lib_sh` fallback in `lib.sh`:
```python
skills_dir = os.environ.get('SKILLS_DIR', '')
if not skills_dir:
    _ws_root = os.environ.get('WORKSPACE_ROOT')
    if not _ws_root:
        _ws_root = os.path.abspath(os.path.join(os.path.dirname(__file__), '../../../..'))
    skills_dir = os.path.join(_ws_root, '.agents/skills')
```
This is defense-in-depth: even if the caller fails to export `SKILLS_DIR`, the embedded Python reconstructs it from `WORKSPACE_ROOT` (preferred) or from the script's own location (final fallback). Verified safe in stdin mode: `__file__` is `''` under `python -c`/stdin, but the fallback only reaches the `os.path.dirname(__file__)` branch when BOTH `SKILLS_DIR` and `WORKSPACE_ROOT` are unset — `os.path.join(os.path.dirname(''), ...)` resolves to `os.path.join('', ...)` which degrades gracefully; and in practice `WORKSPACE_ROOT` is set by the skill wrapper, so the `__file__` branch is a last resort. Tested by `test_bug3_reconcile_skills_dir_passed_and_fallback` (asserts the env-pass strings and the fallback strings are present). No regression. ✓

## §4. Bug 4 Fix — fast-path gating + duplicate-input guard — APPROVED (regression resolved)

### Prior regression (recap from `c197a005`)
The initial Bug 4 implementation moved the `agent prompt` fast-path behind the quiescence/dialog checks (correctly fixing the ordering defect) but wrapped it in an evidence-verification block: on RPC success it did `sleep 0.5; _pane_capture; grep -Eq "● |✽ |[A-Za-z]+ing"`, and only `return 0` if the evidence matched — otherwise control **fell through to paste-buffer**, which re-sent the same text (duplicate input). The `[A-Za-z]+ing` evidence pattern was also overly broad (matched any `-ing` word), and the source-ordering test did not exercise the runtime control flow.

### Revised fix (`lib.sh:1667-1674`)
```bash
  local agent_target
  agent_target=$(_sanitize_herdr_agent_name "$sess")
  # Native herdr 0.8+ fast path: agent prompt handles atomic text + enter submission
  # Gated behind quiescence and dialog checks; returns 0 on RPC success to prevent duplicate input
  if _sks_herdr agent prompt "$agent_target" "$text" >/dev/null 2>&1 || _sks_herdr agent prompt "$sess" "$text" >/dev/null 2>&1; then
    return 0
  fi

  local sks_buf=...
```

**Ordering (core Bug 4 fix, retained):** The fast-path now executes AFTER `_pane_quiescent` (`lib.sh:1652`, returns 1 if the pane never quiesces) and the `_pane_dialog_open` loop (`lib.sh:1654-1665`, returns 2 on dialog timeout). The pane is confirmed ready (quiet, no modal dialog) before the agent-prompt RPC is attempted. ✓

**Duplicate-input regression — RESOLVED:** On RPC success the block now does an unconditional `return 0` (single send, terminal). Paste-buffer (`lib.sh:1675+`) is reached **only** when the agent-prompt RPC **failed** (the `if … || …; then return 0; fi` is false). Therefore the agent receives the text exactly once: either via the atomic `agent prompt` RPC (success) or via the paste-buffer path (RPC failure). The two paths are mutually exclusive — duplicate input is structurally impossible. The comment (`# returns 0 on RPC success to prevent duplicate input`) documents this design decision explicitly. ✓ This is precisely the "gate the paste-buffer fall-through on agent-prompt failure" guard recommended in the prior review.

**Evidence-grep removed — secondary concern RESOLVED:** The fragile `sleep 0.5` + `_pane_capture` + `grep -Eq "● |✽ |[A-Za-z]+ing"` block is gone entirely. The fix trusts the `herdr agent prompt` RPC's exit code as the authoritative delivery signal (text + Enter submitted atomically to the targeted agent pane). This matches the original pre-bug design intent, now layered correctly on top of the readiness checks. The paste-buffer fallback path retains its full marker-verification + 3-try `C-m` submission loop (`lib.sh:1675-1721`), so submission verification is preserved for the fallback case. No verification capability is lost — the fast-path simply delegates trust to the daemon's RPC contract. ✓

**Test-coverage gap — RESOLVED:** New test `test_bug4_no_duplicate_input_on_rpc_success` (lines ~88-128 of `tests/test_bug_fixes_565255de.py`) sources `lib.sh`, mocks `_pane_quiescent` (→0), `_pane_dialog_open` (→1, no dialog), and `_sks_herdr` (returns 0 on `agent prompt`, sets `PASTE_CALLED=1` on `paste-buffer`), then calls `send_keys_safe "test-sess" "my prompt" "job-1"` and asserts `PASTE_CALLED` stays 0 with a clean exit 0. This directly exercises the runtime control flow (not merely source ordering) and would fail if paste-buffer were reached after a successful RPC. ✓

### Trade-off note
Removing the evidence check makes the fast-path trust the daemon's RPC exit code. This is acceptable because (a) `herdr agent prompt <target> <text>` is the daemon's authoritative "deliver text+Enter to this agent" contract, and (b) the fast-path only runs after the pane is confirmed quiescent and dialog-free, so the RPC targets a ready pane. The prior evidence check was an extra (and fragile) layer whose false-negative produced the regression; removing it is the cleaner resolution.

## §5. Bugs 1 & 5 — correctly unaddressed

- **Bug 1** — not addressed. Per prior cross-review `b4e8eaee`, the top-level `lib.sh` sourcing is guarded so it does not collide with an already-running daemon; the reported mechanism was refuted. Correctly no fix here.
- **Bug 5** — not addressed. Per prior cross-review `b4e8eaee`, deferred artifact materialization for non-Claude agents is real but modeled as the `unverified`/`pending-discovery` initial state and self-heals via periodic `reconcile.sh` re-discovery; by-design, not a standalone defect. Correctly no fix here.

## §6. Lint / Behavior / Loss Assessment + Test Verification

- **Lint:** `bash -n` passes for both `lib.sh` and `reconcile.sh`. No syntax errors; the moved block uses `local` mid-function (valid in bash). No stray artifacts.
- **Behavior:** All three fixes are behaviorally correct. Bug 4's revised implementation eliminates the duplicate-input regression structurally (mutually-exclusive fast/fallback paths) while preserving the core ordering fix.
- **Loss:** No functionality lost. The verified paste-buffer path (marker check + 3-try submission loop) is fully preserved as the fallback; the agent-prompt fast-path remains an optimization layered on top of the readiness checks.
- **Tests:**
  - `tests/test_bug_fixes_565255de.py` — **4 passed** (0.16s), including the new `test_bug4_no_duplicate_input_on_rpc_success`.
  - Existing relevant unit tests (`test_b8_send_keys_verification.py`, `test_herdr_shim_contract.py`) — **6 passed** (7.88s), no regression.
  - (The full `tests/` directory includes pre-existing slow integration tests unrelated to this changeset; the relevant fast unit tests all pass.)

## §7. Findings Summary & Verdict

- **Bug 2 fix:** Approved — correct, tested, no regression.
- **Bug 3 fix:** Approved — robust defense-in-depth (env pass + fallback), tested, no regression.
- **Bug 4 fix:** Approved — the revised implementation resolves all three concerns raised in the prior review (`c197a005`): the duplicate-input regression is structurally eliminated (unconditional `return 0` on RPC success; paste-buffer only on RPC failure), the overly-broad evidence pattern is removed, and a dedicated runtime test (`test_bug4_no_duplicate_input_on_rpc_success`) closes the coverage gap. The core ordering fix (quiescence + dialog before the fast-path) is retained.
- **Bugs 1 & 5:** Correctly left unaddressed (refuted / by-design per prior cross-reviews).
- **Escalation:** None. No design rework is required; all fixes are surgical.

The revised changeset correctly and cleanly fixes the three real bugs (2, 3, 4) without introducing regressions, and directly addresses every finding from the prior cross-review. The implementation is merge-ready.

[VERDICT: PASS]