Files
multi-agent-mux/.agents/reports/planner-reviewer-claude-01/report-1d2c21dc.md
T

133 lines
9.7 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# PR #75 후속 라운드 리뷰 — 1d63e88..ec7f5c4 (review round 5)
리뷰어: planner-reviewer-claude-01 (claude) · job 1d2c21dc
## 0. 리뷰 경로 확인
- 검증 경로: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261027T201245Z-63c71b/1`
- HEAD `ec7f5c4e57286fb3c4238b8220f39094f4640391`, tree `fbadb24de60ccdae684b23e2e0a41cdf23ab81eb` 로 [리뷰 identity] 와 같다.
- `git status --porcelain --ignored` 는 비어 있다. `.mam` 은 없다.
- 리뷰 전후로 확인했다. 본 체크아웃 main 은 `0e1e2e8` 그대로다.
- 범위: 1d63e88..HEAD 의 커밋 `ec7f5c4` 하나다.
- 바뀐 파일: `.agents/skills/lib.sh`(+7/−1), `tests/test_issue72_update.py`(+32/−3)
- 브리프가 정한 범위(R2 와 테스트의 `pkill` 제거) 밖은 건드리지 않았다.
## 1. R2 — legacy `start_watchdog` 의 프로세스 그룹 분리
### 1.1 코드 (`lib.sh:2297-2310`)
- 모니터를 띄우는 `nohup bash … &` 직전에 `set -m` 을 켠다.
- 켜기 전에 호출자의 `$-` 에 `m` 이 있었는지 `had_m` 에 기억한다.
- `pid=$!` 를 읽은 뒤, 원래 꺼져 있었으면 `set +m` 으로 되돌린다.
- job control 이 켜진 상태에서 띄운 백그라운드 잡은 자기 프로세스 그룹의 리더가 된다(PGID = PID). 그래서 다음 효과가 있다.
- `$!` 는 그대로 그 PID 다. nohup 과 bash 가 exec 해도 PID 는 바뀌지 않는다.
- 따라서 정확한 PID 보고(`echo "$pid"`, `:2314-2316`)와 `mam_monitor_started` 확인이 바뀌지 않는다.
- 다음 기존 동작은 바뀌지 않았다.
- 중복 방지(`_monitor_pids`, `:2291`)
- `MAM_NO_SUBSCRIBER_DAEMON=1` 분기(`:2295-2296`)
- stdin `/dev/null`, 로그 append(`>> "$log_file"`), 작업 공간 cwd, `--idle-timeout 0`
- 바뀐 것은 분기 안의 세 줄뿐이다.
- 호출하는 곳은 모두 `$(…)` 안이다. 그래서 `set -m` 은 그 서브셸 안에서만 잠깐 켜진다.
- `update.sh:106`, `post_update.sh:41`, `install_phase.sh:64`
- `create_session.sh:748`, `resume_session.sh:162`
### 1.2 이 Mac 에서 직접 확인 (`/bin/bash` 3.2, tty 없음, `set -euo pipefail`)
- 같은 모양의 함수를 스크래치 스크립트로 만들어 돌렸다.
- 결과 1: `$(f)` 안에서 띄운 자식의 PGID 는 자기 PID 와 같다(53727 = 53727). 호출자의 PGID(53714)와 다르다.
- 결과 2: 직접 호출한 경우도 마찬가지다.
- 결과 3: 호출 뒤 `$-` 는 `ehuB` 로 그대로다. 곧 `m` 이 남지 않는다.
- 결과 4: "no job control", "[1] PID", "Done" 같은 잡 메시지가 stdout 에도 stderr 에도 나오지 않았다. 그래서 `$(…)` 로 받는 PID 출력이 오염되지 않는다.
- **고치기 전 코드에서 실패하는지도 확인했다.**
- 방법: `git archive HEAD` 로 TMPDIR 에 사본을 풀었다. 그 사본의 `lib.sh` 만 1d63e88 판으로 바꿨다.
- 그 사본에서 `-k test_every_restart` 를 실행했다.
- 결과: **mode=0 의 3개(installed, killed, rolled-back)가 `left in the caller's group` 로 실패했다. mode=1 의 3개는 통과했다(3 failed, 3 passed).**
- 즉 새 assertion(`tests/test_issue72_update.py:723-724`)이 R2 를 실제로 잡는다. 수정 뒤 6개가 모두 통과한다(§3).
- 사본은 확인 뒤 지웠고, 남은 프로세스도 없다.
- 세 경로와 두 모드: 정상 설치, 정지 시간 초과 뒤 복원, rollback 뒤 복원을 mode 0·1 로 돌리는 6 조합이 모두 같은 함수를 지나고, 모두 통과한다.
- rollback 경로는 되돌린 트리의 `lib.sh` 를 쓴다. 테스트 상자에서는 그 트리가 이 버전이다.
### 1.3 python 경로와의 차이 (비차단, 참고)
- 두 경로가 분리하는 정도가 다르다.
- python 경로(`daemon.py:95`): `start_new_session=True` 로 새 **세션**을 만든다. 그래서 제어 터미널에서도 떨어진다.
- legacy 경로: 새 **프로세스 그룹**만 만든다. 세션과 제어 터미널은 호출자와 같다.
- 결정 2 의 요구("부른 쪽 프로세스 그룹에서 분리")는 이것으로 충족된다.
- 프로세스 그룹 신호(테스트 러너·timeout 래퍼의 killpg, 터미널의 SIGINT)는 이제 모니터에 닿지 않는다. 터미널 신호는 전경 그룹에만 가는데, 모니터는 배경 그룹이다.
- 터미널이 닫힐 때의 SIGHUP 은 nohup 이 무시한다.
- 배경 그룹이 tty 에 읽고 쓸 때 생기는 SIGTTIN/SIGTTOU 도 없다. stdin 은 `/dev/null` 이고 출력은 로그 파일로 간다.
- `lib.sh:246-253`(shim 서버 기동)은 같은 문제에 `start_new_session=True` 를 쓰는 관례를 적어 두었다. legacy 경로만 `set -m` 을 쓴다.
- 브리프가 "가장 작은 수정" 을 요구했고, legacy 경로는 opt-in 이므로 결함으로 보지 않는다.
## 2. 테스트의 `pkill` 제거 (`tests/test_issue72_update.py:184-194`, `:218-240`)
### 2.1 새 teardown 이 정리하는 것
- 패턴 kill 이 없어졌다. `tests/test_issue72_*.py` 전체에 `pkill`·`killall` 이 남아 있지 않다(grep 으로 확인).
- 대신 다음 순서로 정리한다.
1. 테스트가 `Box.helper`(`:132-135`, `start_new_session=True`)로 띄우고 `b.helpers` 에 기록한 프로세스의 그룹을 `killpg` 하고 `wait` 한다.
2. 이 테스트의 작업 공간 `ws` 에 대해 `bash <ws>/…/reconcile.sh --subscribe` 형태로 정확히 맞는 모니터(`_exact_monitor_pids`)와 그 자손(`_descendants`, `pgrep -P` 재귀)만 SIGKILL 한다.
- 남은 프로세스 확인: 명령줄에 `ws` 가 들어 있는 살아 있는 프로세스가 10초 안에 없어지지 않으면 이름을 적어 실패한다(`_processes_naming`, `:236-240`).
- `ws` 는 basetemp 아래의 테스트별 경로다. 그래서 다른 작업 공간의 프로세스와 겹치지 않는다.
### 2.2 남은 프로세스 확인이 약해지지 않았는지
- 예전 `pkill -KILL -f <ws>` 는 `ws` 를 명령줄에 가진 것을 **조용히 죽였을** 뿐이다. 남은 프로세스를 확인하지는 않았다.
- 새 방식은 정확히 고른 것만 죽인다. 나머지 가운데 `ws` 를 가진 프로세스가 남으면 **실패로 드러낸다**.
- 그래서 확인은 더 강해졌다.
- 대가가 하나 있다. 그런 프로세스가 생기면 정리되지 않고 남는다. 다만 이름이 실패 메시지에 나오므로 사람이 정리할 수 있다.
- `ws` 를 명령줄에 갖지 않은 손자(예: stub 모니터의 `sleep 120`)는 두 방식 모두 패턴으로는 못 찾는다.
- 새 방식은 모니터의 자손으로 찾아서 정리한다. 따라서 이 부분도 약해지지 않았다.
- 이번 실행에서 teardown 실패는 없었다. 실행 뒤 `ps` 로 봐도 테스트가 남긴 프로세스가 없다(§3).
## 3. 실행한 테스트 (이 Mac: macOS, `/bin/bash` 3.2, `/usr/bin/python3` 3.9.6)
```
cd <검증 경로>
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="$TMPDIR/claude-j1d2c-bt" -q -rfs \
tests/test_issue72_update.py tests/test_issue19_subscriber_daemon_lifecycle.py \
tests/test_issue5_phase1_delegation.py
```
- 결과: **132 passed, 0 failed, 0 skipped** (191 s)
- `test_every_restart_starts_a_real_monitor_in_both_delegation_modes` 의 6 조합이 이 안에 들어 있다.
- `test_issue19`·`test_issue5_phase1` 의 watchdog 시험(중복 방지, `MAM_NO_SUBSCRIBER_DAEMON`, PID 보고, legacy·python 동등성)이 모두 통과한다.
- 고치기 전 대조 실행: TMPDIR 사본, `-k test_every_restart` → 3 failed(mode=0), 3 passed(mode=1). 자세한 것은 §1.2.
- 테스트 뒤 남은 프로세스: 테스트가 띄운 `reconcile.sh`, stub monitor, mock herdr, 구독자는 없다. 남은 것은 원래 있던 셋이다.
- 본 체크아웃 모니터 3714
- 이번 루프 7552
- 이번 잡을 디스패치할 때 띄운 job_subscriber 53086
## 4. 발견 사항
### 차단
- 없음.
### 비차단
- **N8 (이월, R2 에도 해당).** rollback 뒤 재기동은 결정 2 에 따라 **되돌린 옛 트리의** `start_watchdog` 을 쓴다.
- 그래서 실제로 이 PR 이전 버전으로 되돌리면, `MAM_DELEGATION_PY=0` 에서는 옛 `nohup … &` 그대로라 호출자 그룹에 남는다.
- 이 PR 이후 버전 사이의 rollback 에서는 해결된다.
- 결정 2 가 정한 방식("되돌린 트리의 함수")에서 생기는 한계라 결함으로 보지 않는다.
- **1.3 의 세션 차이.** legacy 경로는 프로세스 그룹만 분리하고, python 경로는 세션까지 분리한다. 결정 요구는 충족한다.
- N5·N6·N1·N3·N4 는 이번 범위 밖이라 그대로다.
## 5. 사용자 확인 필요
- N8: rollback 대상이 이 PR 이전 버전이고 `MAM_DELEGATION_PY=0` 인 경우를 받아들일지.
- 받아들이지 않는다면 결정 2 의 "되돌린 트리의 start_watchdog" 대신 `install_phase.sh` 복사본 쪽에서 분리해 띄우도록 결정을 바꿔야 한다.
- 이전 라운드에서 넘어온 N2, 그리고 결정 8 문구 보강 건은 그대로다.
## 6. 결론
- R2 는 정상 설치, 시간 초과 복원, rollback 복원의 세 경로와 두 모드 모두에서 해결됐다.
- 고치기 전 코드에서 mode=0 이 실패하고, 고친 뒤 6개가 모두 통과하는 것을 이 Mac 에서 확인했다.
- 분리 방식(`set -m`)은 중복 방지, opt-out, PID 보고, 로그를 바꾸지 않는다. bash 3.2 에서 잡 메시지 없이 동작한다.
- 테스트의 패턴 kill 은 기록한 PID 와 정확히 맞는 모니터·자손 정리로 바뀌었다. 남은 프로세스 확인은 오히려 강해졌다.
[VERDICT: PASS]