Final HEAD263d6bf: claude, agy and grok PASS (delta review of the lifecycle-lock commits on top of theirab842f1PASS), codex PASS (its own round after F1/F2). Full suite at263d6bf: 906 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
133 lines
10 KiB
Markdown
133 lines
10 KiB
Markdown
# 🔍 Review Report — Job 48b1b0f8 (Issue #18 델타 리뷰, reviewed HEAD `263d6bf`)
|
||
|
||
- **Reviewer**: `planner-reviewer-claude-01` (role: planner,reviewer) · 2026-09-14
|
||
- **대상**: `git diff ab842f1...HEAD` 2커밋, 4파일 +273/−10.
|
||
- `2b1d590`: 잡별 lifecycle lock, 구현 세션 커밋
|
||
- `263d6bf`: codex F1·F2 교정, creator 커밋
|
||
- 전체 브랜치는 내가 `ab842f1` 에 PASS 한 범위다(`98215f55`).
|
||
- 브리프의 "변경분" 은 작업 트리의 추적되지 않은 codex 리포트 파일 하나뿐이었다. 그래서 델타는 git 으로 직접 확인했다.
|
||
- HEAD 는 이 잡 내내 `263d6bf` 이다(`completed` 발행 명령에서 다시 확인).
|
||
- **방법**:
|
||
- 델타를 정독했다.
|
||
- 임시 레지스트리에서 실제 타이머 프로세스로 독립 실험 4건을 했다(브로커 없음, 닫힌 포트·블랙홀 주소).
|
||
- 선택 테스트 9파일을 실행하고 정적 검사를 했다.
|
||
- 이 잡은 `started --heartbeat 30` 으로 시작했다. 코드는 수정하지 않았다.
|
||
- **안전 규칙 준수**:
|
||
- 실제 `.mam/jobs` 레코드는 수정하지 않았다(실행 전후 비교).
|
||
- monitor 데몬과 `.mam/monitor.lock` 은 건드리지 않았다. purge 는 실행하지 않았다.
|
||
- 실험 타이머 잔여는 0 이다.
|
||
|
||
## 0. 경위
|
||
|
||
| 리뷰 | HEAD | 판정 | 요지 |
|
||
|---|---|---|---|
|
||
| claude `98215f55`·agy·grok | `ab842f1` | PASS | — |
|
||
| codex (4회차) | `ab842f1` | NOT PASS | 동시 `start_timer` 2건이 둘 다 spawn 하고 하나만 추적됨 → `2b1d590` |
|
||
| codex `d1eb9f50` | `2b1d590` | NOT PASS | **F1**: 종료 중인 타이머가 drain 중인 부모가 쥔 lock 을 기다리는 유한 교착(항상 SIGKILL 로 끝남). **F2**: pidfile 기록 실패 뒤 추적되지 않는 타이머가 남음 → `263d6bf` |
|
||
| codex `bad3ef4b` | `263d6bf` | PASS | — |
|
||
|
||
## 1. 검증 항목
|
||
|
||
### (1) lifecycle lock 과 레지스트리·seq lock 의 순서, 교착 가능성
|
||
|
||
- **구조**: lifecycle lock(`heartbeat.py:187-209`, `<job>.heartbeat.lock`)은 레지스트리·seq lock 과 **별개의 flock** 이다. 잡는 곳은 세 군데뿐이다.
|
||
- 터미널 `stop_timer`(`:221`)
|
||
- `start_timer`(`:312`)
|
||
- 종료하는 타이머의 정리(`:473`, **non-blocking**)
|
||
- **lifecycle lock 안에서 하는 일**:
|
||
- `_stop_timer_locked` 의 drain 은 레지스트리 lock 을 잡지 않는다.
|
||
- `load_job` 은 lock 없는 읽기다(`mqtt_common.py:393-399`). `barrier_blocks` 도 파일 읽기뿐이다.
|
||
- 그 뒤 spawn 과 pidfile 기록을 한다.
|
||
- **레지스트리 lock 을 잡는 곳**: `raise_barrier`(`heartbeat.py:270`)와 `next_seq` 뿐이다. 둘 다 lifecycle lock 을 잡지 않는다.
|
||
- 터미널 publish 는 `raise_barrier`(레지스트리 lock 해제) → `stop_timer`(lifecycle) 순으로 **중첩 없이 차례로** 잡는다.
|
||
- 따라서 역순 중첩이 없고, lock 순서 교착은 불가능하다.
|
||
- **drain 중 in-flight beat**: beat 는 seq lock 만 필요하다. 그 lock 은 drain 하는 쪽이 쥐고 있지 않으므로 beat 는 끝까지 갈 수 있다.
|
||
- **F1 교정**: 종료하는 타이머는 lock 이 바쁘면 기다리지 않고 정리를 소유 연산에 맡긴다. 소유 연산(start·stop)은 모두 `_stop_timer_locked` 로 시작하며, 그 안에서 pidfile 을 지운다.
|
||
- **실험 E1 — 유휴 타이머 정지** (interval 60, 2 s 대기 뒤 `stop_timer(drain=5)`):
|
||
- **0.17 s** 에 반환했고, pidfile 이 제거되었으며, "did not drain" 경고가 없었다.
|
||
- codex 가 `2b1d590` 에서 잰 값은 2.09 s, `-9` 였다. 교착이 해소되었다.
|
||
- **실험 E4 — 와이어에 걸린 beat 가 있는 drain** (브로커 `10.255.255.1` 블랙홀, interval 1):
|
||
- `stop_timer(drain=1)` 가 **1.12 s** 에 SIGKILL 로 끝났다. 이 경우의 "did not drain" 경고 1건은 정당하다.
|
||
- 직후 `next_seq` 는 **0.001 s** 에 반환했다(seq lock 은 비어 있음). pidfile 은 제거되었다.
|
||
- drain 과 seq lock 사이에 교착이 없다.
|
||
|
||
### (2) 동시 start, barrier 뒤 start, pidfile 기록 실패
|
||
|
||
- **실험 E2 — 동시 `start_timer` 5건**(스레드 barrier 로 동시 출발):
|
||
- 오류 0, 살아남은 타이머 **정확히 1개**, pidfile 이 그 타이머를 가리킨다.
|
||
- `stop_timer` 뒤 생존 0, pidfile 없음.
|
||
- **실험 E3 — `start_timer` 와 터미널 `completed` publish 경쟁, 8라운드**(시작 간격 0–14 ms 로 변화):
|
||
- **위반 0/8**: 매 라운드 레코드 `completed`, 생존 타이머 0, pidfile 없음, events.log 에 `completed` 뒤 이벤트 0.
|
||
- 1라운드는 start 가 먼저 arm 해서 터미널이 drain 했다. 7라운드는 barrier·터미널을 보고 **arm 하지 않았다**.
|
||
- **pidfile 기록 실패 (F2)**(`:343-364`): spawn 성공 뒤 pidfile 기록이 실패하면 lock 을 쥔 채 그 `Popen` 을 kill·wait 한다. 그리고 이 시도의 부분 pidfile 만 지운 뒤 예외를 다시 던진다.
|
||
- 시험 `test_a_timer_whose_pid_cannot_be_recorded_is_killed_at_once` 가 SIGKILL 종료, pidfile 없음, lock 재사용 가능을 단정한다.
|
||
|
||
### (3) 예외 경로에서 lock 해제와 pidfile 일관성
|
||
|
||
- `lifecycle_lock` 은 `try/finally` 로 `LOCK_UN` 하고, `with open(...)` 가 fd 를 닫으므로 예외에서도 해제된다.
|
||
- non-blocking 이 바쁘면 `False` 를 내주고, 호출자는 소유 상태를 건드리지 않는다(`:473-478`).
|
||
- **spawn 전 실패**(`Popen` 오류): pidfile 을 쓰지 않고 lock 은 해제된다. 시험 `…cannot_be_spawned_leaves_nothing_behind` 가 확인한다.
|
||
- **spawn 후 실패**: 이전 소유자의 pidfile 은 이미 `_stop_timer_locked` 가 지웠다. 따라서 남은 내용은 이번 시도의 것뿐이고, pid 접두 비교로 지우는 판단이 옳다.
|
||
- P3:
|
||
- `lifecycle_lock` 은 `os.makedirs(registry_dir)` 를 한다. 그래서 잘못된 레지스트리 경로로 터미널 publish 를 하면 디렉터리가 생긴다.
|
||
- 모든 터미널 publish 가 잡마다 `<job>.heartbeat.lock` 을 남긴다(기존 `.heartbeat.barrier` 와 같은 성질). `.json` 이 아니므로 잡 목록과 #15 가드에는 영향이 없다. 레지스트리 정리 정책에서 함께 다룰 것을 권한다.
|
||
|
||
### (4) 새 회귀 시험의 결정성과 프로세스 정리
|
||
|
||
| 시험 | 판단 |
|
||
|---|---|
|
||
| `test_concurrent_start_timer_calls_leave_exactly_one_tracked_owner` | 스레드 2개로 순서를 강제하지 않는다(codex 도 지적). 위의 5건 동시 실험이 보완한다. `finally` 에서 두 pid 를 SIGKILL 한다 |
|
||
| `test_start_timer_after_the_terminal_barrier_does_not_arm` | 결정적(프로세스 없음) |
|
||
| `test_an_idle_timer_stops_gracefully_well_before_the_drain_deadline` | 2 s 대기로 TERM 핸들러 설치 뒤를 노린다. 과부하로 핸들러 설치 전에 TERM 이 가도 기본 동작으로 곧 끝나므로 `< 3 s` 단정은 유지된다. `finally` 에서 정리한다 |
|
||
| `test_a_timer_whose_pid_cannot_be_recorded_is_killed_at_once` | `builtins.open` 을 잠깐 패치하고 `finally` 에서 복구한다. spawn 된 프로세스는 `finally` 에서 kill·wait 한다 |
|
||
| `test_a_timer_that_cannot_be_spawned_leaves_nothing_behind` | 없는 python 경로를 쓴다. 두 번째 타이머는 `stop_timer`·SIGKILL 로 정리한다 |
|
||
|
||
- 실행 결과는 §2 에 있다.
|
||
|
||
### (5) 문서·수치·락스텝
|
||
|
||
- MESSAGING §2.5 heartbeat 행이 코드와 일치한다: "lifecycle lock(`<registry>/<job>.heartbeat.lock`, seq lock 과 별개) — 끝난 attempt 는 arm 하지 않음, 종료 타이머는 lock 을 기다리지 않음, pid 기록 실패 시 즉시 종료". VERSIONS 항목 1 도 같다.
|
||
- VERSIONS 품질 절 "신규 테스트 31개"(`VERSIONS.md:54`)는 `grep -c '^def test_'` 결과 31 과 같다.
|
||
- 4.5.0 락스텝 유지: `verify_framework_version` OK, `SKILL.md` 8개 4.5.0.
|
||
- `git diff --check 139bdd4...HEAD` 클린. `py_compile`(`heartbeat.py`, 시험) 통과.
|
||
|
||
## 2. 테스트 실행 결과
|
||
|
||
| 실행 | 결과 |
|
||
|---|---|
|
||
| **선택 9파일** (issue18 31, issue14, issue6 fail_closed·turn_handshake, phase1, phase2a, tier1, tier2, version) @ `263d6bf`(전후 동일) | **355 passed, 0 failed, 742.30 s**, skipped 0. 브로커 의존 AC1 22.09 s, AC3 18.92 s, pane-busy 17.93 s, 벽시계 30.12 s |
|
||
| 독립 실험 E1–E4 | 모두 기대대로(§1). 실험 타이머 잔여 0 |
|
||
| creator `31a24383` (3파일) | 100 passed |
|
||
|
||
- **누수 (실행 전후)**:
|
||
- 실제 monitor 데몬 수가 같다(3개, 실행 전후 동일 PID).
|
||
- pytest 경로의 heartbeat 타이머·구독자·원격 러너는 0 이다.
|
||
- 실제 `.mam/jobs` 추가는 0 이다.
|
||
|
||
## 3. 비차단 권고
|
||
|
||
1. (이월, T-1) 브로커 의존 시간 임계 시험의 여유를 넓힌다: AC1·AC3·AC5 의 K×N, issue14 벽시계·pane-busy, send_keys `< 11`.
|
||
2. 잡마다 남는 `.heartbeat.lock`·`.heartbeat.barrier` 파일의 정리 정책을 정한다. `lifecycle_lock` 의 `makedirs` 부작용도 함께 본다.
|
||
3. (이월) AC5 는 단일 원격 잡 시뮬레이션과 러너 라벨 단위 시험까지만 검증되었다. 루프 종단 수렴은 후속 E2E 로 남는다.
|
||
|
||
## 4. GM 참고
|
||
|
||
- **추적되지 않은 파일**: 작업 트리에 `.agents/reports/planner-reviewer-codex-01/report-bad3ef4b.md` 가 있다(codex 리포트 사본). 브리프의 "변경분" 이 이것 하나로 채워졌다. 나는 건드리지 않았다.
|
||
- **커밋 주체**: `2b1d590` 은 구현 세션(`…01VCgu9Fa9iT7TbJUSzdq4Ff`), `263d6bf` 는 creator(`…01HCGpJXhSZasJLU38ksDghe`)의 커밋이다.
|
||
- **브랜치 ref**: 11커밋 모두 로컬 `main` 에 있다(`origin/main` 보다 11 앞, 미푸시). `fix/issue-18` 은 `139bdd4` 다. 병합 전에 ref 정리가 필요하다.
|
||
|
||
## 5. 종합
|
||
|
||
- `2b1d590` 의 잡별 lifecycle lock 은 레지스트리·seq lock 과 중첩 없이 분리되어 있어 교착이 없다.
|
||
- `263d6bf` 의 non-blocking 종료 정리(F1)와 spawn 후 pid 기록 실패 시 종료(F2)가 정확하다.
|
||
- 독립 실험으로 확인했다:
|
||
- 유휴 정지 0.17 s
|
||
- 동시 start 5건에서 추적 소유자 1개
|
||
- start 와 터미널 경쟁 8라운드에서 위반 0
|
||
- 걸린 beat 가 있어도 seq lock 이 즉시 비어 있음
|
||
- 새 시험들이 이 경로들을 고정한다. 선택 스위트 355건이 모두 통과했다.
|
||
- 문서·수치(31)·4.5.0 락스텝이 정합한다.
|
||
- 남은 것은 비차단 권고뿐이며 설계 재검토는 필요 없다.
|
||
|
||
[VERDICT: PASS]
|