# Review Report — MAM Installer (`scripts/install_mam.sh`) & Manual (`.agents/INSTALL.md`)

- **Reviewer**: Reviewer Cline (`canary-projects-multi-agent-mux-reviewer-cline`, role: reviewer)
- **Date**: 2026-07-11
- **Files reviewed**: `scripts/install_mam.sh` (untracked, new), `.agents/INSTALL.md` (untracked, new)
- **Governing documents**: `AGENTS.md`, `.agents/MULTI_AGENT_RULES.md` / `.ko.md`

---

## Verdict: **NOT PASS** ❌

The script passes its claimed static checks (`bash -n` ✅, `shellcheck` clean ✅) and the manual is well-structured. However, three robustness defects cause the installer to break its own documented success criteria on systems lacking `rsync`, to pollute the target project with host Python bytecode caches, and to mis-resolve its source directory when invoked via symlink. These contradict `AGENTS.md` §4 (Goal-Driven Execution: "Define success criteria. Loop until verified") and §1 (Think Before Coding: "Surface tradeoffs... if unclear, ask"). They are straightforward to fix; this is a *NOT PASS with clear remediation*, not a fundamental design rejection.

---

## 1. Validation Commands Run In-Session

| Check | Command | Result |
|-------|---------|--------|
| Syntax | `bash -n scripts/install_mam.sh` | ✅ `BASH_N_OK` |
| Lint | `shellcheck -f gcc scripts/install_mam.sh` | ✅ clean (0 findings) |
| Git state | `git status --short scripts/install_mam.sh .agents/INSTALL.md` | both untracked (`??`) |
| Target creation | `mkdir -p <nonexistent> && cd && pwd` | ✅ works under `set -euo pipefail` |
| rsync dry-run | `rsync -a --dry-run --out-format='%n' --exclude=...` | ⚠️ reveals `__pycache__/`+`.pyc` leak |
| Symlink resolution | `cd "$(dirname "/tmp/symlinked.sh")/.."` | ❌ resolves to `/` (wrong source) |
| `.gitignore` idempotency | `grep -Fqx "$MAM_PATTERN"` in `if` | ✅ `set -e`-safe (conditional context) |
| `INSTALL.md` inclusion | dry-run file list | ✅ `INSTALL.md` is copied |

---

## 2. Defects Found (blocking)

### D1 — `rsync` is an undeclared hard dependency (MEDIUM)

**Location**: `scripts/install_mam.sh:100`
```bash
rsync -a --exclude='.git/' --exclude='reports/' --exclude='*.log' "$SRC_DIR/.agents/" "$TARGET_DIR/.agents/"
```

**Problem**: `rsync` is invoked as the core copy mechanism, but it is absent from:
- The script's `DEPS` array (line 73: `DEPS=(tmux python3 sqlite3)` — no `rsync`)
- `.agents/INSTALL.md` §1 prerequisite list (tmux, python3, sqlite3, pyyaml — no rsync)

**Impact**: On a system without `rsync` (e.g. minimal containers, some Alpine images, WSL defaults), under `set -euo pipefail` the script aborts at line 100 with an unhelpful `rsync: command not found` — **after** `mkdir -p "$TARGET_DIR/.agents"` (line 96) has already partially created the target tree. The user is left with a half-installed `.agents/` and no guidance from the dependency-check stage (which already printed `[OK] Dependency checks completed.`).

**Why it violates the guidelines**:
- `AGENTS.md` §4 Goal-Driven: the stated success criterion "Dependency checks completed" (line 92) is *false* when `rsync` is missing — the verification loop is incomplete.
- `AGENTS.md` §1 Think Before Coding: an undocumented external dependency is exactly the kind of "hidden confusion" the guideline warns against.

**Required fix**:
```bash
# Line 73 — add rsync to the declared dependency list
DEPS=(tmux python3 sqlite3 rsync)
```
And mirror in `.agents/INSTALL.md` §1: add `**rsync**: install_mam.sh .agents/ 폴더 동기화에 사용`.

### D2 — `__pycache__/` + `.pyc` bytecode caches leak into the target (MEDIUM)

**Location**: `scripts/install_mam.sh:100`

**Problem**: The rsync exclude list does **not** exclude Python bytecode caches. Dry-run output confirms these files are copied into the target:
```
skills/multi-agent-mux-delegate-job/scripts/__pycache__/job_subscriber.cpython-314.pyc
... (4 .pyc files total)
```

**Impact**: The installer pollutes the target with host-specific (CPython-version-stamped) bytecode caches. The source repo already ignores these via root `.gitignore` lines 13–14 (`__pycache__/`, `*.pyc`), but rsync reads the filesystem, not gitignore. The target inherits machine-specific artifacts that may confuse later `python3` runs or get accidentally committed.

**Why it violates**: `AGENTS.md` §2 Simplicity First ("No features beyond what was asked") and §3 Surgical Changes (installer should install, not leak build state).

