117 lines
No EOL
5.8 KiB
Text
117 lines
No EOL
5.8 KiB
Text
<!-- 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'. |