Files
multi-agent-mux/.agents/reports/planner-reviewer-claude-01/report-c666854d.md
T
Godopu 4bbd03bf2d fix(lib): handle agent_not_ready startup state and reject claude fullscreen upsell modal
- Accept agent_not_ready from herdr agent start to allow startup dialog handling without premature rollback, while preserving fail-closed behavior on dead process timeouts.
- Match Claude fullscreen renderer upsell modal via 'Yes, try it' and dismiss with Escape to avoid dropping permission flags or deadlocking on idle /tui tips.
- Add behavioral test suite in test_b19_headless_reconcile_fixes.py and cross-agent review reports.
2026-08-27 10:17:53 +09:00

212 lines
11 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.
# 교차 코드 리뷰 리포트 — Job c666854d (rev.2)
- **대상**: `.agents/skills/lib.sh`, `tests/test_b19_headless_reconcile_fixes.py`, `FIX.md`
- **리뷰어**: claude (`planner-reviewer-claude-01`)
- **선행 리뷰**: Job b9e42784 — `[VERDICT: NOT PASS]` (D-1 ~ D-5)
- **관점**: 린트 / 동작성 / 유실
- **결론**: 선행 리뷰의 지적 5건이 **모두 정확히 해소**되었고, 이번에는 **실제 Claude Code 클라이언트로 종단 검증**까지 마쳤다.
잔여 지적은 전부 경미(Low)하며 병합을 막지 않는다.
작업 트리 diff는 브리프에 첨부된 diff와 **완전히 일치**한다 (`lib.sh` 16줄, 테스트 106줄, `FIX.md` 신규).
---
## 0. 검증 방법
이번 리뷰의 핵심은 **실물 검증**이다. 선행 리뷰에서는 합성 화면으로만 확인했으나, 이번에는
`CLAUDE_CODE_FORCE_FULLSCREEN_UPSELL=1`로 **실제 업셀 모달을 강제 재현**하여 확인했다.
| # | 방법 |
|---|---|
| V-1 | 실제 herdr 0.8.2 출력값으로 `agent start` 분류 로직 재현 |
| V-2 | herdr 워크스페이스에 **Claude Code v2.1.247 실기동** → 모달 강제 표시 → Escape 전송 → 상태 측정 |
| V-3 | `lib.sh`를 그대로 source 하여 실제 함수 실행 (HEAD 대비 비교) |
| V-4 | **거부되었던 rev.1 구현을 격리 worktree에 복원**하고 신규 테스트를 돌려 회귀 검출력 확인 |
| V-5 | pytest 전체 |
프로브 워크스페이스(`w1H`/`w1J`/`w1K`/`w1M`/`w1N`)와 임시 worktree는 **전부 정리 완료**.
현재 남은 워크스페이스는 실사용 `w1E` 하나뿐이며, `git worktree list`도 1개(본체)로 복귀했다.
---
## 1. 선행 지적 해소 확인
### D-1 (🔴 → ✅) — 죽은 프로세스가 성공으로 승격되던 문제
성공 정규식이 `agent_started|agent_not_ready`로 축소되었다.
선행 리뷰에서 **실제 herdr 프로브로 채집한 출력값**을 그대로 넣어 분류를 재현한 결과:
| herdr 실제 출력 | 분류 결과 |
|---|---|
| `{"error":{"code":"timeout","message":"timed out waiting for agent startup"}}` (← `/bin/false`, **죽은 프로세스**) | `retry → exit 1` ✅ |
| `{"error":{"code":"agent_pane_busy", …}}` | `retry → exit 1` ✅ |
| `{"error":{"code":"agent_not_ready", …}}` (프로세스 생존, 다이얼로그 차단) | `success=1` ✅ |
| `agent_started` | `success=1` ✅ |
Fail-Closed 복원 확인. 치명 오류 우선 분류 순서도 유지되었다.
또한 이번 실기동에서 herdr가 **정확히 그 상태를 반환하는 것을 실물로 확인**했다:
```
{"error":{"code":"agent_not_ready","message":"agent probe-fs5 is blocked during startup and is not ready for prompts"}}
```
`agent_not_ready`를 롤백 사유로 보지 않는 처리가 **가정이 아니라 실측으로** 정당화되었다.
부수 확인: 워크스페이스 생성 직후 즉시 `agent start` 하면 `agent_pane_busy`가 실제로 발생한다(2회 재현).
`lib.sh`의 3회 백오프(0.5/1/2초) 재시도가 이 창구를 정확히 덮으므로 **재시도 루프는 유지되어야 한다**.
### D-2 / D-2c (🔴 → ✅) — idle 팁을 차단형 다이얼로그로 오인하던 문제
광의 토큰 `fullscreen renderer|Try the new fullscreen`이 제거되고 모달 고유 문자열만 사용한다.
`lib.sh`를 실제 source 하여 **정상 기동(팁만 표시, TUI 준비 완료)** 화면을 넣은 결과:
| 팁만 있는 정상 화면 | HEAD | rev.1 (거부됨) | **rev.2 (현재)** |
|---|---|---|---|
| `_pane_dialog_open` | false | 🔴 TRUE | ✅ **false** |
| `handle_startup_dialogs` 전송 키 | 0 | 🔴 Enter 20회 | ✅ **0회** |
| `wait_for_tui_ready` | rc=0 | 🔴 rc=1 (+Enter 30회) | ✅ **rc=0** |
정상 경로가 HEAD와 **완전히 동일**하게 복귀했다. 데드락 해소 확인.
### D-3 (🟠 → ✅) — Enter가 업셀을 "수락"하던 문제
**실제 모달을 강제 재현해 캡처했다.** 모달 하단 안내가 결정적이다:
```
Try the new fullscreen renderer?
· Flicker-free output
· Mouse support — click to move your cursor or expand results
· Selected text auto-copies to your clipboard
1. Yes, try it
2. Not now
Enter to confirm · Esc to cancel
```
- `Enter to confirm` → 기본 선택지 `Yes, try it` 수락. 선행 리뷰의 D-3 지적이 **모달 자체 문구로 확증**되었다.
- `Esc to cancel` → 수정이 택한 Escape가 **모달이 스스로 안내하는 취소 키**다.
Escape 전송 후 실측:
| 항목 | 결과 |
|---|---|
| 모달 제거 | ✅ 사라짐 (`Yes, try it` 0건) |
| 잔여 다이얼로그 토큰 | ✅ 0건 → `send_keys_safe` 차단 해제 |
| ready 토큰 가시성 | ✅ 2건 (`Claude Code v2.1.247`, `Sonnet 5 with high effort`) → `wait_for_tui_ready` 통과 |
| **`⏵⏵ bypass permissions on` 유지** | ✅ **세션 재시작 없음 — `--dangerously-skip-permissions` 보존** |
마지막 항목이 중요하다. 우려했던 "수락 시 permission flag 없이 재시작" 경로를 **Escape가 회피함을 실물로 확인**했다.
### D-4 (🟠 → ✅) — 테스트가 소스 문자열만 확인하던 문제
신규 4건 중 3건이 `_pane_capture`를 stub 하고 **함수를 실제 실행**하는 행위 테스트로 바뀌었다.
(`_pane_tail``_pane_capture``_sks_herdr` 체인이므로 `_pane_capture` stub은 올바른 주입 지점이다.)
회귀 검출력을 직접 측정했다. **거부되었던 rev.1 구현을 격리 worktree에 복원**하고 신규 테스트를 실행:
```
FAILED test_agent_start_success_tokens_exclude_startup_timeout
FAILED test_fullscreen_tip_is_not_a_blocking_dialog
FAILED test_fullscreen_modal_is_rejected_not_accepted
FAILED test_wait_for_tui_ready_succeeds_on_fullscreen_tip
4 failed
```
→ **4건 전부 rev.1에서 실패하고 rev.2에서 통과**한다. 실질적 회귀 방지력이 확인되었다.
`test_wait_for_tui_ready_succeeds_on_fullscreen_tip` 실패 로그에는 rev.1의 `Enter` 30회 주입과
`⚠️ TUI readiness check timed out`이 그대로 찍혔다 — 정확히 선행 리뷰가 지적한 증상이다.
### D-5 (🔵 → 대부분 해소)
`FIX.md`가 재작성되어 순서 변경·타임아웃 토큰 배제 근거·팁/모달 구분이 모두 기술되었고, 말미 개행도 정상이다.
---
## 2. 테스트 / 린트 결과
- `tests/` 전체 **397 passed** (8분 30초). HEAD 393 + 신규 4건과 정확히 일치.
- 변경 파일 단독 **10 passed** (신규 4건 포함).
- `bash -n .agents/skills/lib.sh` **통과**. `shellcheck`는 이 환경에 미설치라 미실행.
---
## 3. 잔여 지적 (전부 Low — 병합 차단 아님)
### N-1 `_MAM_DIALOG_TOKENS`의 `|Yes, try it`은 **불필요하며** 오탐 면적만 넓힌다
실제 모달 문구에는 `Esc to cancel`이 포함되어 있고, 이 토큰은 **HEAD의 기존 토큰 목록에 이미 존재**한다.
HEAD 토큰만으로 실제 모달이 매칭되는 것을 확인했다 → **탐지 목적으로는 추가가 중복**이다.
(선행 리뷰의 합성 픽스처에는 이 하단 안내줄이 없어 드러나지 않았던 부분이다.)
반면 `Yes, try it`은 평문 대화에 등장할 수 있는 자연어다. 실제로 아래 한 줄이 `_pane_dialog_open`을 참으로 만든다:
```
⏺ Sure — if the build fails again, Yes, try it with the --clean flag.
```
`send_keys_safe`가 30초 대기 후 `rc=2`로 실패한다. 확률은 낮고, HEAD에도 `Allow this` / `No, exit` 같은
평문형 토큰 선례가 있어 **새로운 부류의 위험은 아니다.** 다만 이 건은 얻는 것이 없으므로 제거를 권한다.
- **권고**: `_MAM_DIALOG_TOKENS`에서 `|Yes, try it` 제거. `handle_startup_dialogs`의 분기는 그대로 둔다
(모달 탐지는 기존 `Esc to cancel`이 이미 담당). 더 좁히려면 팁에 없는 물음표형
`Try the new fullscreen renderer\?`를 앵커로 쓰는 편이 가장 정확하다.
### N-2 테스트의 `_init_herdr_isolation` stub이 **동작하지 않는다**
`_run_lib_helpers``source` **뒤에** `_init_herdr_isolation() { :; }`을 정의하지만,
`lib.sh:1881`에서 이미 source 시점에 실호출된다. 따라서 stub은 사실상 죽은 코드이고,
매 테스트가 `$WORKSPACE_ROOT/.mam/shim/herdr`를 실제로 기록한다(실행 중 mtime 갱신 확인).
`.mam/`은 gitignore 대상이라 git 오염은 없고 멱등이라 실피해도 없으나, **의도와 실제가 어긋나 있다.**
- **권고**: 아래 N-3의 미사용 파라미터를 활용해 `WORKSPACE_ROOT`를 임시 디렉터리로 넘긴다.
### N-3 `_run_lib_helpers(env_extra=...)`가 **어떤 호출부에서도 사용되지 않는다** (미사용 파라미터)
N-2의 해법 통로이므로 제거보다 활용을 권한다.
### N-4 테스트가 `/tmp/mam-fs-*-keys.$$`를 하드코딩한다
스크립트 말미의 `rm -f``set -euo pipefail` 하에서 앞 단계가 실패하면 실행되지 않아 잔여 파일이 남을 수 있다
(이번 실행에서는 잔여물 없음). pytest `tmp_path` 사용을 권한다.
### N-5 `FIX.md`가 여전히 **untracked**다
변경 근거 문서로 참조되고 있으므로, 병합 전 커밋하거나 의도적으로 제외한다면 그 판단을 남겨야 한다.
---
## 4. git discard 여부
**discard 하지 말 것.** 두 문제 모두 실재함이 이번에 실물로 확정되었다 —
herdr가 기동 중 차단 상태에서 `agent_not_ready`를 반환하는 것, 그리고
Claude Code v2.1.247이 `Try the new fullscreen renderer?` 모달로 기동을 막는 것 모두 직접 재현했다.
---
## 5. 요약
| 선행 지적 | 상태 | 근거 |
|---|---|---|
| D-1 죽은 프로세스 → 성공 승격 | ✅ 해소 | 실채집 herdr 출력 4종 분류 재현 |
| D-2 팁을 다이얼로그로 오인 | ✅ 해소 | 실함수 실행, HEAD와 동일 동작 복귀 |
| D-2c `wait_for_tui_ready` 데드락 | ✅ 해소 | rc=1 → **rc=0** |
| D-3 Enter가 업셀 수락 | ✅ 해소 | 실모달 `Enter to confirm · Esc to cancel`, Escape 후 permission flag 보존 확인 |
| D-4 회귀 방지력 없음 | ✅ 해소 | rev.1 복원 시 신규 4건 전부 실패 |
| D-5 문서/린트 | ✅ 대부분 해소 | FIX.md 재작성 |
| 신규 지적 | 심각도 | 요지 |
|---|---|---|
| N-1 | 🔵 Low | `Yes, try it` 토큰 추가는 중복이며 평문 오탐 면적만 넓힘 |
| N-2 | 🔵 Low | `_init_herdr_isolation` stub 무효 → 테스트가 `.mam/shim` 실제 기록 |
| N-3 | 🔵 Low | `env_extra` 미사용 파라미터 |
| N-4 | 🔵 Low | `/tmp` 하드코딩, 실패 시 잔여 가능 |
| N-5 | 🔵 Low | `FIX.md` untracked |
핵심 결함은 모두 해소되었고 잔여는 전부 위생 수준이므로 병합 가능하다고 판단한다.
설계 재작업 사유가 없어 PLANNER 에스컬레이션은 두지 않는다.
[VERDICT: PASS]