diff --git a/docs/design/reviews/review-agy-fd384d6b.md b/docs/design/reviews/review-agy-fd384d6b.md new file mode 100644 index 0000000..964405a --- /dev/null +++ b/docs/design/reviews/review-agy-fd384d6b.md @@ -0,0 +1,138 @@ +# Design Review Report: Worktree Creator Operating Model + +**Reviewer**: Antigravity (`herdr:reviewer-agy-01`) +**Job ID**: `fd384d6b` +**Target Document**: `docs/design/worktree-creator-model.md` +**Branch / Commit**: `design/worktree-model` @ [`fe96db1`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/design-worktree-model/docs/design/worktree-creator-model.md) +**Date**: 2026-09-14 +**Status**: Review Complete + +--- + +## 1. Executive Summary + +The proposal in [`docs/design/worktree-creator-model.md`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux-wt/design-worktree-model/docs/design/worktree-creator-model.md) addresses two critical limitations resulting from MAM's historical assumption that `"workspace" == "single checkout path"`: +1. **Inability to run concurrent issue workflows**: Multiple feature branches cannot be checked out simultaneously in a single working tree. +2. **Accidental branch switching collisions**: In Issue #18, a checkout switch caused 12 consecutive commits to accumulate directly on `main` without isolation. + +The proposal evaluates three candidate models: +- **Candidate A (Full Worktree Isolation)**: Spawns 5 full agent instances per issue/worktree. +- **Candidate B (Orchestrator-Only Worktree)**: Isolates orchestrator commits while keeping all agents in the root checkout. +- **Candidate C (Proposed - Worktree Creator with Shared Reviewer Pool)**: Isolates only mutating agents (Creators) in dedicated git worktrees while sharing a persistent, read-only Reviewer pool and centralized `.mam` control plane in the root checkout. + +### Recommendation +**Candidate C is fundamentally sound, resource-efficient, and practical.** It minimizes token quota consumption while providing strict git branch isolation for code modifications. With the targeted code supplements detailed below (addressing brief path relativity, loop runner worktree diff collection, reviewer test isolation, and worktree lifecycle cleanup), Candidate C is fully adoptable. + +--- + +## 2. Responses to Document §6 Questions + +### Q1: Does Candidate C's role separation solve the two problems? What failure paths are missed? +- **Verdict**: **Agree with Supplement (보완)**. +- **Analysis**: + - **Branch Switching Collision**: Resolved completely. The Creator is the *only* agent that edits source code and creates git commits. Binding the Creator's `cwd` to `$WT` (e.g. `fix/issue-20`) guarantees that commits can only land on the issue branch. Even if another process alters HEAD in `$MAIN`, the worktree's HEAD remains untouched. + - **Parallelism**: Resolved for implementation. Multiple Creators can work simultaneously on separate worktrees without filesystem contention. +- **Missed Failure Paths & Countermeasures**: + 1. **Reviewer Test Pollution in Worktree**: §3 states that reviewers run tests in the worktree path (`$WT`). However, standard test runners (e.g. `pytest`) create untracked artifacts (`.pytest_cache`, `__pycache__`, `.pyc`, and temporary scratch files) directly in `$WT`. This contaminates the Creator's working tree and risks accidental inclusion in git commits. + *Countermeasure*: Reviewers must run verification with cache suppression (e.g., `PYTHONDONTWRITEBYTECODE=1 pytest -o cache_dir=/tmp/...`), or tests must be executed with `--basetemp` outside `$WT`. + 2. **Stale Session Rows After Worktree Deletion**: When an issue branch is merged and `$WT` is unlinked (`git worktree remove $WT`), the Creator session row in `agent-sessions.yaml` still records `pane.cwd: $WT`. If the session is resumed or reconciled later, missing paths can trigger errors. + *Countermeasure*: Establish a documented retirement procedure (`scripts/worktree_remove.sh`) enforcing that `multi-agent-mux-stop --session --purge` runs before `git worktree remove`. + +--- + +### Q2: Is control plane sharing via environment override safe in current code? +- **Verdict**: **Agree with Supplement (보완)**. +- **Analysis across Skill Tooling**: + - [`lib.sh:26`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib.sh#L26): Correctly prioritizes `${WORKSPACE_ROOT:-$SKILL_DIR/../..}`. Overriding `WORKSPACE_ROOT="$MAIN"`, `AGENT_SESSIONS_YAML="$MAIN/.mam/agent-sessions.yaml"`, `MAM_JOBS_DIR="$MAIN/.mam/jobs"`, and `DELEGATE_JOB_LOGS_DIR` successfully routes all state into the primary repository. + - [`multi-agent-mux-create/scripts/create_session.sh:309`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-create/scripts/create_session.sh#L309): Records `pane.cwd: $WORKSPACE` ($WT) in the YAML row, while registering into `$AGENT_SESSIONS_YAML`. + - [`multi-agent-mux-monitor/scripts/reconcile.sh:745-752`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh#L745-L752): Drift C conversation discovery uses `cwd = (s.get('pane') or {}).get('cwd', '')` and `key = workspace_key(cwd)`. Because Claude stores conversations under `~/.claude/projects/` derived from `cwd`, Claude conversation IDs are accurately resolved against `$WT`! + - [`multi-agent-mux-status/scripts/status.sh:59, 220`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-status/scripts/status.sh#L59): Accurately reads `pane.cwd` and displays the worktree directory without drift errors. + - [`multi-agent-mux-stop/scripts/stop_session.sh:133, 143`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh#L133): Retrieves `TARGET_CWD` directly from the row's `pane.cwd`. + - [`multi-agent-mux-resume/scripts/resume_session.sh:42, 52`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh#L42) & [`workspace_uuid.py:57-58`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/workspace_uuid.py#L57-L58): Resolves UUID by matching `s_cwd == ws`. Resuming a Creator requires `--workspace "$WT"`. + - **Cautionary Bug Identified**: In [`create_session.sh:526`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-create/scripts/create_session.sh#L526): + ```bash + (bash "$WORKSPACE/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh" --once >/dev/null 2>&1 &) || true + ``` + This background subshell executes inside `$WT` without explicitly inheriting the exported `WORKSPACE_ROOT` / `AGENT_SESSIONS_YAML` variables, risking spawning a monitor tied to `$WT/.mam`. The recipe must ensure these envs are propagated. + +--- + +### Q3: Cost/benefit evaluation of Candidates A, B, and C? +- **Verdict**: **Agree (동의)**. +- **Analysis**: + - **Candidate A**: Highly wasteful. Spawning 5 agents per issue multiplies token consumption and rate limits by $N$. Reviewer context is completely discarded between issues. + - **Candidate B**: Ineffective. Preserves the single-checkout bottleneck and fails to eliminate checkout collisions for creators. + - **Candidate C**: Achieves optimal resource efficiency. Exactly 1 lightweight Creator agent is spawned per worktree; all Reviewers remain warm and retain conversational memory in the main session pool. + +--- + +### Q4: Evaluation of §5 Task Items & Multi-Loop Contention +- **Verdict**: **Supplement (보완)**. +- **Analysis of §5 Items**: + - **Items 1, 2, 3, 5**: High priority, straightforward, and essential. + - **Item 4 (Multi-Loop Contention)**: Concurrently executing multiple loops against a single `.mam` directory is currently **infeasible** due to hardcoded singletons in the codebase: + 1. [`lib_py/loop/runner.py:60`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L60): `self.checkpoint_file = self.mam_dir / "loop-checkpoint.json"`. Two loops will overwrite each other's checkpoint. + 2. [`lib_py/loop/runner.py:801`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L801): When one loop finishes, it calls `checkpoint.remove_checkpoint()`, unlinking the checkpoint file while the other loop is still active! + 3. [`lib_py/loop/runner.py:427`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L427): Guard marker is hardcoded to `.mam/loop-guard-active`. + 4. [`lib_py/loop/runner.py:312-326`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L312-L326): When a reviewer is busy, the runner retries once after 30s. If a reviewer in Loop 1 is running tests for 2 minutes, Loop 2 fails with exit code 5 immediately. +- **Phasing Recommendation**: + - **Phase 1 (Immediate)**: **"Parallel Creators, Serial Loops"** (루프는 하나씩, 구현만 병렬). Creators can implement code in parallel in their respective worktrees. Loops are triggered sequentially. This delivers immediate branch isolation and parallel coding without requiring a major rewrite of the loop control plane. + - **Phase 2 (Future)**: Support fully concurrent loops by parameterizing `loop-checkpoint-.json`, `loop-guard-active-`, and implementing a proper reviewer dispatch queue. + +--- + +### Q5: Ordering relative to Issue #17 Item 4 (Shell Layer Reduction) +- **Verdict**: **Agree (동의)**. +- **Rationale**: + - Documenting and validating Candidate C via recipes and targeted wrapper patches first establishes clear operational requirements. + - When Issue #17 Item 4 consolidates shell wrappers into Python (`lib_py`), the distinction between `worktree_root` (code under test) and `control_plane_root` (shared `.mam` registry) can be implemented natively in Python classes (`LoopRunner`, `Registry`) without having to refactor bash scripts twice. + +--- + +## 3. Code-Level Evidence and Review Checklist + +### 3.1 `lib.sh:26` and Environment Overrides +- **Finding**: Overrides for `WORKSPACE_ROOT`, `AGENT_SESSIONS_YAML`, `MAM_JOBS_DIR`, and `DELEGATE_JOB_LOGS_DIR` are respected across all skill tools. +- **Evidence**: + - [`lib.sh:26, 34`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib.sh#L26-L34): Defaults are evaluated with `${VAR:-...}`, permitting clean external injection. + - [`create_session.sh:353, 372`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-create/scripts/create_session.sh#L353): Respects `MAM_JOBS_DIR` and `AGENT_SESSIONS_YAML`. + - [`stop_session.sh:99, 151`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-stop/scripts/stop_session.sh#L99): Respects `AGENT_SESSIONS_YAML` and `MAM_JOBS_DIR`. + +### 3.2 Prompt Injection Relative Brief Paths (§5-1) +- **Finding**: Prompt injection templates currently hardcode relative paths (`.mam/jobs//brief.md`), which will break when an agent's `cwd` is `$WT`. +- **Evidence**: + - [`multi-agent-mux-delegate-job:199`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job#L199): + `"task brief file at: .mam/jobs/$JOB_ID/brief.md. Please READ and follow .mam/jobs/$JOB_ID/brief.md"` + - [`multi-agent-mux-delegate-job:455`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job#L455): + `"documented in the task brief file at: .mam/jobs/$JOB_ID/brief.md"` + - [`multi-agent-mux-delegate-job:352`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job#L352): + `- **Output Report Path**: .mam/jobs/$JOB_ID/${clean_session}-reports/report-final.md` +- **Actionable Fix**: Update these three lines to use absolute path `$REGISTRY_DIR/$JOB_ID/brief.md` and `$REGISTRY_DIR/$JOB_ID/${clean_session}-reports/report-final.md`. + +### 3.3 Loop Runner Diff Collection & `base_commit` (§5-2) +- **Finding**: `LoopRunner` currently evaluates git status strictly against `self.workspace_root` ($MAIN). +- **Evidence**: + - [`lib_py/loop/runner.py:564`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L564): + `subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(self.workspace_root))` + - [`lib_py/loop/runner.py:402, 408`](file:///Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.agents/skills/lib_py/loop/runner.py#L402): + `subprocess.run(..., cwd=str(self.workspace_root))` +- **Actionable Fix**: Introduce `--worktree ` (or `MAM_LOOP_WORKTREE`). Set `self.worktree_root = Path(args.worktree).resolve() if args.worktree else self.workspace_root`. Execute `git rev-parse HEAD` and `collect_changes_diff()` in `cwd=str(self.worktree_root)`. + +--- + +## 4. Specific Document Modifications + +| Section in Document | Proposed Modification | Rationale | +|---|---|---| +| **§3 (동작 방식)** | `create_session.sh` 호출 시 `create_session.sh:526` 백그라운드 reconcile 호출에 env 가 상속되도록 레시피 보강. 워크트리 준비 시 `.mam.env` 복사/링크 항목 명시. | 누락 시 `$WT` 기준의 고아 monitor 가 기동되거나 MQTT 비밀정보가 누락될 위험 차단. | +| **§4 (기대 효과와 한계)** | "다중 루프 동시 기동"을 2단계(1단계: 병렬 구현 + 순차 루프 검증, 2단계: 다중 루프 동시 기동)로 명확히 분리 서술. | `loop-checkpoint.json` 및 단일 `loop-guard-active` 파일 충돌로 인한 즉시 결함 방지. | +| **§5 (필요한 변경)** | **§5-6 추가**: 리뷰어 테스트 실행 격리 규칙 추가 (`PYTHONDONTWRITEBYTECODE=1`, `-p no:cacheprovider`, 임시 디렉터리 기반 테스트). | 리뷰어가 `$WT` 에 빌드 아티팩트나 캐시 파일을 남겨 커밋을 오염시키는 문제 방지. | +| **§5 (필요한 변경)** | **§5-7 추가**: 워크트리 종료 헬퍼 `scripts/worktree_remove.sh` 추가 (`multi-agent-mux-stop --purge` 후 `git worktree remove`). | 워크트리 삭제 후 `agent-sessions.yaml` 에 죽은 경로의 세션 행이 잔류하는 문제 방지. | + +--- + +## 5. Final Verdict + +The proposal accurately diagnoses the architectural limitation and provides a high-leverage, elegant solution. The recommended supplements are non-breaking and address operational robustness. + +[VERDICT: PASS] diff --git a/docs/design/reviews/review-claude-e89f62d5.md b/docs/design/reviews/review-claude-e89f62d5.md new file mode 100644 index 0000000..6f980c2 --- /dev/null +++ b/docs/design/reviews/review-claude-e89f62d5.md @@ -0,0 +1,160 @@ +# 🔍 Design Review — Job e89f62d5 (워크트리 운영 모델 제안, `design/worktree-model` @ `fe96db1`) + +- **Reviewer**: `planner-reviewer-claude-01` (role: planner,reviewer) · 2026-09-14 +- **대상**: `docs/design/worktree-creator-model.md`(워크트리 `multi-agent-mux-wt/design-worktree-model`, 커밋 `fe96db1`, 1파일 +86). + - 본 체크아웃은 이 잡 내내 `main` @ `5611859`(v4.5.0 코드)에 있다. + - 아래의 코드 근거(파일:라인)는 모두 이 `main` 기준이다. +- **방법**: + - 문서를 정독했다. + - 문서가 기대는 코드 경로를 create·status·monitor·stop·resume·delegate-job·loop 러너 순으로 추적했다. + - scratchpad 임시 git 저장소에서 실험 1건을 했다(gitignore 와 symlink). + - 문서·코드는 수정하지 않았다. 체크아웃 전환, 워크트리 조작, 실제 `.mam`·세션 조작도 없었다. + +## 0. 요약 + +| 항목 | 판정 | 핵심 | +|---|---|---| +| §6-Q1 두 문제 해소 | **보완** | 커밋 격리는 해소된다. 병렬성은 "루프 밖 creator 병렬 + 리뷰 루프 1개" 로만 가능하다(루프 lock 이 두 번째 루프를 거부). 놓친 경로 5건(§2) | +| §6-Q2 / 브리프 (1) 제어 평면 env override | **보완** | status·monitor·stop 은 행의 `pane.cwd` 를 존중해 안전하다. 그러나 resume 은 `--workspace $WT` 가 필수이고, 모니터 데몬이 워크트리 코드로 뜰 수 있다 | +| 브리프 (2) 주입 문구 상대경로 | **동의 + 범위 확대** | brief 경로뿐 아니라 `--registry-dir .mam/jobs` 와 Output Report Path 도 상대경로다. 이대로면 creator 의 `started` 가 실패한다(rc 1 `agent_blocked`). §5-1 은 첫 사용 전 필수다 | +| 브리프 (3) 러너 diff·`base_commit` | **동의 + 구체화** | `runner.py:397-411`, `:564-565` 는 모두 `workspace_root` 기준이다. `--worktree` 는 diff·base 만 바꾸고 wrapper·레지스트리·체크포인트는 본 체크아웃에 두어야 한다 | +| 브리프 (4) `.mam` 경합·§5-4 | **보완** | 문서의 "경고" 가 아니라 **거부**다(`run_loop.sh:189-203`). 1단계는 §5-4 없이 시작할 수 있다 | +| 브리프 (5) 놓친 실패 경로 | **보완** | `.venv` symlink 가 무시되지 않음(실험), `.pytest_cache` 미무시, 리뷰어가 creator 의 살아 있는 트리에서 검증, 워크트리 모니터 데몬, 워크트리 삭제 뒤 resume 불가 | +| §6-Q3 / 브리프 (6) A/B/C 평가 | **동의 + 대안 2건** | C 가 맞다. 추가로 ① 본 체크아웃 커밋 가드, ② 리뷰마다 detached 워크트리 | +| §6-Q4 §5 누락·과잉 | **보완** | §5-1·§5-2·§5-3 범위를 넓히고 3건을 추가한다. §5-4 는 보류해도 된다 | +| §6-Q5 / 브리프 (7) #17 항목 4 순서 | **조건부 동의** | 레시피·문서 먼저. 단 §5-1·§5-2 두 코드 변경은 지금 한다 | + +**결론**: 근본 결함은 없고 C 안은 채택 가능하다. 다만 §7 의 문서 수정과 §5 보강이 첫 적용 전에 필요하다. + +## 1. 브리프 (1) · §6-Q2 — 제어 평면 env override 가 스킬마다 존중되는가 + +| 구성요소 | override 존중 | 워크트리 `pane.cwd` 행 처리 | 근거 | +|---|---|---|---| +| `lib.sh` | `WORKSPACE_ROOT`(`:26`)와 `AGENT_SESSIONS_YAML`(`:34`)을 env 우선으로 쓴다 | — | `lib.sh:26`, `:34` | +| **create** | yaml·레지스트리 ✅(`create_session.sh:353`, `:372-386`) | pane cwd = `--workspace`(`:219`, `:223`, `:229`)이고 행에 기록된다(`:309`, `:428`, `:494`). herdr 서버는 `--herdr-session` 을 주지 않으면 **워크트리 슬러그**가 된다(`:163-172`) | 아래 ⚠️1 | +| **status** | yaml ✅(`status.sh:22`) | claude 대화 키를 **행의 cwd** 로 계산한다 ✅(`:59-76`) | 안전 | +| **monitor (reconcile)** | yaml·`WORKSPACE_ROOT` ✅ | herdr 서버를 **행마다** 모아 조회한다(`reconcile.sh:491-500`). 다른 서버의 워크트리 행도 관측되므로 거짓 drift A 가 없다. drift B 자동 등록은 본 체크아웃 아래 pane 만 한다(`:651-653`). C0 검증은 행 cwd 를 쓴다(`:727-730`, `verify_session.py:90-93`) | 안전 | +| **stop** | yaml ✅, purge 마커는 본 체크아웃 `.mam`(`stop_session.sh:103`) | uuid 캡처·purge 가 **행 cwd** 를 쓴다(`:133-143`, `:189`). herdr 소켓은 행의 `herdr_session` 을 먼저 쓴다(`lib.sh:1484-1490`) | 안전 | +| **resume** | yaml ✅ | `--workspace` 가 필수다(`resume_session.sh:42`). 행을 `cwd == --workspace` 로 찾는다(`workspace_uuid.py:58`). claude 키는 워크트리 경로다(`verify_session.py:75-81`) | ⚠️2 | +| **delegate-job 래퍼** | `.mam.env` 는 `REPO_ROOT`(= 본 체크아웃, `:20-24`) ✅. **`MAM_JOBS_DIR` 은 참조하지 않는다**. `REGISTRY_DIR_DEFAULT=".mam/jobs"`(`:71`) | 주입 문구·브리프가 상대경로다 | §3 에서 다룬다 | +| **loop 러너** | `workspace_root` = `MAM_REAL_ROOT`/`WORKSPACE_ROOT`/cwd(`runner.py:58`) | — | §4 에서 다룬다 | + +- ⚠️1 **워크트리 모니터 데몬**: + - create 는 `start_watchdog "$DELEGATE_JOB_ID" "$WORKSPACE"`(`create_session.sh:527`)를 부른다. + - 그러면 데몬이 `/.agents/skills/…/reconcile.sh`(`daemon.py:35-36`, `:63-85`)로 뜬다. 중복 판정이 **스크립트 경로 기준**(`:39-55`)이라 본 체크아웃 데몬을 보지 못한다. + - `WORKSPACE_ROOT=$MAIN` 이 상속되면, 이 데몬은 본 체크아웃의 `monitor.lock` 을 얻지 못해 곧 끝난다. + - 그러나 **본 체크아웃 데몬이 없던 순간**이면 이 데몬이 워크스페이스 모니터가 된다. 그러면 제어 평면이 **이슈 브랜치의 미검토 코드**로 돈다. + - §3 레시피에 `MAM_NO_SUBSCRIBER_DAEMON=1`(#19 옵트아웃)을 넣어야 한다. +- ⚠️2 **resume**: + - `--workspace $WT` 로 불러야 워크트리 행과 대화를 찾는다. `$MAIN` 을 주면 행을 찾지 못하거나 pane 이 본 체크아웃 cwd 로 떠 대화 키가 어긋난다. + - **워크트리를 지우면 대화를 재개할 수 없다**(키가 경로다). 같은 경로로 워크트리를 다시 만들어야 한다. + - 문서 §2-C 의 "끝나면 정지(대화 보존, 필요 시 재개)" 에 "재개하려면 워크트리를 유지하거나 같은 경로로 재생성" 을 덧붙여야 한다. +- **pane 런타임 env** (확인 권고): + - create 는 herdr 에 `CMD_FULL` 만 넘긴다(`create_session.sh:219`). create 호출자가 export 한 override 가 **에이전트 프로세스의 env** 에 남는지는 herdr 서버 동작에 달렸다. + - 남지 않으면 creator 가 워크트리에서 `bash .agents/skills/…` 로 부르는 스킬은 **워크트리의 `.mam`** 을 제어 평면으로 쓴다(`lib.sh:26`). + - 브리프에 "워크트리에서 제어 평면 스킬을 실행하지 말 것" 을 넣거나, `CMD_FULL` 을 `env WORKSPACE_ROOT=… MAM_JOBS_DIR=…` 로 감싸야 한다. + +## 2. §6-Q1 · 브리프 (5) — 두 문제의 해소와 놓친 경로 + +- **체크아웃 전환 사고**: 커밋하는 주체(creator, 오케스트레이터)가 모두 워크트리에서 커밋하므로 **해소된다**. + - 다만 본 체크아웃에서 누구든 `git checkout` 할 수 있다는 점은 그대로다. + - 싸고 직교하는 방어로 **본 체크아웃 커밋 가드**를 함께 권한다: `pre-commit` hook 이 `git branch --show-current` 를 확인해 거부한다. +- **병렬성**: 루프 lock 이 같은 `.mam` 에서 두 번째 루프를 **exit 1 로 거부**한다(`run_loop.sh:144`, `:189-203`, loop SKILL.md `:235`). + - 따라서 C 안의 1단계 병렬은 다음 형태뿐이다: **creator 구현은 루프 밖(delegate-job direct 나 대화)에서 병렬로, 리뷰 루프는 한 번에 하나**. + - §4 와 §6-Q4 에 이 운영 형태를 명시해야 한다. +- **놓친 실패 경로**: + 1. **리뷰어가 creator 의 살아 있는 워크트리에서 검증**한다(§2-C "그 경로에서 `git diff`·테스트"). + - creator 가 리뷰 중에 커밋·편집하면 검증 대상이 바뀐다. #18·#19 에서 리뷰 중 HEAD 이동이 세 번 있었다. + - 리뷰어의 테스트 산출물이 creator 트리에 남는다. + - → 리뷰마다 **detached 워크트리**(`git worktree add --detach "$TMP/review-" `, 리뷰 뒤 제거)로 검증 대상을 불변으로 만들 것을 권한다. 새 에이전트 인스턴스가 필요 없는 C 안의 보강이다. + 2. **§3 의 `.venv` symlink 가 git 에서 무시되지 않는다.** 실험(임시 저장소, `.gitignore` 에 `.venv/`): symlink `.venv` 는 `?? .venv` 로 보인다. 디렉터리 전용 패턴은 symlink 에 맞지 않는다. + - 그래서 creator 의 `git add -A` 가 symlink 를 커밋한다. + - 또 저장소 `.gitignore` 에 `.pytest_cache` 가 없어서(`.gitignore:14-25` 에 `.mam/`·`.venv/`·`__pycache__/`·`.mam.env*` 만 있음), `-p no:cacheprovider` 없이 돌린 pytest 도 산출물을 남긴다. + - → 워크트리의 `.git/info/exclude` 에 `/.venv`·`.pytest_cache/` 를 넣는 것을 준비 헬퍼(§5-3)에 포함한다. + 3. **워크트리 모니터 데몬**(§1 ⚠️1). + 4. **워크트리 삭제 뒤 남는 yaml 행**: herdr 가 죽으면 reconcile 이 drift A 로 `terminated` 표기한다(`reconcile.sh:581-600`). 행은 남지만 대화는 재개할 수 없다. → 해체 순서를 문서화한다: **stop → (재개 불필요 확인) → 워크트리 제거**. + 5. **토픽 지문·HMAC**: 문제없다. + - 잡 토픽은 레코드의 `topic_prefix`(`python/mqtt/jobs/`, `mqtt_common.py:137`, `:351`)이고, HMAC 은 잡 레코드의 `auth_token` 이다. 레지스트리 경로만 맞으면(§3) 워크트리와 무관하다. + - 지문은 **모니터 구독**에만 쓰인다(`reconcile.sh:288-291`). 제어 평면이 하나면 지문도 하나다. + - 문서 §1 표의 해당 행에 이 점을 적어 둘 것. + 6. (부수) 러너는 리포트를 `workspace_root/.agents/reports/…` 로 보관한다(`runner.py:554`, `:770-780`). C 안에서는 본 체크아웃, 즉 `main` 트리로 간다. 이슈 브랜치로 보낼지 정해야 한다. + +## 3. 브리프 (2) — 주입 문구의 상대경로 (§5-1) + +- **동의, 그리고 범위가 더 넓다.** 래퍼가 에이전트에 주는 세 경로가 모두 cwd 상대다. + +| 문자열 | 위치 | 워크트리 cwd creator 에게 | +|---|---|---| +| 브리프 `.mam/jobs/$JOB_ID/brief.md` | 래퍼 `:199`, `:455` | 없음(문서가 짚은 부분) | +| 발행 명령 `--registry-dir $REGISTRY_DIR`(기본 `.mam/jobs`, `:71`) | `:198`, `:454` | `$WT/.mam/jobs` 로 해석된다 → `job record not found`, rc 1. heartbeat 타이머도 상대 레지스트리·cwd 로 뜬다. **`started` 가 안 나가므로 T 뒤 `agent_blocked` 로 닫힌다** | +| Output Report Path `.mam/jobs/$JOB_ID/…/report-final.md` | `:178`, `:352` | 리포트가 `$WT/.mam/…` 에 쓰인다. 러너의 `find_report`(`runner.py:389-395`, 본 체크아웃 `.mam`)가 찾지 못한다 | + +- 래퍼가 `MAM_JOBS_DIR` 을 보지 않으므로 env override 로는 풀리지 않는다. +- → §5-1 을 "`REGISTRY_DIR` 을 절대경로로 정규화하고, 브리프·발행 명령·리포트 경로 세 곳 모두 절대경로로" 로 고치고, **첫 C 안 적용 전 필수**로 표시한다. + +## 4. 브리프 (3)·(4) — 러너 diff·`base_commit`, `.mam` 경합 + +- **(3) 동의**: + - `collect_changes_diff` 는 `workspace_root` 의 스크립트와 cwd 를 쓴다(`runner.py:397-411`). + - `base_commit` 은 `workspace_root` 의 `HEAD` 다(`:564-565`). 본 체크아웃이 `main` 이면 diff 는 비고 base 는 `main` 이다. 이번 #18 브리프들의 "(no changes since base commit)" 과 같은 증상이다. + - §5-2 의 `--worktree` 는 다음을 정확히 지정해야 한다: **바뀌는 것**은 diff 수집 경로, `base_commit`(워크트리에서 `git merge-base main HEAD`), 브리프에 넣는 경로·base·head 뿐이다. **그대로 두는 것**은 wrapper 경로(`:288`), 레지스트리, 체크포인트(`:60`), 리포트 보관이며 본 체크아웃의 것을 쓴다. +- **(4) 보완**: + - 문서 §4 는 "동시 루프 금지" 를 경고로 서술하지만, 실제로는 lock 이 **거부**한다(`run_loop.sh:189-203`). 체크포인트도 파일 하나다(`runner.py:60`). + - 세션 DB 는 SQLite 읽기이고 `timeout=10`(`runner.py:199-203`)이다. 쓰기 경합은 yaml 의 `atomic_dump_yaml`(flock) 쪽이다. + - 리뷰어 busy 는 rc 5 → `MAM_SESSION_BUSY_RETRY_S`(기본 30) 뒤 1회 재시도(`runner.py:312-322`) → 실패로 이어진다. 루프 둘이 같은 리뷰어를 두고 다투면 한쪽이 실패한다. + - **결론**: 1단계를 "리뷰 루프는 하나씩, creator 구현만 병렬" 로 시작하면 §5-4 는 필요 없다. 루프 여러 개를 동시에 돌리는 것은 §5-4(루프 id 별 marker·체크포인트, 리뷰어 대기 큐)를 끝낸 뒤로 미룬다. + +## 5. §6-Q3 · 브리프 (6) — A/B/C 평가 + +- **동의**: + - A 의 비용(에이전트 수·토큰 한도가 이슈 수에 비례, 문맥 초기화, 관측 분산) 평가는 타당하다. + - B 는 creator 가 여전히 본 체크아웃에서 브랜치를 옮기므로 #18 유형이 남는다는 판단이 옳다. + - C 가 쓰는 쪽만 격리하는 최소 변경이다. +- **대안·보강 2건**: + 1. **본 체크아웃 커밋 가드**(`pre-commit` hook 또는 커밋 명령 래퍼의 `git branch --show-current` 확인). 문제 2 를 C 와 무관하게 막는 가장 싼 방어다. C 채택과 함께 도입할 것. + 2. **리뷰마다 detached 워크트리**(§2-1). 리뷰어 풀은 공유하되 검증 대상은 불변으로 만든다. A 의 격리 이점을 에이전트 증설 없이 얻는다. + +## 6. §6-Q4 · §6-Q5 — §5 작업 항목과 #17 항목 4 순서 + +- **§5 수정 제안**: + - §5-1: 범위 확대(§3). 첫 적용 전 필수. + - §5-2: `--worktree` 의 영향 범위 명시(§4). 첫 적용 전 필수. + - §5-3: 준비 헬퍼에 다음을 포함한다. + - `MAM_NO_SUBSCRIBER_DAEMON=1` + - `.git/info/exclude`(`/.venv`, `.pytest_cache/`) + - `--herdr-session <본 서버>` 고정 + - `.mam.env` 는 워크트리 스크립트를 쓸 때만 필요(래퍼·`mqtt_common` 은 스크립트 위치 기준으로 찾는다: 래퍼 `:20-24`, `mqtt_common.py:54-65`) + - §5-4: 보류해도 된다(§4). + - **추가**: + - §5-6: 워크트리 creator 의 stop·resume 레시피와 해체 순서(resume 은 `--workspace $WT`, 삭제 전 stop). + - §5-7: 리뷰어 detached 워크트리 규약. + - §5-8: 본 체크아웃 커밋 가드. +- **§6-Q5 (조건부 동의)**: + - 이 모델을 문서·레시피로 먼저 굳히고 #17 항목 4(셸 계층 축소)에서 코드로 흡수하는 순서가 맞다. + - 단 §5-1(래퍼 경로)과 §5-2(러너 옵션)는 작은 변경이고 **없으면 C 안이 동작하지 않으므로** 지금 한다. 항목 4 가 래퍼를 `lib_py` 로 옮길 때 그대로 따라가면 된다. + - 헬퍼(§5-3)와 큐잉(§5-4)은 항목 4 안에서 구현하는 편이 중복을 줄인다. + +## 7. 문서 수정 지점 (절·문장) + +- **§1 표** "모니터 토픽 지문" 행: "(모니터 구독에만 쓰인다. 잡 토픽·HMAC 은 잡 레코드의 `topic_prefix`·`auth_token`)" 을 덧붙인다. +- **§2-C** "creator = 워크트리 전용 인스턴스 … 끝나면 정지(대화 보존, 필요 시 재개)" 에 "재개는 같은 워크트리 경로가 있어야 한다" 를 추가한다. +- **§2-C** "제어 평면 = …": 모니터 데몬 옵트아웃과, pane 런타임 env 에 override 가 남지 않을 수 있다는 점을 추가한다. +- **§2-C** "리뷰어 풀 = …": 리뷰어는 creator 의 워크트리가 아니라 head sha 의 detached 워크트리에서 검증한다고 고친다. +- **§3 레시피**: + - `MAM_NO_SUBSCRIBER_DAEMON=1` 을 추가한다. + - `ln -s .venv` 옆에 `.git/info/exclude` 처리를 적는다. + - stop·resume 명령 예시를 추가한다. + - "주입 문구의 상대경로 …" 문장을 발행 명령의 `--registry-dir` 와 Output Report Path 까지 넓힌다. +- **§4** 세 번째 항목: "경고" 를 "lock 이 두 번째 루프를 거부(`run_loop.sh:189-203`)" 로 고친다. 1단계 운영 형태(구현 병렬, 리뷰 루프 1개)를 명시한다. +- **§5**: 1·2·3 을 수정하고 6·7·8 을 추가하며, 4 는 "보류 가능" 으로 표시한다(§6). + +## 8. 종합 + +- C 안(쓰는 쪽만 워크트리, 읽는 쪽 공유, 제어 평면 하나)은 두 문제에 대한 올바른 방향이다. +- status·monitor·stop 이 행의 `pane.cwd` 와 행별 herdr 서버를 이미 존중하므로 현재 코드와도 대체로 맞는다. +- 보강이 필요한 곳: + - 첫 적용 전 필수: 래퍼 경로 3종의 절대경로화, 러너 `--worktree` 범위, 모니터 데몬 옵트아웃, `.venv` symlink exclude + - 운영 규약: resume·해체, 리뷰어 detached 워크트리, 커밋 가드 +- 모두 문서 보강과 작은 코드 변경으로 해결되며 설계를 뒤집을 결함은 아니다. + +[VERDICT: PASS] diff --git a/docs/design/reviews/review-codex-7770259b.md b/docs/design/reviews/review-codex-7770259b.md new file mode 100644 index 0000000..f10aa58 --- /dev/null +++ b/docs/design/reviews/review-codex-7770259b.md @@ -0,0 +1,75 @@ +# Design review — creator worktrees and a shared reviewer pool + +Reviewed proposal `fe96db1`, `docs/design/worktree-creator-model.md`, through `git show design/worktree-model:...` and the named design worktree. That worktree's HEAD is `fe96db1`; the source checkout remains on `main` at `5611859`. Both working trees were clean. The document-only diff passes `git diff --check`. + +**Verdict: NOT PASS as written. C is a viable direction, but the current-code recipe overstates control-plane compatibility and leaves review identity and lifecycle ownership underspecified. Revise those contracts before adopting the recipe. This does not require abandoning C.** + +## Answers to the five questions in §6 + +### 1. Role separation — 보완 (agree with the direction; strengthen the execution contract) + +Dedicated creator checkouts prevent an ordinary branch switch in the main checkout from changing a creator's HEAD, and allow independent implementation work. Sharing reviewer conversations is also reasonable. However, §2-C's sentence “읽기 전용” is not true of executing tests in the creator's checkout: tests, build systems and package installers can write tracked files, generated files, caches and dependencies. Multiple reviewers can interfere with each other, and with the creator, even without editing source deliberately. + +Replace “리뷰어는 그 경로에서 ... 테스트를 실행한다” with an explicit choice: + +- Freeze the creator at a committed head SHA for the whole review round, serialize tests that share writable outputs, and use isolated output/config directories; or +- Keep the reviewer conversation pool, but give each test execution a disposable checkout/worktree of that exact SHA and isolated outputs. Reviewer process/session location does not have to equal test execution location. + +The example in §3 says `git diff ...HEAD`, whereas §2-C mentions a fixed head. Use an immutable `head_sha` consistently, record it in every verdict, and invalidate approval after a new creator commit. A diff against a SHA followed by tests against a changing working directory is not evidence for that SHA. Define how uncommitted changes are handled; preferably require a clean, committed review snapshot. + +The statement “편집·커밋하는 유일한 에이전트” also needs qualification: the orchestrator commits, and the existing loop promotes reports into the source checkout (`lib_py/loop/runner.py:770`, `:780`). Separate product-code ownership, report artifacts and orchestration changes. A pre-commit branch-name check helps but is not an atomic exclusion mechanism. Require expected checkout/branch/head checks and one owner for each writable source tree. + +### 2. Shared control plane through environment overrides — 반대 to the present blanket claim + +The four variables are not consistently honored across all entry points. The narrow recipe running main-tree scripts from main may happen to align several defaults, but that is not a general split-root contract. + +| Component | Code evidence and assessment | +|---|---| +| Common shell roots | `.agents/skills/lib.sh:26` honors `WORKSPACE_ROOT`; `:35` honors `AGENT_SESSIONS_YAML`. This supports a separate session registry for the command invoking the library. | +| Create | `multi-agent-mux-create/scripts/create_session.sh:219` spawns with `--workspace` as cwd; `:386` writes through the chosen YAML path, and `:428` records the actual pane cwd. Thus registration and cwd can differ. **But pane environment forwarding is not established by exporting variables to the create command.** `CMD_FULL` is an agent command (`:193`), and `lib.sh:667–680` generates pane `--env` flags only from assignments embedded in that command. No explicit forwarding of these four variables is present. A persistent herdr server/pane cannot be assumed to inherit each client's environment. Persist and explicitly pass the control-plane binding during create and resume. | +| Status | `multi-agent-mux-status/scripts/status.sh:59–77` uses each row's cwd for Claude transcript lookup, which is appropriate. Job lookup is different: `:29` derives `PROJECT_ROOT` from the script location; `:357–377` searches cwd/script-relative paths rather than `MAM_JOBS_DIR` or `DELEGATE_JOB_LOGS_DIR`. Calling the worktree copy against central state can therefore show an unknown/wrong job location. | +| Monitor/reconcile | `multi-agent-mux-monitor/scripts/reconcile.sh:117` anchors the monitor lock on `WORKSPACE_ROOT`, but `:239–243` computes the event registry from that root and ignores `MAM_JOBS_DIR`. Existing registered rows use their `pane.cwd` for UUID checks (`:727–757`); an external worktree cwd does **not by itself** mark an existing row as drift. However, discovery of a missing row explicitly rejects cwd outside the root at `:651–654`. Sibling worktrees in the example cannot be rediscovered through this path. | +| Monitor startup | `create_session.sh:526–527` passes the worktree to reconcile/watchdog startup. `lib_py/delegation/daemon.py:98–110` identifies the daemon by its worktree script, and `:70–76` puts its log/cwd there. This is not automatically one central daemon with a central lifetime, even if a central lock prevents duplicate active subscribers. Specify one central daemon owner and suppress worktree-local auto-starts. | +| Stop | `multi-agent-mux-stop/scripts/stop_session.sh:133–143` obtains the row cwd; `:151` honors the job-root override, and `:204–208` captures conversation identity using that cwd. These support the proposed split when the correct environment and server/row identity are supplied. | +| Resume | `multi-agent-mux-resume/scripts/resume_session.sh:42` requires `--workspace`; `:51–52` resolves identity using that argument and `:165` spawns there. It does not automatically substitute the row cwd. Resume must use the original worktree path and persisted control-plane binding. Claude discovery is cwd-keyed (`lib_py/agents/adapters/claude.py:117–118`) and verification checks cwd (`:78`). Deleting or renaming a worktree needs an explicit recovery policy. | +| Delegate job | Python defaults honor both job/log overrides (`lib_py/delegation/mqtt_common.py:152–186`). The shell wrapper instead defaults `REGISTRY_DIR` to relative `.mam/jobs` (`multi-agent-mux-delegate-job:71`, `:103`), changes cwd (`:153`), and passes that explicit registry argument (`:157`), overriding Python's default selection. | +| Loop | `lib_py/loop/runner.py:58–60` prefers `MAM_REAL_ROOT` and derives `.mam` from it. It honors `AGENT_SESSIONS_YAML` (`:195`), but job waiting/report lookup use `self.mam_dir/jobs` (`:348`, `:391`), not `MAM_JOBS_DIR`. The shell freeze path resets roots to its repository (`multi-agent-mux-loop/scripts/run_loop.sh:134–135`). | + +Amend §2-C's “... 넘기면 ... 한 곳에 모인다” and §3's “현재 코드 기준” to describe **partial existing support plus required corrections**, not an already-safe environment recipe. Extend §5-3 beyond documentation to explicit propagation, persistence and a create/status/reconcile/stop/resume/delegate/loop compatibility matrix. + +### 3. A/B/C costs and alternatives — 보완 + +C offers the best initial reuse of long-lived reviewer context while isolating writers. B leaves the principal creator checkout risk and gives little implementation parallelism. A provides stronger runtime separation at greater setup cost, but “완전 격리” is too strong: git worktrees still share repository metadata/refs, and shared `.venv`, caches, services and credentials remain shared resources. “현재 코드가 그대로 동작한다” needs qualification for worktree preparation and lifecycle behavior. + +The costs are capacity estimates, not established laws: five fresh agents per issue are a policy choice, and token use depends on actual work, not just the number of live sessions. Amend §2-A accordingly. A better variant of C is **persistent reviewer conversations with disposable per-review execution directories**, rather than requiring a fresh reviewer conversation for filesystem isolation. + +### 4. Missing/excess work and concurrency sequencing — 보완; start with one loop + +§5-1 and §5-2 identify real blockers, but require broader definitions: + +- Absolute paths must include brief, report output and every lifecycle command's `--registry-dir`, with proper shell quoting. Current wrapper lines `:178`, `:199`, `:352`, `:455` emit relative report/brief paths. Merely changing the brief path leaves the start command reading the wrong relative registry from a creator's worktree. +- Introduce separate **control root**, **source/worktree root**, **runtime-code root**, and a review identity `(loop_id, base_sha, head_sha)`. Preserve that binding in jobs/checkpoints and verify it on resume. `runner.py:399–408` collects diffs from the control workspace today, and `:564–565` chooses the base there. Report promotion, discovery and cleanup also need intentional root ownership; do not globally redirect `WORKSPACE_ROOT` to fix diff collection. +- §5-4 must account for the existing single-loop guard: `run_loop.sh:144`, `:188–200` rejects a second loop with the same marker before reviewer scheduling. This is an enforced boundary, not just the SKILL warning. Do not bypass it by assigning marker names alone. +- Checkpoints share one filename (`runner.py:60`); atomic replacement (`loop/checkpoint.py:18–30`) prevents torn files but not last-writer replacement, and successful completion removes that same file (`runner.py:801`). Namespacing, recovery ownership and cleanup must be designed together. +- SQLite/YAML corruption is not an automatic consequence of two local writers: `lib_py/atomic_yaml.py:59–86` already uses flock, busy timeout and `BEGIN IMMEDIATE`. Distinguish shared logical loop state and agent ownership from DB transaction safety, and test supported filesystem semantics instead of treating “NFS lock shadowing” as a complete explanation. +- Busy handling is not a queue: `runner.py:313–328` sleeps and retries once. A scheduler needs exclusive reviewer assignment/lease, fair bounded waiting, release on failure, cancellation and crash recovery. Additional reviewers need explicit assignment policy; `--all-reviewer` otherwise includes the whole running reviewer pool. + +A safe first milestone is one guarded loop at a time, with separately delegated implementation jobs in distinct creator trees and queued review entry. The current loop itself does not become a multi-creator scheduler merely through a recipe. Defer multi-loop concurrency until its ownership contract and tests exist. + +### 5. Order relative to #17 item 4 — 동의 with a staged rollout + +Agree with first specifying the root/identity contract, then absorbing it into the shell-layer reduction. Do not bless the current recipe as safe while deferring its correctness gaps. First revise this design, then implement the minimal absolute-path/propagation/diff-root fixes and test one-loop operation; use #17 item 4 to consolidate those semantics rather than creating a second set of shell-specific routing rules. Multi-loop scheduling follows separately. This assessment uses the proposal's description of #17 item 4; it does not claim an independent review of the remote issue discussion. + +## Additional failure paths to add to §5 + +1. **Worktree retirement:** stop/capture the creator first, retain its cwd/UUID and source SHA, finish/cancel owned jobs, then remove its execution tree through an explicit lifecycle. A stale YAML row is not evidence that a removed worktree can still resume. Preserve resumable worktrees or define recreation at the same path and revision. +2. **Test writes and environment isolation:** tests must not inherit production control-plane overrides. Give test registries, logs, monitor settings, ports and temporary directories isolated values. Shared `.venv` is only safe if dependency mutation is prohibited; dependencies differing by issue require separate environments. The proposed one-line preparation helper must validate preexisting paths/submodules rather than imply universal isolation. +3. **Central state is not a security boundary:** shared job credentials permit participation across these local processes. `mqtt_common.py:137`, `:351–352` currently use `python/mqtt/jobs/`; the monitor also subscribes to a root fingerprint and the legacy wildcard (`reconcile.sh:288–294`). Correct §2-C/§1 so it does not imply every job topic is fingerprinted by the worktree/control root. Preserve one canonical control identity, per-job HMAC and dispatch/attempt checks. Use an explicit central `MAM_ENV_FILE` policy instead of copying credentials into every tree by default. + +## Validation and adoption gate + +This was a read-only design/code-trace review. No create/resume/stop/status/monitor command, test suite, checkout switch or worktree mutation was run. No runtime compatibility is claimed from static tracing. The named document and commit were verified; only this report and authorized lifecycle events were written. + +Before adopting C, revise the cited sentences in §§2–5 and add acceptance cases for: sibling-worktree creation with a preexisting herdr server; explicit pane environment propagation; correct central brief/start/report routing; main remaining unchanged; fixed-SHA review under concurrent creator activity; row recovery and resume at the original cwd; isolated test state; and rejection of a second loop until a scheduler is implemented. These are targeted revisions within C, not a request to replace the design with A. + +[VERDICT: NOT PASS] diff --git a/docs/design/reviews/review-grok-2174eaaf.md b/docs/design/reviews/review-grok-2174eaaf.md new file mode 100644 index 0000000..9aee2d6 --- /dev/null +++ b/docs/design/reviews/review-grok-2174eaaf.md @@ -0,0 +1,69 @@ +# 🔍 Design Review — Job 2174eaaf (`worktree-creator-model.md` @ `fe96db1`) + +- **Reviewer**: `reviewer-grok-01` · 2026-09-14 +- **문서**: `multi-agent-mux-wt/design-worktree-model/docs/design/worktree-creator-model.md` (`git log -1` = `fe96db1`). 본 체크아웃은 `main`. +- **성격**: 설계 리뷰. 코드·문서 미수정, 체크아웃 미전환. + +제안 C(creator=워크트리, 리뷰어 풀=본 체크아웃, `.mam` 공유)는 두 문제(병렬 구현, 공유 체크아웃 `checkout` 사고)를 **채택 가능한 수준으로** 풀며, §5 에 빠진 실패 경로를 보완하면 구현에 들어갈 수 있다. + +## §6 질문 + +### 1. 역할 분리가 두 문제를 푸는가? — **동의 + 보완** + +- **병렬성**: creator 가 이슈별 cwd/브랜치를 가지면 구현은 동시에 진행된다. 리뷰어 풀은 잡 단위 직렬(`session_busy` rc 5 → `MAM_SESSION_BUSY_RETRY_S` 1회, `runner.py:313-314`). 문서 §4 서술과 맞다. +- **checkout 사고**: 본 체크아웃을 `main` 에 고정하고 오케스트레이터·creator 커밋을 워크트리에서만 하면, 공유 `checkout` 한 방으로 전원이 움직이는 경로는 사라진다. +- **놓친 경로 (문서를 고칠 곳: §4 한계 / §5 추가)**: + - 리뷰어가 `$WT` 에서 pytest 를 돌리면 `.pytest_cache`/`__pycache__`/실패 산출물이 워크트리에 남아 creator 가 dirty 로 본다. PYTHONDONTWRITEBYTECODE, `--basetemp` 를 WT 밖, 리뷰 후 `git status` 제약을 §5 에 적을 것. + - 워크트리 삭제 전 `stop` 없이 `worktree remove` 하면 yaml 행·herdr pane 이 남는다. 레시피에 stop → worktree remove 순서. + - 모니터 **자동등록** (`reconcile.sh:651-654`) 은 `pane.cwd` 가 `WORKSPACE_ROOT` 또는 그 하위일 때만 탄다. WT 가 `.../multi-agent-mux-wt/issue-20` 이면 MAIN 의 prefix 가 아니므로 **미등록 herdr 세션은 자동등록되지 않는다**. yaml 에 이미 있는 행은 이 필터를 타지 않는다. “create 레시피로만 등록”을 §3 에 명시할 것. yaml 행을 drift 로 지우지는 않음(이름 매칭). + +### 2. 제어 평면 env override 가 안전한가? — **동의 + 보완 (필수)** + +| 변수 | 근거 | C 안에서 | +|---|---|---| +| `WORKSPACE_ROOT` | `lib.sh:26-27` 이미 override | MAIN 으로 두면 yaml/jobs 기본이 MAIN | +| `AGENT_SESSIONS_YAML` | `lib.sh:34` | 명시하면 안전. 미설정 시 `WORKSPACE_ROOT/.mam/...` | +| `MAM_JOBS_DIR` / `DELEGATE_JOB_LOGS_DIR` | `mqtt_common.py:150-163`, `186-187` — `MAM_JOBS_DIR` 우선, 아니면 `WORKSPACE_ROOT/.mam` | `WORKSPACE_ROOT=MAIN` 만으로도 jobs 는 MAIN. 문서의 네 변수는 과잉이 아니라 방어. | + +- **status**: `status.sh:59-76` 이 **행의 `pane.cwd`** 로 claude 프로젝트 키를 만든다. WT cwd 행을 MAIN `WORKSPACE_ROOT` 와 혼동하지 않음. **동의.** +- **resume/stop**: `resume_session.sh:42,165` 은 `--workspace` 가 pane 생성 cwd. `verify_session_uuid` (`verify_session.py:90-93`) 는 `workspace_key(pane.cwd) != workspace_key(ws)` 이면 **즉시 False**. `find_workspace_uuid` (`workspace_uuid.py:57-59`) 도 pane.cwd 와 `--workspace` 일치만 본다. 따라서 resume/stop 의 `--workspace` 는 **MAIN 이 아니라 WT(pane.cwd)** 여야 한다. `WORKSPACE_ROOT`/yaml 은 MAIN. **§3 레시피에 이 이원성을 쓰지 않으면 resume 이 깨진다.** 고칠 곳: §3 코드블록 다음 문단. +- **claude 저장 키**: `claude.py:117-118` `workspace_key(ctx.workspace)` → `~/.claude/projects/`. create `--workspace WT` 이면 키가 WT. resume 도 WT 를 넘겨야 한다. 문서 표(§1)와 맞음. +- **토픽 지문**: `reconcile.sh:288-290` `sha256(abspath(WORKSPACE_ROOT))[:12]`. 모니터·creator 가 같은 `WORKSPACE_ROOT=MAIN` 이면 지문이 하나. HMAC/`auth_token` 은 job 레코드 단위라 공유 레지스트리와 맞음. 워크트리에서 override 없이 reconcile 을 돌리면 지문이 갈라진다 → 모니터는 항상 MAIN 에서. + +### 3. A/B 대비 C 비용·이득 — **동의**. 더 싼 1단계가 있다. + +- A: 이슈당 에이전트 5개·문맥 리셋·`.mam` 분산. 비쌈. 동의. +- B: 병렬 없음 + creator 가 본 체크아웃을 옮김. #18 유형 잔존. 동의. +- C: 구현 병렬 + 리뷰 문맥 유지 + 제어 평면 하나. 맞다. +- **대안 D (비차단)**: 리뷰어에게도 **같은 브랜치의 읽기 전용 워크트리**를 주면 MAIN 테스트 오염을 줄인다. 비용↑. 1단계 C 로 시작하고 D 는 선택. +- **대안 E** (훅으로 main checkout 금지): 병렬을 안 줌. C 를 대체하지 못함. + +### 4. §5 누락·과잉 / 다중 루프를 먼저? — **보완** + +| §5 | 판정 | +|---|---| +| 1 브리프 절대경로 | **필수, 범위 부족.** `multi-agent-mux-delegate-job:199` 주입이 `.mam/jobs//brief.md` 상대경로. creator cwd=WT 에는 없음. 같은 파일 `:178` **Output Report Path** 도 상대경로. 루프 `find_report` (`runner.py:389-394`) 는 `self.mam_dir/jobs/` = MAIN. creator 가 WT 에 리포트를 쓰면 루프가 못 찾는다. **brief 와 report-final.md 모두 `$REGISTRY_DIR` 절대경로.** | +| 2 `--worktree` | **필수.** `collect_changes_diff` `runner.py:399-408` 과 `base_commit` `runner.py:564-565` 가 `cwd=self.workspace_root`(MAIN HEAD). 워크트리 브랜치 diff 가 아님. 인용 정확. | +| 3 create 레시피 | **필수.** resume `--workspace $WT` 이원성 추가. | +| 4 `.mam` 경합 | **두 `run_loop.sh` 전에 필수, 1단계에는 과잉 가능.** `loop-checkpoint.json` (`runner.py:60`)·`loop-guard-active` (`:427`) 는 파일 하나. 두 루프는 체크포인트를 덮어쓴다. **시작은 “루프는 하나, creator 잡은 delegate-job 으로만 병렬”.** 이슈당 루프를 동시에 돌리려면 §5-4 가 선행. §4 문장에 이 단계를 밝힐 것. | +| 5 브리프 필드 | 필수. | + +### 5. #17 항목 4 와의 순서 — **동의** + +운영 모델(C)을 문서·레시피로 고정한 뒤, 항목 4(셸 계층 축소)에서 `--worktree`·절대 브리프/리포트 경로를 코드로 흡수하는 순서가 맞다. 항목 4 를 먼저 하면 워크트리 cwd 가 다시 깨진다. + +## 브리프 (1)–(7) 요약 + +| # | 판정 | +|---|---| +| (1) env 일관성 | 동의. resume/stop `--workspace`=pane.cwd(WT). status 는 pane.cwd 사용. | +| (2) 상대 brief | 동의, 깨짐. 리포트 경로도 깨짐. | +| (3) diff/base | 동의, `runner.py:399-408`·`564-565`. | +| (4) 다중 루프 | 실위험. 1단계는 루프 하나. | +| (5) 실패 경로 | pytest 오염, yaml 잔행, 모니터 자동등록 prefix, 잘못된 WORKSPACE 로 resume. 토픽/HMAC 은 공유 `.mam` 이면 안전. | +| (6) A/B/C | 타당. D 는 후속. | +| (7) #17-4 | 문서 먼저. | + +채택 가능. 근본 결함 없음. 구현 전 §3·§5-1·§5-4 문구를 고치면 된다. + +[VERDICT: PASS]