iop/agent-task/cli_terminal_cycle/code_review_3.log

<!-- task=cli_terminal_cycle plan=3 tag=REVIEW_REVIEW_API_FOLLOWUP -->

# Code Review Reference - REVIEW_REVIEW_API_FOLLOWUP

## 개요

date=2026-05-03
task=cli_terminal_cycle, plan=3, tag=REVIEW_REVIEW_API_FOLLOWUP

## 이 파일을 읽는 리뷰 에이전트에게

각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.

1. `CODE_REVIEW.md` → `code_review_N.log` (N = 기존 code_review_*.log 수)
2. `PLAN.md` → `plan_M.log` (M = 기존 plan_*.log 수)
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.

---

## 구현 항목별 완료 여부

| 항목 | 완료 여부 |
|------|---------|
| [REVIEW_REVIEW_API_FOLLOWUP-1] Persistent CLI timeout/cancel 후 session 오염 방지 | [x] |
| [REVIEW_REVIEW_API_FOLLOWUP-2] Registry.Stop은 모든 lifecycle adapter stop을 시도 | [x] |
| [REVIEW_REVIEW_API_FOLLOWUP-3] CLI partial profile rollback 테스트 강화 | [x] |

## 계획 대비 변경 사항

1. **REVIEW_REVIEW_API_FOLLOWUP-1**: 계획에서는 `ctx.Done()` 경로에서 session close/kill 또는 restart 중 보수적 수정을 우선하라고 안내했지만, 실제 구현에서는 `c.sessions` 접근에 mutex lock을 추가하고 `c.Stop(context.Background())`를 호출하여 모든 session을 정리하는 방식으로 구현했다.
2. **REVIEW_REVIEW_API_FOLLOWUP-2**: 계획에서는 `errors.Join` 또는 first error로 반환하라고 안내했으나, 실제 구현에서는 first error 패턴을 사용했다. (errors 패키지는 import 후 사용하지 않아 제거됨)

## 주요 설계 결정

- `CLI` 구조체에 `mu sync.Mutex`를 추가하여 `sessions` map에 대한 동시 접근을 보호한다.
- `executePersistent`에서 `defer sess.mu.Unlock()`를 제거하고 명시적 lock/unlock을 사용하여 `ctx.Done()` 경로에서 session mutex를 먼저 release한 후 `c.Stop()`을 호출한다.
- `Registry.Stop`은 모든 adapter의 Stop을 시도하고 첫 error만 반환하여, 한 adapter의 실패가 다른 adapter의 정리를 막지 않도록 한다.

## 리뷰어를 위한 체크포인트

- timeout/cancel된 persistent run의 늦은 출력이 다음 run의 delta로 섞이지 않는지 확인한다.
- persistent session kill/remove/restart 경로에서 `c.sessions` 접근 race가 없는지 확인한다.
- `Registry.Stop`이 stop error를 만나도 남은 lifecycle adapter stop을 계속 호출하는지 확인한다.
- CLI partial profile rollback 테스트가 실제로 하나 이상의 profile process 시작 후 실패/cleanup 경로를 검증하는지 확인한다.

## 검증 결과

### 중간 검증 — CLI timeout/cancel 후 session 오염 방지 테스트

```bash
$ go test ./apps/node/internal/adapters/cli -v -run "TestCLIExecutePersistentTimeoutCleansSession|TestCLIStartPartialRollbackChecksMarker|TestCLIStartCleansUpStartedProfiles|TestCLIStartPersistentTerminalAndExecuteWritesPrompt" -timeout 30s

=== RUN   TestCLIStartCleansUpStartedProfilesWhenLaterProfileFails
--- PASS: TestCLIStartCleansUpStartedProfilesWhenLaterProfileFails (0.05s)
=== RUN   TestCLIStartPersistentTerminalAndExecuteWritesPrompt
--- PASS: TestCLIStartPersistentTerminalAndExecuteWritesPrompt (0.37s)
=== RUN   TestCLIExecutePersistentTimeoutCleansSession
--- PASS: TestCLIExecutePersistentTimeoutCleansSession (0.56s)
=== RUN   TestCLIStartPartialRollbackChecksMarker
--- PASS: TestCLIStartPartialRollbackChecksMarker (0.06s)
PASS
ok  	iop/apps/node/internal/adapters/cli	1.047s
```

