126 lines
7.3 KiB
Text
126 lines
7.3 KiB
Text
<!-- task=cli_terminal_cycle plan=0 tag=API -->
|
|
|
|
# Code Review Reference - API
|
|
|
|
## 개요
|
|
|
|
date=2026-05-03
|
|
task=cli_terminal_cycle, plan=0, tag=API
|
|
|
|
## 이 파일을 읽는 리뷰 에이전트에게
|
|
|
|
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
|
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
|
|
|
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` 스텁 작성.
|
|
|
|
---
|
|
|
|
## 구현 항목별 완료 여부
|
|
|
|
| 항목 | 완료 여부 |
|
|
|------|---------|
|
|
| [API-1] CLI profile 설정 계약에 persistent terminal 옵션 추가 | [x] |
|
|
| [API-2] CLI adapter에 node-start persistent terminal process 구현 | [x] |
|
|
| [API-3] Node bootstrap에서 adapter lifecycle 시작/정리 연결 | [x] |
|
|
| [API-4] Edge 콘솔 수동 테스트를 `cli/claude` 1-cycle에 맞게 정리 | [x] |
|
|
|
|
## 계획 대비 변경 사항
|
|
|
|
없음 — 계획에 명시된 모든 항목을 계획대로 구현했다.
|
|
|
|
## 주요 설계 결정
|
|
|
|
- `drainUntilIdle`은 synchronous하게 `Start()` 내에서 호출해 startup banner가 완전히 제거된 뒤 `Start()`가 반환되도록 했다.
|
|
- PTY reader goroutine에서 `strings.TrimRight(line, "\r")`로 PTY의 `\r\n` 줄 끝을 정규화한다.
|
|
- `executePersistent`의 `done` channel select는 process 종료를 detected하고 complete event를 emit한 뒤 반환한다. 이후 호출에서는 closed output channel이 `!ok`를 반환해 error로 처리된다.
|
|
- `LifecycleAdapter` interface는 optional로 registry가 type assertion으로 확인하므로 기존 mock/ollama/vllm adapter에 변경이 없다.
|
|
|
|
## 리뷰어를 위한 체크포인트
|
|
|
|
- `CLIProfileConf` 새 필드가 edge YAML, edge payload, node payload parser에서 모두 동일한 이름과 의미로 roundtrip 되는지 확인한다.
|
|
- `cli` adapter의 기존 one-shot 동작이 `persistent=false` profile에서 깨지지 않았는지 확인한다.
|
|
- `persistent=true`, `terminal=true` profile이 node bootstrap 시점에 시작되고 node shutdown 시 정리되는지 확인한다.
|
|
- persistent CLI response 경계가 context timeout, process exit, idle timeout을 모두 안전하게 처리하는지 확인한다.
|
|
- registry lifecycle이 optional interface로 구현되어 mock/ollama/vllm adapter에 불필요한 의존을 만들지 않는지 확인한다.
|
|
- `configs/edge.yaml` 기본값이 `cli/claude` 수동 테스트 목적과 일치하며, README의 절차가 실제 command/config와 맞는지 확인한다.
|
|
|
|
## 검증 결과
|
|
|
|
_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._
|
|
|
|
### API-1 중간 검증
|
|
```
|
|
$ go test ./apps/edge/internal/transport ./apps/node/internal/adapters
|
|
ok iop/apps/edge/internal/transport 0.007s
|
|
ok iop/apps/node/internal/adapters 0.003s
|
|
```
|
|
|
|
### API-2 중간 검증
|
|
```
|
|
$ go test ./apps/node/internal/adapters/cli
|
|
ok iop/apps/node/internal/adapters/cli 0.368s
|
|
```
|
|
|
|
### API-3 중간 검증
|
|
```
|
|
$ go test ./apps/node/internal/adapters ./apps/node/...
|
|
ok iop/apps/node/internal/adapters
|
|
ok iop/apps/node/internal/adapters/cli
|
|
ok iop/apps/node/internal/node
|
|
ok iop/apps/node/internal/transport
|
|
```
|
|
|
|
### API-4 중간 검증
|
|
```
|
|
$ go test ./packages/config ./apps/edge/cmd/edge
|
|
ok iop/packages/config 0.002s
|
|
? iop/apps/edge/cmd/edge [no test files]
|
|
```
|
|
|
|
### 최종 검증
|
|
```
|
|
$ go mod tidy && go test ./...
|
|
ok iop/apps/edge/internal/node
|
|
ok iop/apps/edge/internal/transport
|
|
ok iop/apps/node/internal/adapters
|
|
ok iop/apps/node/internal/adapters/cli
|
|
ok iop/apps/node/internal/node
|
|
ok iop/apps/node/internal/transport
|
|
ok iop/packages/config
|
|
(그 외 패키지는 [no test files])
|
|
```
|
|
|
|
수동 검증: node 호스트 PATH에 `claude` 명령이 있어야 실행 가능하며, 자동 CI 환경에서는 불가.
|
|
|
|
---
|
|
|
|
## 코드리뷰 결과
|
|
|
|
### 종합 판정
|
|
|
|
FAIL
|
|
|
|
### 차원별 평가
|
|
|
|
| 차원 | 평가 | 근거 |
|
|
|------|------|------|
|
|
| correctness | Fail | persistent CLI가 첫 출력 전에 idle complete를 보낼 수 있고, 프로세스 종료/오류가 실패로 전파되지 않는다. |
|
|
| completeness | Fail | 계획의 "첫 output 이후 idle", "프로세스 종료는 error event", "adapter start 이후 실패 cleanup" 요구가 완전히 충족되지 않았다. |
|
|
| test coverage | Fail | slow first output, persistent process exit/error, bootstrap post-start failure cleanup 회귀 테스트가 없다. |
|
|
| API contract | Pass | CLI profile 설정 필드는 edge payload와 node parser까지 연결되어 있다. |
|
|
| code quality | Warn | lifecycle 자체는 작게 들어갔지만 persistent session 상태 전이가 아직 불명확하다. |
|
|
| plan deviation | Fail | `reg.Start` 위치와 error handling이 계획과 다르고, persistent response 경계가 계획과 다르다. |
|
|
| verification trust | Warn | `go test ./...`는 통과했지만 핵심 실패 시나리오가 테스트에 없다. 리뷰어가 재실행한 `go test ./...`도 통과했다. |
|
|
|
|
### 발견된 문제
|
|
|
|
- Required: `apps/node/internal/adapters/cli/cli.go:211`에서 idle timer를 prompt write 직후 시작하고 `apps/node/internal/adapters/cli/cli.go:242`에서 타이머 만료를 complete로 처리한다. 계획은 "첫 output 이후 `ResponseIdleTimeoutMS` 동안 새 output이 없으면 complete"였는데, 현재는 Claude가 첫 응답을 1.5초보다 늦게 내면 빈 응답으로 run을 완료하고, 뒤늦은 출력은 다음 run의 delta로 섞일 수 있다. 수정: 첫 `sess.output` 수신 전에는 idle timer를 비활성화하고, 첫 출력 이후에만 idle timer를 arm/reset한다. 첫 출력이 영원히 없을 때는 `ctx`/`TimeoutSec`로만 종료되게 하고, slow-first-output 테스트를 추가한다.
|
|
- Required: `apps/node/internal/adapters/cli/cli.go:219`의 output channel close 경로는 `EventTypeError`를 emit해도 `sink.Emit`의 nil을 그대로 반환하므로 `apps/node/internal/node/node.go:100-106`에서 run이 completed로 저장된다. `apps/node/internal/adapters/cli/cli.go:253`의 `sess.done` 경로도 `err != nil`을 complete event message로만 바꿔 실패로 전파하지 않는다. 수정: persistent process 종료/reader close/write failure는 `EventTypeError`를 emit한 뒤 non-nil error를 반환하고, persistent process가 정상 종료되어도 재사용 불가능한 session이면 error로 다룬다. 해당 회귀 테스트를 추가한다.
|
|
- Required: `apps/node/internal/bootstrap/module.go:50`에서 `reg.Start(ctx)`가 store 초기화보다 먼저 실행되지만, 이후 `storeDSN`/`store.New` 실패 경로(`apps/node/internal/bootstrap/module.go:55-63`)는 session만 닫고 `reg.Stop`을 호출하지 않는다. workspace 생성이나 SQLite open 실패 시 node 시작은 실패하지만 persistent Claude process가 남을 수 있다. 수정: `reg.Start`를 store 생성 이후로 옮기거나, start 이후 모든 실패 경로에서 `reg.Stop`을 호출하도록 cleanup guard를 둔다. 실패 cleanup 테스트 또는 최소한 registry-level fake lifecycle 테스트를 추가한다.
|
|
|
|
### 다음 단계
|
|
|
|
FAIL: Required 항목을 반영하는 새 `PLAN.md`와 `CODE_REVIEW.md`를 작성해 후속 구현 루프를 계속한다.
|