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
9.7 KiB
9.7 KiB
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, treefbadb24de60ccdae684b23e2e0a41cdf23ab81eb로 [리뷰 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:64create_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 출력이 오염되지 않는다.
- 결과 1:
- 고치기 전 코드에서 실패하는지도 확인했다.
- 방법:
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를 쓴다. 테스트 상자에서는 그 트리가 이 버전이다.
- rollback 경로는 되돌린 트리의
1.3 python 경로와의 차이 (비차단, 참고)
- 두 경로가 분리하는 정도가 다르다.
- python 경로(
daemon.py:95):start_new_session=True로 새 세션을 만든다. 그래서 제어 터미널에서도 떨어진다. - legacy 경로: 새 프로세스 그룹만 만든다. 세션과 제어 터미널은 호출자와 같다.
- python 경로(
- 결정 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 으로 확인). - 대신 다음 순서로 정리한다.
- 테스트가
Box.helper(:132-135,start_new_session=True)로 띄우고b.helpers에 기록한 프로세스의 그룹을killpg하고wait한다. - 이 테스트의 작업 공간
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 가 정한 방식("되돌린 트리의 함수")에서 생기는 한계라 결함으로 보지 않는다.
- 그래서 실제로 이 PR 이전 버전으로 되돌리면,
- 1.3 의 세션 차이. legacy 경로는 프로세스 그룹만 분리하고, python 경로는 세션까지 분리한다. 결정 요구는 충족한다.
- N5·N6·N1·N3·N4 는 이번 범위 밖이라 그대로다.
5. 사용자 확인 필요
- N8: rollback 대상이 이 PR 이전 버전이고
MAM_DELEGATION_PY=0인 경우를 받아들일지.- 받아들이지 않는다면 결정 2 의 "되돌린 트리의 start_watchdog" 대신
install_phase.sh복사본 쪽에서 분리해 띄우도록 결정을 바꿔야 한다.
- 받아들이지 않는다면 결정 2 의 "되돌린 트리의 start_watchdog" 대신
- 이전 라운드에서 넘어온 N2, 그리고 결정 8 문구 보강 건은 그대로다.
6. 결론
- R2 는 정상 설치, 시간 초과 복원, rollback 복원의 세 경로와 두 모드 모두에서 해결됐다.
- 고치기 전 코드에서 mode=0 이 실패하고, 고친 뒤 6개가 모두 통과하는 것을 이 Mac 에서 확인했다.
- 분리 방식(
set -m)은 중복 방지, opt-out, PID 보고, 로그를 바꾸지 않는다. bash 3.2 에서 잡 메시지 없이 동작한다. - 테스트의 패턴 kill 은 기록한 PID 와 정확히 맞는 모니터·자손 정리로 바뀌었다. 남은 프로세스 확인은 오히려 강해졌다.
[VERDICT: PASS]