release(v4.12.0): update skill with in-place install and an update admission gate; resume starts the monitor (Issue #72) - archive the review reports, bump the lockstep version
The update skill joins the version matrix and the lockstep test's skill list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
GodopuandClaude Opus 5.5 committed
1 parent
ec7f5c4e57
commit
56e6713292
26 files changed
+542
-30
No files matched your search
@@ -0,0 +1,132 @@
|
||||
# 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]
|
||||
Reference in new issue
Block a user