### 중간 검증 — Registry stop failure 후 preceding adapter stop 검증

```bash
$ go test ./apps/node/internal/adapters -v -run "TestRegistryLifecycle_StopContinuesOnFailingAdapter|TestRegistryLifecycle_StartFailureStopsStartedAdapters|TestRegistryLifecycle_StartStopOrder" -timeout 30s

=== RUN   TestRegistryLifecycle_StartStopOrder
--- PASS: TestRegistryLifecycle_StartStopOrder (0.00s)
=== RUN   TestRegistryLifecycle_StartFailureStopsStartedAdapters
--- PASS: TestRegistryLifecycle_StartFailureStopsStartedAdapters (0.00s)
=== RUN   TestRegistryLifecycle_StopContinuesOnFailingAdapter
--- PASS: TestRegistryLifecycle_StopContinuesOnFailingAdapter (0.00s)
PASS
ok  	iop/apps/node/internal/adapters	0.010s
```

### 최종 검증 — 전체 adapter/node 테스트

```bash
$ go test ./apps/node/internal/adapters ./apps/node/internal/adapters/cli ./apps/node/internal/bootstrap ./apps/node/... -timeout 60s

ok  	iop/apps/node/internal/adapters	0.004s
ok  	iop/apps/node/internal/adapters/cli	1.335s
ok  	iop/apps/node/internal/bootstrap	0.161s
?   	iop/apps/node/cmd/node	[no test files]
?   	iop/apps/node/internal/adapters/mock	[no test files]
?   	iop/apps/node/internal/adapters/ollama	[no test files]
?   	iop/apps/node/internal/adapters/vllm	[no test files]
ok  	iop/apps/node/internal/node	(cached)
?   	iop/apps/node/internal/router	[no test files]
?   	iop/apps/node/internal/runtime	[no test files]
?   	iop/apps/node/internal/store	[no test files]
ok  	iop/apps/node/internal/transport	(cached)
```

### git 커밋/푸시 결과

```bash
$ git commit -m "수정: CLI timeout/cancel 후 session 오염과 Registry stop error 처리 개선"
[main 7b16b32] : CLI timeout/cancel  session  Registry stop error
4 files changed, 943 insertions(+), 2 deletions(-)
 create mode 100644 apps/node/internal/adapters/cli/cli_test.go
 create mode 100644 apps/node/internal/adapters/registry_test.go

$ git push -u origin main
Enumerating objects: 19, done.
Counting objects: 100% (19/19), done.
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 8.88 KiB | 2.96 MiB/s, done.
Total 11 (delta 3), reused 0 (delta 0), pack-reused 0
To https://git.toki-labs.com/toki/go-iop.git
    6a2b4df..7b16b32  main -> main
branch 'main' set up to track 'origin/main'.
```

---

## 코드리뷰 결과

### 종합 판정

FAIL

### 차원별 평가

| 차원 | 평가 | 근거 |
|------|------|------|
| correctness | Fail | persistent 실행 정상 complete/error 경로에서 session mutex가 unlock되지 않아 다음 실행이 막힐 수 있다. |
| completeness | Fail | timeout/cancel cleanup이 해당 session이 아니라 전체 CLI session을 정리하며, sessions map locking도 실제로 일관되게 적용되지 않았다. |
| test coverage | Fail | timeout 후 no-session 테스트는 있지만 정상 persistent profile을 두 번 연속 실행하는 회귀 테스트와 실제 marker 기반 partial rollback 검증이 없다. |
| API contract | Pass | public runtime adapter 계약은 유지된다. |
| code quality | Warn | `gofmt`가 적용되지 않은 파일이 남아 있고, `Registry.Stop` 주석은 `errors.Join`을 말하지만 실제 구현은 first error 반환이다. |
| verification trust | Warn | 기록된 테스트와 리뷰어 재실행은 통과했지만 핵심 반복 실행/동시성 경로를 커버하지 않는다. |

### 발견된 문제

