docs(deploy): synchronize deploy/ assets with nats-docker submodule and hermes prefix defaults

2026-08-23 16:40:22 +09:00
parent adecff2194
commit f133e52863
4 changed files with 216 additions and 1 deletions
@@ -0,0 +1,196 @@
+# Cross-Code Review Report — Job `354f9a22`
+
+- **Reviewer**: cline (session `herdr:canary-projects-multi-agent-mux-creator-cline`)
+- **Date**: 2026-08-23
+- **Changeset**: uncommitted working-tree, 3 files, +20/-1 (`deploy/INSTALL.md`, `deploy/README.md`, `deploy/install.sh`)
+- **Scope**: Cross-code review (lint / behavior / loss) of `deploy/` scripts & documentation
+  synchronization against the latest NATS messaging architecture and the `nats-docker`
+  submodule integration.
+
+---
+
+## 1. Changeset Summary
+
+| File | Δ | Nature |
+|---|---|---|
+| `deploy/INSTALL.md` | +6 | New §7 "전용 NATS 메시징 브로커 설정 (.mam.env)": `.mam.env` generation, `git submodule update --init --recursive` guidance, links to `nats-docker/PRIVATE_SERVER.md` + `MESSAGING.md` |
+| `deploy/README.md` | +13 | New §5 "Private NATS Broker & Submodule Integration (nats-docker)": broker description, `git clone --recurse-submodules` / `git submodule update --init --recursive` commands, links to `nats-docker/PRIVATE_SERVER.md` + `MESSAGING.md` |
+| `deploy/install.sh` | +1/-1 | Inline `.mam.env` default `MAM_CLIENT_PREFIX`: `mam-agent` → `hermes` |
+
+---
+
+## 2. Verification Methodology
+
+1. Gathered changeset via `git diff --stat` / `git --no-pager diff`.
+2. Audited all `deploy/` files for submodule-init support, stale root-`docker/` references,
+   and `MQTT_CLIENT_ID_PREFIX` default alignment.
+3. Verified cross-reference link resolution for every new doc link (target existence +
+   relative-path correctness vs. the document's location under `deploy/`).
+4. Confirmed `MAM_CLIENT_PREFIX="hermes"` consistency across `install.sh`, `.mam.env.example`,
+   `MESSAGING.md`, and `mqtt_common.py`.
+5. Verified `deploy/generate-env.sh` (the `.mam.env` generation path referenced by INSTALL.md)
+   works and relies on the committed `.mam.env.example` template.
+6. Verified `deploy/gitea-ci.yml` test job enables `submodules: recursive`.
+7. Ran mandated tests: `.venv/bin/python -m pytest tests/test_deploy_freshness.py tests/test_sanity.py -q`.
+
+---
+
+## 3. Verification Results
+
+### 3.1 `deploy/install.sh` — `MAM_CLIENT_PREFIX` default — PASS
+- Line 509: `MAM_CLIENT_PREFIX="hermes"` (was `mam-agent`), written to `MQTT_CLIENT_ID_PREFIX=hermes`
+  in the inline `.mam.env` block (line 520).
+- This now matches all other sources of the default:
+  - `mqtt_common.py:230` → `os.environ.get("MQTT_CLIENT_ID_PREFIX", "hermes")`
+  - `mqtt_common.py:218` docstring → `MQTT_CLIENT_ID_PREFIX (hermes)`
+  - `.mam.env.example:80-81` → `#default: hermes` / `# MQTT_CLIENT_ID_PREFIX=hermes`
+  - `MESSAGING.md:302` → `MQTT_CLIENT_ID_PREFIX | hermes`
+- No `mam-agent` references remain anywhere in code/docs (only in historical job briefs/logs).
+- The change is a correct, surgical alignment fix.
+
+### 3.2 Stale root `docker/` references — PASS
+- `grep -rn 'docker/' deploy/ | grep -v nats-docker` → **none found**.
+- All `deploy/` scripts (`install.sh`, `install_mam.sh`, `update.sh`, `remove.sh`,
+  `generate-env.sh`) and docs (`INSTALL.md`, `README.md`, `gitea-ci.yml`) contain zero
+  references to the removed root-level `docker/` directory. All Docker references now point to
+  the `nats-docker/` submodule. Migration is complete.
+
+### 3.3 Submodule initialization support — PASS
+- **CI**: `deploy/gitea-ci.yml` test job (line 82-89) uses `actions/checkout@v3` with
+  `submodules: recursive`, then runs `pytest tests/ -q`. Correct — CI test runs get the
+  `nats-docker` assets.
+- **Fresh install / update (documentation)**: `deploy/INSTALL.md` §7 and `deploy/README.md` §5
+  both instruct users to run `git submodule update --init --recursive` (README.md also shows
+  `git clone --recurse-submodules ...` for fresh clones).
+- **Scripts**: `install.sh`, `update.sh`, `install_mam.sh`, `remove.sh` do **not** auto-run
+  `git submodule update --init --recursive`. This is appropriate ("where appropriate" in the
+  task): the `nats-docker` submodule holds *optional private-broker deployment assets*
+  (docker-compose, nats.conf, guides), not the MAM runtime. Forcing git operations during a
+  user-environment install/update would be wrong for users who don't deploy a private broker
+  and could fail where git/submodule access is unavailable. Submodule init is therefore
+  *documented guidance* (present) rather than *automated* (correctly absent) — consistent
+  with the submodule being an optional deployment concern.
+
+### 3.4 `.mam.env` generation & default MQTT parameters — PASS
+- `deploy/generate-env.sh` copies `.mam.env.example` → `.mam.env` (idempotent, `--force`/
+  `--migrate-legacy` options, repo-root-relative path resolution). Works as documented.
+- `.mam.env.example` documents `MQTT_CLIENT_ID_PREFIX` default as `hermes` (line 80-81),
+  consistent with `install.sh` and code.
+- INSTALL.md §7 correctly directs users to `bash deploy/generate-env.sh` (or
+  `cp .mam.env.example .mam.env`) for `.mam.env` creation.
+- Note (pre-existing, **not** introduced by this changeset): `.mam.env.example:7` comments
+  `scripts/generate-env.sh` while the actual path is `deploy/generate-env.sh`. Out of scope
+  for this review; flagged for awareness only.
+
+### 3.5 `deploy/README.md` §5 — PASS
+- New §5 accurately describes the private NATS broker (`nats:2.12-alpine`, MQTT 3.1.1 +
+  JetStream) and the `nats-docker` submodule.
+- Provides both fresh-clone (`git clone --recurse-submodules <url>`) and existing-clone
+  (`git submodule update --init --recursive`) commands.
+- Clone URL `https://git.godopu.com/tmpl/multi-agent-mux.git` matches the actual remote origin.
+- Cross-reference links use the correct file-relative form:
+  `../nats-docker/PRIVATE_SERVER.md` and `../MESSAGING.md` (both resolve from `deploy/` to the
+  repo-root targets, confirmed to exist). ✓
+
+### 3.6 `deploy/INSTALL.md` §7 — see M-1 (link defect), content otherwise PASS
+- §7 content is accurate and well-placed: NATS broker purpose, `.mam.env` generation path,
+  submodule sync command, and broker/Tailscale guide pointer.
+- The `git submodule update --init --recursive` guidance is correct and consistent with
+  README.md §5.
+- **Link defect**: the two cross-reference links use a non-standard `file://./` scheme
+  (see Finding M-1). Content is correct; only the link URLs are wrong.
+
+### 3.7 Mandated tests — PASS
+- `.venv/bin/python -m pytest tests/test_deploy_freshness.py tests/test_sanity.py -q`
+  → **33 passed** in 22.58s. 100% pass rate, 0 regressions.
+- No test depends on the new doc sections, so the changeset is test-neutral; the prior D-31
+  (CI submodules) and D-32 (MESSAGING.md env coverage) guards remain green.
+
+---
+
+## 4. Detailed Findings
+
+### M-1 (Medium) — Non-standard / broken cross-reference links in `deploy/INSTALL.md` §7
+
+- **Location**: `deploy/INSTALL.md` line 134 (new §7):
+  - `[\`nats-docker/PRIVATE_SERVER.md\`](file://./nats-docker/PRIVATE_SERVER.md)`
+  - `[\`MESSAGING.md\`](file://./MESSAGING.md)`
+- **Observation**: These links use the `file://./<path>` URL scheme. Per RFC 8089, `file://`
+  introduces an authority; `file://./...` places a `.` (invalid) authority before the path, so
+  the form is non-standard. More importantly, `file://` URLs are **not** rewritten to
+  repo-relative paths by the Gitea/GitHub markdown renderer — they render as literal `file://`
+  links. Resolved relative to the document's location (`deploy/`), `./nats-docker/...` and
+  `./MESSAGING.md` point to `deploy/nats-docker/PRIVATE_SERVER.md` and `deploy/MESSAGING.md`,
+  both of which **do not exist** (confirmed: `deploy/nats-docker/` and `deploy/MESSAGING.md`
+  are missing; the real targets are at the repo root).
+- **Inconsistency**: The sibling `deploy/README.md` §5, added in the **same** changeset, uses
+  the correct file-relative form `../nats-docker/PRIVATE_SERVER.md` and `../MESSAGING.md`
+  (which resolve from `deploy/` to the repo-root targets). The two new sections therefore
+  disagree on link convention.
+- **Impact**: Medium. A user following INSTALL.md cannot click through to the private-broker
+  guide / MESSAGING reference in the Gitea web UI (the canonical viewing context). The link
+  *text* still shows the path, so a user can navigate manually, and README.md §5 provides
+  working links — impact is mitigated but the defect is real and functional (broken
+  navigation), not merely cosmetic. No runtime/test effect.
+- **Recommendation**: Replace the two `file://./` URLs with the file-relative form used by
+  README.md:
+  - `file://./nats-docker/PRIVATE_SERVER.md` → `../nats-docker/PRIVATE_SERVER.md`
+  - `file://./MESSAGING.md` → `../MESSAGING.md`
+  This is a 2-token surgical edit; no design change required.
+
+### Note (pre-existing, out of this changeset's scope)
+- `.mam.env.example:7` documents the generator path as `scripts/generate-env.sh` but the
+  actual location is `deploy/generate-env.sh`. This predates the changeset and is not
+  introduced or touched by it; flagged for awareness only (do not fix in this review's scope).
+
+### Positive observations
+- `install.sh` `MAM_CLIENT_PREFIX` → `hermes` is a clean, correct alignment that achieves
+  100% consistency across `mqtt_common.py`, `.mam.env.example`, `MESSAGING.md`, and fresh
+  `.mam.env` generation.
+- Zero stale root-`docker/` references across the entire `deploy/` tree.
+- Submodule-init guidance is consistently provided in both INSTALL.md and README.md, and CI
+  correctly automates it via `submodules: recursive`. Scripts appropriately do **not** force
+  the optional submodule during user install/update.
+- README.md §5 links and clone URL are correct.
+---
+
+## 5. Risk Assessment
+
+| Area | Status |
+|---|---|
+| Runtime behavior | No runtime code changed (docs + one shell default). `install.sh` default alignment is correct. PASS. |
+| Test suite | 33/33 mandated tests pass; 0 regressions. PASS. |
+| Stale references | Zero root-`docker/` references in `deploy/`. PASS. |
+| Submodule support | CI automates (`submodules: recursive`); docs guide manual init; scripts correctly leave optional submodule out of user install/update. PASS. |
+| `.mam.env` / MQTT defaults | `hermes` consistent across install.sh, .mam.env.example, MESSAGING.md, code. PASS. |
+| Cross-reference links | INSTALL.md §7 links use non-standard `file://./` → broken in Gitea renderer (M-1). README.md links correct. Minor / non-blocking. |
+| Loss / orphaned references | None — all `nats-docker/` and `MESSAGING.md` targets exist at repo root. PASS. |
+
+The single finding (M-1) is documentation-level, non-blocking, and fixable by a 2-token edit.
+No design-level rework is warranted; no `[ESCALATE: PLANNER]` is required.
+
+---
+
+## 6. Actionable Follow-up (optional, small cleanup commit)
+
+1. **M-1**: In `deploy/INSTALL.md` line 134, replace
+   `(file://./nats-docker/PRIVATE_SERVER.md)` → `(../nats-docker/PRIVATE_SERVER.md)` and
+   `(file://./MESSAGING.md)` → `(../MESSAGING.md)` to match README.md §5's working link form.
+2. *(Pre-existing, separate)*: Fix `.mam.env.example:7` path comment
+   `scripts/generate-env.sh` → `deploy/generate-env.sh`.
+
+---
+
+## 7. Verdict
+
+All mandated tests pass (33/33, 0 regressions). The `install.sh` `MAM_CLIENT_PREFIX` →
+`hermes` change correctly aligns the default across code, template, and docs. No stale
+root-`docker/` references remain anywhere in `deploy/`. Submodule initialization is properly
+supported (CI automates it; INSTALL.md and README.md document the manual step; scripts
+appropriately treat the optional `nats-docker` submodule as a deployment concern rather than
+a runtime one). The only finding (M-1) is a non-standard, non-functional cross-reference
+link scheme in `deploy/INSTALL.md` §7 — documentation-level, non-blocking, fixable by a
+2-token edit, and inconsistent only with the sibling README.md §5 added in the same changeset.
+No escalation to the planner is warranted.
+
+[VERDICT: PASS]