GodopuandClaude Fable 5.1
9e1f0b68b1
Final-round reports of the fix/issue-9-14 review loop: planner-reviewer-claude-01
(2d3f24e4), reviewer-agy-01 (47a845e0), reviewer-grok-01 (d7e66053) and
planner-reviewer-codex-01 (0ad7cb06), all [VERDICT: PASS] on ebe58f6.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
77 lines
7.9 KiB
Markdown
77 lines
7.9 KiB
Markdown
# 🔍 Review Report — Job 2d3f24e4 (branch `fix/issue-9-14`, HEAD `ebe58f6`)
|
|
|
|
- **Reviewer**: `planner-reviewer-claude-01` (role: planner,reviewer) · 2026-09-13
|
|
- **대상**: `git log 4d444ce..HEAD` 9커밋 — `497633f`, `16376c5`, `4b31345`, `eaa49aa`, `0918917`(release v4.4.2), `7c40bf4`, `d5a191c`, `952657e`, **`ebe58f6`**(이번 라운드 신규). 이전 라운드에서 본 커밋은 변경분만 재확인하고, `ebe58f6` 는 전체를 검토했다.
|
|
- **방법**: 코드 정독, `events.log` 소비자 전수 확인, 선택 테스트(부하 없는 단독 실행) + publish 계약 스위트. 리뷰 중 45초 간격 `progress` heartbeat 발행. 코드 수정 없음.
|
|
|
|
## 0. 직전 라운드(HEAD `952657e`) 결과와 처리
|
|
|
|
| 리뷰어 | 판정 | 요지 |
|
|
|---|---|---|
|
|
| `planner-reviewer-claude-01`(`9323564c`) | PASS | 벽시계 경계 확인, t1_20 부하 민감성 권고 |
|
|
| `reviewer-agy-01`(`f191b557`) | PASS | — |
|
|
| `reviewer-grok-01`(`b0591d20`) | **NOT PASS** | `publish_event.py` 가 heartbeat 를 **와이어 발행(브로커 ACK·재시도) 뒤에** `events.log` 에 기록 → R1 의 디스크 근거가 늦게 도착해 살아 있는 잡이 `deadline_timeout` 으로 닫힘. 실제 `publish_event` 경로 테스트가 2/2 실패 |
|
|
| `planner-reviewer-codex-01`(`e657c660`) | **리뷰 없음** | 구독자가 120 s 동안 이벤트 0건(`idle timeout … still pending`) 후 `deadline_timeout`. codex 가 `started` 조차 발행하지 않았으므로 heartbeat 순서 문제와는 무관 |
|
|
|
|
creator 는 grok 지적을 반론 없이 수용해 `ebe58f6` 로 고쳤다. `952657e` 에서 새 순서 테스트 2건과 grok 의 테스트가 실패함을 역검증했고, 수정 후 grok 의 테스트는 무변경으로 3/3 통과했다.
|
|
|
|
## 1. `ebe58f6` 검토 — ✅
|
|
|
|
### 1-1. 순서와 중복 방지
|
|
- **CLI `publish_event.main`**:
|
|
1. token·attempt 검증 — 불일치는 rc 4 로 거부되며 **기록되지 않는다**(`publish_event.py:290-341`)
|
|
2. `prepare_event`(seq 소비)(`:343-356`)
|
|
3. **`record_local_event`**(`:360`) → `registry.append_event`(`:165-175`)
|
|
4. `with_retry(_publish_once)`(`:364-376`)
|
|
5. `record_outcome(..., local_event_recorded=True)`(`:385-388`) — 감사 로그 `published` 항목과 상태 동기화만 발행 뒤에 수행하고, `events.log` append 는 건너뛴다(`:204-205`)
|
|
- **`DelegationService.publish_event`**: 같은 순서다(`service.py:82-100`).
|
|
- **다른 호출자**:
|
|
- `record_outcome` 의 기본값은 `local_event_recorded=False` 라서 직접 호출하는 테스트(G-1/G-2/G-4)는 동작이 그대로다.
|
|
- `prepare_event` 의 유일한 다른 호출자 `bench.py:149` 는 원래 `events.log` 를 쓰지 않았으므로 영향이 없다.
|
|
- 새 테스트가 "발행 시작 순간 이미 1줄", "재시도 2회에도 1줄 + 감사 `published=False`"를 단정한다.
|
|
|
|
### 1-2. `events.log` 소비자 영향
|
|
- **`_recent_disk_heartbeat`(래퍼)**: 의도한 수혜자다. 브로커 지연(실측 약 5 s/발행)과 무관하게 heartbeat 가 즉시 디스크에 보인다.
|
|
- **`registry.get_feedback` 폴백(`registry.py:241-260`)**: 감사 ndjson 에 terminal 이벤트가 없을 때만 `events.log` 에서 detail 을 읽는다. 호출 시점이 잡 종료 이후이고, 기록 **내용**은 이전과 같다(CAS 로 거부된 terminal 이벤트도 원래 기록되었다). 달라진 것은 시점뿐이다.
|
|
- **그 밖의 소비자**: 셸·파이썬 전수 grep 결과 없다. 상태의 권위는 계속 레지스트리 레코드(`_disk_status`)다.
|
|
|
|
### 1-3. 의도된 의미 변화 (creator 문서화)
|
|
- 발행 도중 프로세스가 죽으면 `events.log` 에는 줄이 남지만 감사 `published` 항목과 상태 동기화는 없다.
|
|
- terminal 이벤트라면 `events.log` 에 `completed` 가 있어도 레코드는 아직 비종료일 수 있다. R1 은 이를 **liveness 근거로만** 쓰고 결과는 레코드에서 읽으므로 잘못된 완료 판정으로 이어지지 않는다.
|
|
|
|
## 2. 검증 항목별 결과
|
|
|
|
| # | 항목 | 결과 |
|
|
|---|---|---|
|
|
| (1) | 서버 env denylist | ✅ 무변경(1회차 판단 유지, 호스트 마커 권고는 비차단) |
|
|
| (2) | `MAM_NO_AUTOSTART` · `ls` 번역 | ✅ 무변경(`952657e` 에서 확인한 구조화 판정·unknown 종료 유지) |
|
|
| (3) | `send_keys_safe` | ✅ 무변경 |
|
|
| (4) | `_inject_acknowledged` · `_agent_still_working` · `_recent_disk_heartbeat` · R1 창 | ✅ 벽시계 경계(`952657e`)는 그대로다. 이번 커밋으로 heartbeat 가 발행 **전에** 디스크에 기록되어 grok 이 재현한 미탐이 사라졌다(해당 테스트가 이번 실행에서 통과) |
|
|
| (5) | T1-7 트리거 | ✅ 무변경 |
|
|
| (6) | 버전 락스텝 · 릴리스 커밋 | ✅ `lib.sh:32`·`eager.py:16` = 4.4.2. `VERSIONS.md` 항목 6 이 "와이어 발행 **전** 로컬 기록(CLI·service 두 경로)"으로 코드와 일치. "신규 테스트 21개" = `test_issue9_server_env.py` 9 + `test_issue14_delivery_ack.py` 12 함수(직접 셈) |
|
|
| (7) | 테스트 · 정적 검사 | ✅ 아래 |
|
|
|
|
- **선택 실행 11파일**(부하 없이 단독): **163 passed in 431 s**
|
|
- 대상: `test_issue9_server_env`, `test_issue14_delivery_ack`, `test_issue6_fail_closed`, `test_issue6_turn_handshake`, `test_a4_adapter_contract`, `test_loop_cli`, `test_issue5_phase4_runtime`, `test_version_consistency`, `test_herdr_shim_contract`, `test_b19_headless_reconcile_fixes`, `test_deploy_registry_merge`
|
|
- grok 의 `test_wrapper_idle_timeout_grace_accepts_disk_heartbeat`, 새 순서 테스트 2건, 직전 라운드에 부하 속에서 한 번 실패했던 `test_t1_20` 모두 통과
|
|
- **publish 계약 스위트** `tests/test_issue5_phase1_delegation.py`(G-1/G-2/G-4, transport parity, `record_outcome` 직접 호출): **66 passed**(주 실행이 끝난 뒤 따로 실행)
|
|
- **정적 검사**: `py_compile`(publish_event·service), `bash -n`(lib.sh·래퍼·status.sh·reconcile.sh), `git diff --check 4d444ce...HEAD` 모두 클린. 작업 트리 깨끗, HEAD `ebe58f6`.
|
|
|
|
## 3. 비차단 권고
|
|
|
|
1. **`events.log` 의미 명시**: 이제 이 파일은 "수락된 상태"가 아니라 "시도된 이벤트"의 기록이다(발행 전 기록, CAS 거부분 포함). `MESSAGING.md` 나 `registry.append_event` docstring 에 한 줄 적어 두면, 이후 이 파일을 상태 판단에 쓰는 코드가 생기는 것을 막을 수 있다.
|
|
2. **t1_20 예산**(직전 라운드 권고, 미반영): 이번 부하 없는 실행에서는 통과했지만 `--timeout 12` 는 부하가 걸린 CI 에서 여전히 간헐 실패할 수 있다. 20–25 s 로 늘리기를 권한다.
|
|
3. **loop 모드 마감 서술**(직전 라운드 권고, 미반영): `START_TIME` 은 `cmd_submit` 에서 한 번만 정해진다(`:166-167`). 주석의 "this dispatch's deadline" 은 loop 모드에서 "이 래퍼 실행의 마감"이 정확하다.
|
|
4. 보류된 후속 항목 묶음(호스트 마커, `$WRAPPER` 경로, pane 변화 신호, ack dispatch 고정, heartbeat token 필터, `_within_deadline` 의 `Terminated` 표시 잡음)은 변동이 없다. 하나의 후속 이슈로 모으기를 권한다.
|
|
|
|
## 4. GM 참고
|
|
|
|
- **codex 는 `952657e`·`ebe58f6` 를 리뷰하지 않았다.** 마지막 codex 판정은 `d5a191c` 에 대한 NOT PASS(`75c1d06c`)였고, 그 두 지적은 `952657e` 에서 해소되었다(본 세션 확인). `e657c660` 은 codex 가 이벤트를 하나도 발행하지 않은 채 닫혔다. 병합 전에 codex 세션 상태를 확인하고 재리뷰할지 결정할 것을 권한다.
|
|
|
|
## 5. 종합
|
|
|
|
- `ebe58f6` 는 grok 의 차단 지적(디스크 heartbeat 가 브로커 지연 뒤에 기록됨)을 정확한 위치에서 최소 변경으로 고쳤다. 검증 우선·seq·G-1/G-2/G-4 계약을 유지하고, 중복 기록이 없으며, `events.log` 소비자에도 부작용이 없다.
|
|
- 브랜치 전체가 브리프의 7개 검증 항목을 충족하고, 부하 없는 선택 실행 163건과 계약 스위트 66건이 모두 통과했다. 설계 재검토는 필요 없다.
|
|
|
|
[VERDICT: PASS]
|