46 lines
4.7 KiB
Markdown
46 lines
4.7 KiB
Markdown
# 🏛️ Planner Claude — MAM Installer Final Architecture Re-Review
|
|
|
|
- **Scope**: Brief `.agents/reports/brief-rereview-all.md` §3 (Planner Claude)
|
|
- **Commits under review**: `d7e19fe` → `66fd1c4` → `5cb8c39`
|
|
- **Date**: 2026-07-11 (supersedes prior NOT PASS revision of this report)
|
|
- **Method**: Static diff review + live end-to-end install diagnostics on native host PATH (fresh target, re-run idempotency, pre-existing `AGENTS.md`/`.gitignore`, symlink invocation, shellcheck)
|
|
|
|
---
|
|
|
|
## Verdict: ✅ PASS — with one working-tree regression that must NOT be committed
|
|
|
|
Both blockers from my prior review (B-1 attach inconsistency in `INSTALL.md`, B-2 false `sqlite3` CLI dependency) are resolved and verified live. The architecture now aligns with MAM standards on the version-control axis. However, an **uncommitted `.gitignore` change adding `.agents/reports`** directly contradicts the durable-reports policy ratified in `d7e19fe` and must be reverted before any commit.
|
|
|
|
---
|
|
|
|
## ✅ Resolved and verified
|
|
|
|
| Item | Evidence |
|
|
|---|---|
|
|
| B-1: RC-1 attach alignment | `INSTALL.md` §3-1 create example now includes `--tmux-server multi-agent-mux`, matching the §3-2 attach socket. Manual flow is now internally consistent (default server in `lib.sh:33` is `default`, so the explicit flag is required and now present). |
|
|
| B-2: sqlite3 dependency | `sqlite3` CLI removed from `DEPS`; replaced with hard `python3 -c "import yaml, sqlite3"` check — matching actual runtime usage (all DB access is via Python module in heredocs). **Verified live: install now succeeds on this host's native PATH, which has no `sqlite3` binary.** `INSTALL.md` §1 updated accordingly. |
|
|
| `flock` removal (correction) | My earlier review implied `flock` was a CLI dependency; on inspection all locking is Python `fcntl.flock` (`reconcile.sh:76`) — the other grep hits are comments. Removing `flock` from `DEPS` in `66fd1c4` was **correct**. |
|
|
| `uuidgen` retained | Genuine CLI dependency (`create_session.sh:125,127`, required for `--isolate`); correctly kept in `DEPS`. |
|
|
| Report migration | `git ls-files .mam/` empty; durable reports tracked under `.agents/reports/<session>/`; `MULTI_AGENT_RULES.md` (+`.ko`) and `INSTALL.md` §🛡️ consistent. Anchored rsync `--exclude='/reports/'` verified to keep internal reports out of targets. |
|
|
| `AGENTS.md` non-invasive injection | Verified live with pre-existing `AGENTS.md`: content preserved, marker block appended once, rerun is a no-op. Version-control safe. |
|
|
| Hygiene | `bash -n` + `shellcheck` clean; symlink invocation resolves `SRC_DIR`; idempotent second run on fresh and pre-populated targets. |
|
|
|
|
---
|
|
|
|
## 🚫 Must fix before commit
|
|
|
|
**Working-tree `.gitignore` adds `.agents/reports`** (uncommitted). This un-does the durable-reports migration for all *future* reports: already-committed files stay tracked, but new mandated artifacts (e.g., the Reviewer Cline `report-mam-installer-final.md` this brief requires, and this very report) would be silently untracked — reintroducing the exact audit-trail loss the migration fixed. It also conflicts verbatim with `MULTI_AGENT_RULES.md` ("must be explicitly copied to tracked directory paths (specifically under `.agents/reports/<tmux_session_name>/`…)"). **Recommendation: revert this hunk.** If the intent was to exclude transient briefs, ignore a narrower pattern (e.g., `.agents/reports/brief-*.md`) — but do not ignore the reports tree itself.
|
|
|
|
## ⚠️ Minor (non-blocking)
|
|
|
|
1. `INSTALL.md` §2 step 1 still says the installer checks "`tmux`, `python3`, `sqlite3`" — stale; actual check is `tmux`, `python3`, `rsync`, `uuidgen` + Python `yaml`/`sqlite3` modules.
|
|
2. `INSTALL.md` §1 omits `uuidgen` (hard dep for `--isolate`) and presents `rsync` without noting it is installer-only.
|
|
3. Source `AGENTS.md` lacks the MAM marker block, so a fresh-copy install converges only on the second run (pointer self-injection). Cosmetic; append the marker at copy time to converge in one run.
|
|
4. Still open from planning (roadmap, not gating): `.agents/.mam-version` stamping + `--update --delete` upgrade mode; shared `check_deps.sh` used by both installer and `create_session.sh`; bilingual `INSTALL.md`/`INSTALL.ko.md` split per repo convention.
|
|
|
|
---
|
|
|
|
## Architecture alignment summary
|
|
|
|
With `66fd1c4` and `5cb8c39`, the installer satisfies MAM standards: dependency diagnosis now reflects the true runtime contract, the manual's create/attach flow is consistent, the `.mam/` runtime tree vs. tracked `.agents/` configuration boundary is crisp, and downstream projects' behavioral guidelines are preserved. Deployment across other projects is approved once the `.gitignore` working-tree regression is discarded; the minor doc drift can ride along in a follow-up docs commit.
|