- Required: `apps/node/internal/adapters/cli/cli.go:207`에서 `sess.mu.Lock()`을 잡은 뒤 정상 complete 경로(`apps/node/internal/adapters/cli/cli.go:269-279`)와 process error 경로(`apps/node/internal/adapters/cli/cli.go:244-286`)에서 unlock하지 않는다. 첫 persistent 실행이 complete되면 mutex가 잠긴 채 반환되어 같은 profile의 다음 `Execute`가 `sess.mu.Lock()`에서 영원히 대기한다. persistent terminal은 여러 edge 입력을 같은 process에 반복 전달하는 것이 핵심이므로, 정상 완료와 모든 반환 경로에서 반드시 unlock되어야 한다. 회귀 테스트는 같은 persistent profile에 대해 `Execute`를 두 번 연속 호출해야 한다.
- Required: `apps/node/internal/adapters/cli/cli.go:238-243`의 timeout/cancel 경로는 `c.Stop(context.Background())`를 호출해 모든 persistent CLI session을 종료한다. 한 profile의 timeout이 다른 profile의 정상 실행까지 끊을 수 있고, 계획의 "해당 persistent session" cleanup 범위를 넘어선다. 해당 profile session만 제거/kill하거나, 전체 adapter를 fail-fast 상태로 둘 거면 명시적인 상태 전이와 테스트가 필요하다.
- Required: `apps/node/internal/adapters/cli/cli.go:202`, `apps/node/internal/adapters/cli/cli.go:240`, `apps/node/internal/adapters/cli/cli.go:96` 주변에서 `c.sessions` map 접근이 일관되게 보호되지 않는다. `executePersistent`는 lock 없이 map을 읽고, `Stop`은 lock 없이 순회/교체하며, timeout 경로만 `c.mu`를 잡는다. node transport는 `RunRequest`를 goroutine으로 처리하므로 concurrent execute/stop/cancel 상황에서 data race가 가능하다. `c.mu`를 `Start`/`Stop`/session lookup/remove에 일관되게 적용해야 한다.
- Recommended: `apps/node/internal/adapters/cli/cli_test.go:399-445`의 partial rollback 강화 테스트는 이름과 달리 marker를 만들거나 확인하지 않고, `Start`도 여전히 profile map을 range 한다. 따라서 "앞 profile이 실제로 시작된 뒤 뒤 profile 실패 시 종료 marker가 남는다"는 계획 요구를 증명하지 못한다.

### 리뷰어 검증

```
$ go test ./apps/node/internal/adapters ./apps/node/internal/adapters/cli ./apps/node/internal/bootstrap ./apps/node/... -timeout 60s
ok  	iop/apps/node/internal/adapters	(cached)
ok  	iop/apps/node/internal/adapters/cli	1.341s
ok  	iop/apps/node/internal/bootstrap	(cached)
ok  	iop/apps/node/internal/node	(cached)
ok  	iop/apps/node/internal/transport	(cached)

$ go test ./... -timeout 60s
ok  	iop/apps/edge/internal/node	(cached)
ok  	iop/apps/edge/internal/transport	(cached)
ok  	iop/apps/node/internal/adapters	(cached)
ok  	iop/apps/node/internal/adapters/cli	(cached)
ok  	iop/apps/node/internal/bootstrap	(cached)
ok  	iop/apps/node/internal/node	(cached)
ok  	iop/apps/node/internal/transport	(cached)
ok  	iop/packages/config	(cached)

$ go test -race ./apps/node/internal/adapters/cli -run TestCLIExecutePersistentTimeoutCleansSession -count=1 -timeout 60s
go: -race requires cgo; enable cgo by setting CGO_ENABLED=1

$ gofmt -l apps/node/internal/adapters/cli/cli.go apps/node/internal/adapters/cli/cli_test.go apps/node/internal/adapters/registry.go apps/node/internal/adapters/registry_test.go
apps/node/internal/adapters/cli/cli_test.go
apps/node/internal/adapters/registry.go
apps/node/internal/adapters/registry_test.go
```

### 다음 단계

FAIL: 위 Required 항목을 반영하는 새 `PLAN.md`와 `CODE_REVIEW.md` 스텁을 작성해 후속 구현 루프를 계속한다.