GodopuandClaude Opus 5.5
d400add841
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
108 lines
12 KiB
Markdown
108 lines
12 KiB
Markdown
# Code Review Report — PR #82 (Issue #78) — Loop 3 Iteration 1 (`@b49314c3`)
|
||
|
||
- **Job ID**: `8eb66b73`
|
||
- **Reviewer**: `agy` (`herdr:reviewer-agy-01`)
|
||
- **Loop ID**: `loop-20261030T002908Z-7d0414`
|
||
- **Review Snapshot**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261030T002908Z-7d0414/1`
|
||
- **Commit Range**: `a3364e82e1f9e9c6ac504eef377e8dbefba02b8d..b49314c3d039027d3f9478cacecfda587fdc8abc` (11 commits)
|
||
- **Tree**: `d49ff60bbf89437e997f7da21e2fae8df570456b`
|
||
|
||
---
|
||
|
||
## 1. 확인한 것 (파일:줄)
|
||
|
||
### 1.1 `568385c7` 이후 `b49314c3` 변경분 검증 (R1: 다른 live reset의 fence 침범 금지, R2: 아직 존재하지 않는 행 레지스트리 fence)
|
||
- **`.agents/skills/lib_py/delegation/registry.py:97-136`**:
|
||
- `reset_marker(registry_dir)`(`97-117`)가 `<registry_dir>/.reset-in-progress` 마커 상태를 `"absent"`(파일 없음), `"stale"`(`pid` 숫자 아님 / 프로세스 종료 / `lstart` 불일치), `"live"`(동일 `pid` 및 `lstart`), `"unknown"`(파일 읽기 불가 `OSError`/`ValueError` 또는 현재 프로세스에 대한 `ps` 실패)으로 판별.
|
||
- 일반 잡 등록 경로(reset 미실행 시)에서는 `open(marker)`가 즉시 `FileNotFoundError`를 일으켜 `"absent"`로 반환되므로 **`ps` 서브프로세스 호출이 전혀 발생하지 않음**(성능 영향 없음).
|
||
- `reset_refusal(registry_dir, agent_session)`(`120-136`)은 `with registry_lock(registry_dir):`(`150`) 내부에서 호출되며, `"absent"`/`"stale"`이거나 `sessions=`에 해당 세션이 없거나 `MAM_RESET_OWNER_PID == fields["pid"]`(reset 자신의 `create_session.sh` 온보딩 잡)인 경우 통과시키고, `"unknown"` 및 `"live"`는 거부 사유를 반환.
|
||
- **`.agents/skills/lib_py/update.py:188-238`**:
|
||
- `jobs_dirs(workspace, state, env)`(`188-202`)가 `<ws>/.mam/jobs`, `MAM_JOBS_DIR` (`addressed_only=False`), 그리고 각 행의 `control_plane.mam_jobs_dir` (`addressed_only=True`)를 **디렉터리 존재 여부와 무관하게** `_real(c)` 기준으로 중복 제거하여 반환.
|
||
- `open_jobs(workspace, state, env)`(`205-238`)와 `fence_registries`(`reset.py:492-521`)가 동일한 `jobs_dirs()` 열거 함수를 공유하여 검사 대상과 fence 대상이 정확히 일치.
|
||
- **`.agents/skills/lib_py/reset.py:488-521, 887-900` 및 `.agents/skills/multi-agent-mux-reset/scripts/reset.sh:117-125, 150-175`**:
|
||
- `fence_registries`가 `jobs_dirs`의 각 디렉터리에 대해 `os.makedirs(d, exist_ok=True)`를 수행한 뒤(존재하지 않던 `control_plane.mam_jobs_dir`도 생성하여 fence), `with registry_lock(d):` 아래에서 `reset_marker(d)`를 먼저 확인.
|
||
- 이미 다른 live 또는 unknown reset(`held in ("live", "unknown") and fields.get("pid") != me`)이 fence를 보유하고 있으면 마커를 덮어쓰지 않고 `FenceBusy`를 발생시켜 거부(`refused ...`).
|
||
- 자신이 새로 쓴 디렉터리만 `fenced <realpath>`로 출력하고 `reset.sh`의 `FENCED` 배열에 기록하며, `_release_reset_lock`(`reset.sh:118-125`)은 `FENCED`에 기록된 경로 중 `pid == $$`인 마커만 제거하므로 다른 워크스페이스의 live reset이 잡은 공유 레지스트리 fence를 절대 뺏거나 지우지 않음.
|
||
|
||
### 1.2 제안 1–6 및 "함께 고칠 것" 대조표
|
||
|
||
| 항목 | 명세 요구사항 (Issue #78) | 구현 위치 (`파일:줄`) | 테스트 커버리지 | 상태 |
|
||
|---|---|---|---|---|
|
||
| 제안 1 | `multi-agent-mux-reset` 스킬 신설 (`SKILL.md`, `scripts/reset.sh`, `lib_py/reset.py`) | `.agents/skills/multi-agent-mux-reset/SKILL.md:1-48`, `scripts/reset.sh:1-349`, `.agents/skills/lib_py/reset.py:1-934` | `tests/test_issue78_reset.py`, `tests/test_issue42_skill_blocks.py` | 충족 |
|
||
| 제안 2 | 대상 선택 (`status: running` + `realpath(pane.cwd) == realpath(WS)` 만 포함; `stopped`/`terminated`, 다른 폴더/worktree, `orchestrator_uuids` 제외) 및 필터(`--session`, `--role`, `--agent`, `--all`) | `lib_py/reset.py:157-249` (`select_targets`), `reset.sh:29-68` | `test_select_targets_and_plan_cli` | 충족 |
|
||
| 제안 3 | 사전 확인 및 잠금(update admission -> `loop-guard-active` & `reset-in-progress` -> job registry fence -> open job / 호출자 자신 / 미기록 claude·grok ID 거부, `[y/N]` 프롬프트 및 `--yes`/`--dry-run`) | `reset.sh:81-208`, `lib.sh:198-214` (`mam_reset_admit`), `lib_py/reset.py:488-589`, `lib_py/delegation/registry.py:97-160`, `lib_py/delegation/remote.py:114-116` | `test_no_yes_non_tty_exits_3_without_touching_locks`, `test_dry_run_changes_nothing_in_mam_tree`, `test_precheck_refusals_change_nothing`, `test_registry_cli_is_refused_after_the_target_recheck`, `test_other_reset_cannot_take_a_live_fence` | 충족 |
|
||
| 제안 4 | `.mam/reset-spec.json` 원자적 기록(`0600`) 및 중단/시그널/실패(`exit 4`, `exit 5`) 후 재실행 시 이미 재생성된 에이전트 재-purge 방지 | `lib_py/reset.py:252-449` (`write_spec`, `read_spec`, `entries`), `reset.sh:210-348` | `test_spec_file_write_read_and_validation`, `test_entries_5_states_and_resumed_row_9b`, `test_mid_run_failure_and_rerun_resumes_from_spec`, `test_signal_interrupts_reset_preserves_spec_and_holds_lock_until_child_exits` | 충족 |
|
||
| 제안 5 | 에이전트별 순차 실행: ⓪ 대상 재확인(`target-check`) -> ① `stop_session.sh --reason reset` (post-discovered ID 포착) -> ② ID 일치 및 폴더 소유 검증(`id-owned`) -> ③ `stop_session.sh --reason reset --purge-conversation --yes` -> ④ `create_session.sh` | `reset.sh:218-323`, `lib_py/reset.py:645-736` (`target_check`, `record_id`, `target_id_owned`) | `test_full_reset_success_spec_ordering_table_and_loop_block`, `test_post_discovered_without_recorded_id_8b_and_exit_5`, `test_post_discovered_id_is_purged_only_when_it_verifies_for_its_folder` | 충족 |
|
||
| 제안 6 | 종료 코드 표(`0`, `1`, `2`, `3`, `4`, `5`) 및 결과 요약 테이블 출력 | `SKILL.md:31-40`, `reset.sh:325-348`, `lib_py/reset.py:739-782` | `test_exit_table_agrees_with_script`, `test_purge_refused_exit_4_preserves_stopped_row_and_rerun_completes`, `test_post_discovered_uncapturable_id_8b_ii` | 충족 |
|
||
| 함께 고칠 것 1 | `in_use_hints`에서 `lsof` holder PID·명령어 표시 (`opencode` 백그라운드 서비스 제외) | `.agents/skills/lib_py/agents/base.py:268-310`, `adapters/agy.py:136-138`, `adapters/opencode.py:259-273` | `tests/test_issue78_hints_codex.py::test_in_use_hints_*`, `test_stop_purge_refusal_prints_holders` | 충족 |
|
||
| 함께 고칠 것 2 | `codex` `purge_artifacts` 및 `residual_artifacts`에 `~/.codex/tui-thread-reference-capabilities/<uuid>` 포함 | `.agents/skills/lib_py/agents/adapters/codex.py:420-467` | `tests/test_issue78_hints_codex.py::test_codex_purge_removes_tui_thread_capability`, `tests/test_a4_adapter_contract.py::test_codex_purge_artifacts` | 충족 |
|
||
| 패키징·문서 | `deploy/install.sh`, `deploy/remove.sh`, `deploy/gitea-ci.yml`, `lib_py/update.py`, `README.md`, `README.ko.md`, `docs/SKILLS_SYNC.json` 반영 | `deploy/install.sh:23,121,164`, `deploy/remove.sh:12`, `deploy/gitea-ci.yml:69`, `lib_py/update.py:38`, `README.md:107`, `README.ko.md:107` | `tests/test_issue42_skill_blocks.py`, `tests/test_deploy_layout.py` | 충족 |
|
||
|
||
### 1.3 `INSTRUCTION.md` 기준 `SKILL.md` 줄 단위 대조
|
||
- `.agents/skills/multi-agent-mux-reset/SKILL.md:1-48`의 모든 문장을 `reset.sh` 및 `lib_py/reset.py`와 줄 단위로 대조함:
|
||
- 진입점 단일 호출(`bash .agents/skills/multi-agent-mux-reset/scripts/reset.sh ...`), 옵션(`--yes`, `--dry-run`, `--session`, `--role`, `--agent`, `--all`), 대상 선택 규칙, 확인 프롬프트, 사전 확인 거부 조건, 잠금·동시성 제어(`update` admission lock, `loop-guard-active`, `reset-in-progress`, job registry `.reset-in-progress` fence, `MAM_RESET_OWNER_PID`), `.mam/reset-spec.json` 재실행 복구, 순차 실행 단계(⓪~③ 및 `create_session.sh`), 건너뜀(`exit 4`) 및 중단(`exit 5`), 종료 코드(`0`~`5`), 플레이스홀더(`0.0.0-dev`) 모두 스크립트/파이썬 코드와 1:1로 일치하며 **SKILL.md에만 있고 코드로 강제되지 않는 규칙은 없음**.
|
||
|
||
### 1.4 macOS 이식성 확인
|
||
- `/bin/bash` 3.2 호환성: `reset.sh` 및 `lib.sh`에 `mapfile`, `readarray`, `declare -A`, `sed -i`, `stat -c`, `readlink -f` 미사용. `set -uo pipefail` 아래 빈 배열 확장 시 `${FENCED[@]+"${FENCED[@]}"}` 패턴(`reset.sh:120`)을 사용하여 bash 3.2 `nounset`에서도 안전함.
|
||
- Python 3.9.6 호환성: `lib_py/reset.py`, `lib_py/delegation/registry.py`, `lib_py/update.py` 모두 `Optional[...]` / `Tuple[...]` / `List[...]`(`typing`)만 사용하며 Python 3.10+ 전용 `X | Y` 타입 유니온이나 `match` 문을 사용하지 않음.
|
||
|
||
---
|
||
|
||
## 2. 실행한 테스트와 결과 (명령·passed/failed)
|
||
|
||
검증 경로 `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261030T002908Z-7d0414/1`에서 아래 명령을 실행함:
|
||
|
||
1. **셸 문법 검사**:
|
||
```bash
|
||
cd /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261030T002908Z-7d0414/1 && \
|
||
/bin/bash -n .agents/skills/multi-agent-mux-reset/scripts/reset.sh .agents/skills/lib.sh deploy/install.sh deploy/remove.sh
|
||
```
|
||
- 결과: **exit 0**
|
||
|
||
2. **Issue #78 및 Skill Blocks 테스트 스위트**:
|
||
```bash
|
||
cd /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261030T002908Z-7d0414/1 && \
|
||
env -u HERDR_SOCKET_PATH -u HERDR_PANE_ID -u HERDR_TAB_ID -u HERDR_WORKSPACE_ID -u HERDR_SESSION_NAME -u HERDR_SESSION -u WORKSPACE_ROOT -u MAM_ENV_FILE -u MAM_JOBS_DIR -u DELEGATE_JOB_LOGS_DIR \
|
||
PYTHONDONTWRITEBYTECODE=1 /Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.venv/bin/python -m pytest -p no:cacheprovider \
|
||
--basetemp=/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-pytest-8eb66b73 \
|
||
-v tests/test_issue78_reset.py tests/test_issue78_hints_codex.py tests/test_issue42_skill_blocks.py
|
||
```
|
||
- 결과: **84 passed, 1 skipped** in `699.23s` (exit 0)
|
||
- `b49314c3` 신규/보강 케이스 모두 통과:
|
||
- `test_registry_cli_is_refused_after_the_target_recheck[canonical/alias/custom/row-absent]` (`4 passed`)
|
||
- `test_fence_waits_for_a_registration_and_covers_every_registry_read` (`PASSED`)
|
||
- `test_other_reset_cannot_take_a_live_fence[False/True]` (`2 passed`)
|
||
|
||
3. **Deploy Layout / Adapter Contract / Purge 관련 스위트**:
|
||
```bash
|
||
cd /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261030T002908Z-7d0414/1 && \
|
||
env -u HERDR_SOCKET_PATH -u HERDR_PANE_ID -u HERDR_TAB_ID -u HERDR_WORKSPACE_ID -u HERDR_SESSION_NAME -u HERDR_SESSION -u WORKSPACE_ROOT -u MAM_ENV_FILE -u MAM_JOBS_DIR -u DELEGATE_JOB_LOGS_DIR \
|
||
PYTHONDONTWRITEBYTECODE=1 /Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.venv/bin/python -m pytest -p no:cacheprovider \
|
||
--basetemp=/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-pytest-8eb66b73-2 \
|
||
-v tests/test_deploy_layout.py tests/test_a4_adapter_contract.py tests/test_issue10_13_purge.py tests/test_issue31_purge_residual.py
|
||
```
|
||
- 결과: **128 passed** in `71.65s` (exit 0)
|
||
|
||
4. **작업 트리 상태 및 잔여 프로세스 확인**:
|
||
- 임시 `--basetemp` 디렉터리 정리 완료.
|
||
- `git status --short`: 빈 출력(깨끗함).
|
||
- `ps -ef | grep -E 'reconcile\.sh|reset\.sh'`: 본 체크아웃의 기존 모니터 데몬(`PID 3714`) 1개만 유지되고 테스트 잔여 프로세스 없음.
|
||
|
||
---
|
||
|
||
## 3. 발견 사항
|
||
|
||
### 차단 (Blocking)
|
||
- 없음.
|
||
|
||
### 비차단 (Non-Blocking)
|
||
- 코드 양 및 중복: `lib_py/reset.py:139-145`의 `_real()` 헬퍼와 `lib_py/update.py:140-144`의 `_real()` 헬퍼가 동일한 역할(`os.path.realpath(os.path.abspath(p))`)을 수행하므로 추후 리팩터링 시 공통 유틸로 합칠 수 있음(동작·안전성에는 영향 없음).
|
||
|
||
---
|
||
|
||
## 4. 사용자 확인 필요
|
||
|
||
- 이슈 #78 본문의 "정할 것" 3가지에 대해 작성자가 선택한 설계(① 모든 에이전트를 멈추기만 하고 새로 띄우지 않는 옵션 없이 순서대로 `create_session.sh`까지 수행, ② `grok` 메모리 관찰 기록 보존 옵션 없이 기본 purge 수행, ③ 이미 `stopped`인 행은 기본 대상에서 제외)가 의도와 맞는지 최종 확인 바랍니다.
|
||
|
||
[VERDICT: PASS]
|