**Required fix**:
```bash
rsync -a --exclude='.git/' --exclude='reports/' --exclude='*.log' \
  --exclude='__pycache__/' --exclude='*.pyc' \
  "$SRC_DIR/.agents/" "$TARGET_DIR/.agents/"
### D3 — Symlink-invoked source resolution mis-resolves to `/` (MEDIUM)

**Location**: `scripts/install_mam.sh:60`
```bash
SRC_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
```

**Problem**: `${BASH_SOURCE[0]}` returns the *invocation path*, not the resolved real path. When the script is run via a symlink (e.g. `ln -sfn .../install_mam.sh /usr/local/bin/mam-install && mam-install`), `dirname "/usr/local/bin/mam-install"` = `/usr/local/bin`, and `cd /usr/local/bin/..` = `/usr/local`. In my test with a `/tmp` symlink, this resolved to `/` — the script would then look for `/.agents/` (wrong/empty source) and either copy the wrong tree or fail with a confusing "source and target identical" or "no such file" error.

**Impact**: The documented usage (`bash scripts/install_mam.sh --target ...`) works only when invoked from a real path. Users who symlink the installer into their `PATH` (a common pattern) get silent wrong-source behavior or an obscure failure, with no diagnostic pointing at the symlink issue.

**Why it violates the guidelines**:
- `AGENTS.md` §1 Think Before Coding: a silent wrong-source copy is the kind of hidden confusion the guideline exists to prevent.
- `MULTI_AGENT_RULES.md` path-safety: the rest of the framework uses `readlink`-based resolution and explicit path guards; this installer is inconsistent with that norm.

**Required fix** (Linux; the project's documented platform):
```bash
SRC_DIR="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")/.." && pwd)"
```
`readlink -f` is GNU coreutils. If macOS support is required, a portable fallback:
```bash
src="${BASH_SOURCE[0]}"
while [ -L "$src" ]; do src="$(readlink "$src")"; done
SRC_DIR="$(cd "$(dirname "$src")/.." && pwd)"
```
Add `readlink` to `DEPS` if taking the `readlink -f` route.

---

## 3. Conformance to `AGENTS.md`

| Principle | Assessment |
|-----------|------------|
| §1 Think Before Coding | ❌ D1/D3 hide undocumented dependencies and a symlink footgun instead of surfacing them. |
| §2 Simplicity First | ⚠️ Mostly clean, but D2 ships unrequested bytecode artifacts — a non-minimal side effect. |
| §3 Surgical Changes | ✅ No drive-by refactors; existing-user `AGENTS.md` is backed up before overwrite (lines 106–110). |
| §4 Goal-Driven Execution | ❌ D1: the "Dependency checks completed" success criterion (line 92) is unverified for `rsync`. |

---

## 4. Conformance to `MULTI_AGENT_RULES.md`

| Rule | Assessment |
|------|------------|
| `.mam/` under gitignore | ✅ `.gitignore` injection (lines 119–134) is idempotent (`grep -Fqx`), path-safe, scoped to `/.mam/`. |
| Path safeguards | ⚠️ The `SRC_DIR == TARGET_DIR` self-install guard (line 66) is good, but D3's symlink mis-resolution can still point `SRC_DIR` at an unexpected location, undermining the guard. |
| Markdown collaboration | ✅ `.agents/INSTALL.md` is a proper markdown manual; this report is persisted under `.mam/reports/<session>/`. |
| Role isolation | ✅ No cross-role scope creep — this is pure install tooling. |

---

## 5. What's Good (acknowledge correctly done)

- `set -euo pipefail` at the top — correct strict-mode hygiene.
- `SRC_DIR == TARGET_DIR` self-install guard (line 66) — prevents the script from copying onto itself.
- `AGENTS.md` backup-before-overwrite with timestamped `.bak.<epoch>` (lines 107–110) — respects existing user files; `--force` is opt-in.
- `.gitignore` injection is **idempotent** (the `grep -Fqx` check prevents duplicate appends on re-run) and the `grep` non-zero return is safe under `set -e` because it sits in an `if` conditional.
- `INSTALL.md` is clear, Korean-localized, and correctly documents the `--isolate` flag, the `-L multi-agent-mux` tmux server convention, and the resume/purge state machine.
- `bash -n` and `shellcheck` claims are **accurate** — I reproduced both.

---

## 6. Remediation Summary (for the developer)

| ID | Fix | Effort |
|----|-----|--------|
| D1 | Add `rsync` to `DEPS` array (line 73) and to `INSTALL.md` §1 prereq list | 2 lines |
| D2 | Add `--exclude='__pycache__/' --exclude='*.pyc'` to the rsync invocation (line 100) | 1 line |
| D3 | Resolve symlinks: `readlink -f "${BASH_SOURCE[0]}"` before `dirname`/`cd` (line 60); add `readlink` to `DEPS` | 1–2 lines |
| Re-verify | Re-run `bash -n` + `shellcheck` + a scratch-target dry-run after fixes | — |

All three are small, surgical edits that trace directly to the defects above. No design rework is needed.

---

## 7. Final Statement

The installer's static hygiene is genuine (`bash -n`/`shellcheck` pass as claimed), and the manual is solid. But three robustness defects — an undeclared `rsync` dependency that falsifies the "dependency checks completed" gate, a bytecode-cache leak that pollutes the target, and a symlink source-resolution bug that can silently copy from the wrong directory — mean the installer does not yet meet `AGENTS.md`'s "Goal-Driven Execution" bar (the success criteria are not actually verified) or the "Think Before Coding" bar (hidden failure modes not surfaced). These are fixable in under five lines total.

**NOT PASS** — return to developer with D1/D2/D3 remediation. Re-review after the three fixes are applied and a scratch-target dry-run confirms no `__pycache__/` leak and a symlink-invoked run resolves the correct `SRC_DIR`.
```