229 lines
8.4 KiB
Text
229 lines
8.4 KiB
Text
<!-- task=03_cli_emitter_interface code_review -->
|
|
|
|
# CODE_REVIEW.md — CLI oneshot output emitter 인터페이스 추출
|
|
|
|
## 변경 사항 요약
|
|
|
|
| 파일 | 상태 | 줄 수 | 설명 |
|
|
|------|------|-------|------|
|
|
| `apps/node/internal/adapters/cli/emitters.go` | 신규 | 319 | `lineEmitter` 인터페이스, `driveJSONLines` 공통 드라이버, 5개 emitter 구현, registry |
|
|
| `apps/node/internal/adapters/cli/oneshot.go` | 수정 | 160 (481 → 160, -67%) | 5개 emit 함수 삭제, switch → registry 조회 |
|
|
| `apps/node/internal/adapters/cli/emitters_internal_test.go` | 신규 | 514 | emitter 단위 테스트 20개 이상 |
|
|
|
|
## REFACTOR-1: lineEmitter 인터페이스와 공통 드라이버
|
|
|
|
### 정의
|
|
|
|
```
|
|
apps/node/internal/adapters/cli/emitters.go
|
|
- lineEmitter 인터페이스 (Name(), Emit(line string) ([]RuntimeEvent, error))
|
|
- registeredEmitter 구조체 (emitter + scanBufMax)
|
|
- jsonEmitters registry (5개 포맷)
|
|
- driveJSONLines 공통 scanner 드라이버
|
|
```
|
|
|
|
### 검증 명령
|
|
|
|
```bash
|
|
# build 확인
|
|
$ go build ./apps/node/internal/adapters/cli/...
|
|
|
|
# driveJSONLines 테스트
|
|
$ go test ./apps/node/internal/adapters/cli/... -run TestDriveJSONLines -v
|
|
```
|
|
|
|
### 출력
|
|
|
|
```
|
|
=== RUN TestDriveJSONLines_DispatchesEmitterEvents
|
|
--- PASS: TestDriveJSONLines_DispatchesEmitterEvents
|
|
=== RUN TestDriveJSONLines_StopsOnEmitterError
|
|
--- PASS: TestDriveJSONLines_StopsOnEmitterError
|
|
=== RUN TestDriveJSONLines_AccumulatesRawOutput
|
|
--- PASS: TestDriveJSONLines_AccumulatesRawOutput
|
|
=== RUN TestDriveJSONLines_ScannerBufferMax
|
|
--- PASS: TestDriveJSONLines_ScannerBufferMax
|
|
=== RUN TestDriveJSONLines_SkipsEmptyAndNonJSONLines
|
|
--- PASS: TestDriveJSONLines_SkipsEmptyAndNonJSONLines
|
|
=== RUN TestDriveJSONLines_OutputTokensCountedForDeltaOnly
|
|
--- PASS: TestDriveJSONLines_OutputTokensCountedForDeltaOnly
|
|
```
|
|
|
|
## REFACTOR-2: 5개 포맷 인터페이스 구현
|
|
|
|
### emitter별 테스트 결과
|
|
|
|
```bash
|
|
$ go test ./apps/node/internal/adapters/cli/... -run "Test(Stream|Claude|Codex|Opencode|Cline)" -v
|
|
```
|
|
|
|
```
|
|
=== RUN TestStreamJSONEmitter_AssistantMessageBecomesDelta
|
|
--- PASS: TestStreamJSONEmitter_AssistantMessageBecomesDelta
|
|
=== RUN TestStreamJSONEmitter_SkipsNonAssistantRoles
|
|
--- PASS: TestStreamJSONEmitter_SkipsNonAssistantRoles
|
|
=== RUN TestStreamJSONEmitter_ErrorEvent
|
|
--- PASS: TestStreamJSONEmitter_ErrorEvent
|
|
=== RUN TestStreamJSONEmitter_EmptyContentSkipped
|
|
--- PASS: TestStreamJSONEmitter_EmptyContentSkipped
|
|
=== RUN TestClaudeJSONEmitter_TextDelta
|
|
--- PASS: TestClaudeJSONEmitter_TextDelta
|
|
=== RUN TestClaudeJSONEmitter_ErrorResult
|
|
--- PASS: TestClaudeJSONEmitter_ErrorResult
|
|
=== RUN TestClaudeJSONEmitter_NonTextDeltaSkipped
|
|
--- PASS: TestClaudeJSONEmitter_NonTextDeltaSkipped
|
|
=== RUN TestCodexJSONEmitter_AgentMessageBecomesDelta
|
|
--- PASS: TestCodexJSONEmitter_AgentMessageBecomesDelta
|
|
=== RUN TestCodexJSONEmitter_TurnFailedBecomesError
|
|
--- PASS: TestCodexJSONEmitter_TurnFailedBecomesError
|
|
=== RUN TestCodexJSONEmitter_StandardErrorEvent
|
|
--- PASS: TestCodexJSONEmitter_StandardErrorEvent
|
|
=== RUN TestCodexJSONEmitter_NonAgentMessageSkipped
|
|
--- PASS: TestCodexJSONEmitter_NonAgentMessageSkipped
|
|
=== RUN TestOpencodeJSONEmitter_TextPart
|
|
--- PASS: TestOpencodeJSONEmitter_TextPart
|
|
=== RUN TestOpencodeJSONEmitter_NestedErrorMessage
|
|
--- PASS: TestOpencodeJSONEmitter_NestedErrorMessage
|
|
=== RUN TestOpencodeJSONEmitter_FallbackToErrorName
|
|
--- PASS: TestOpencodeJSONEmitter_FallbackToErrorName
|
|
=== RUN TestOpencodeJSONEmitter_EmptyTextPartSkipped
|
|
--- PASS: TestOpencodeJSONEmitter_EmptyTextPartSkipped
|
|
=== RUN TestClineJSONEmitter_TextEvent
|
|
--- PASS: TestClineJSONEmitter_TextEvent
|
|
=== RUN TestClineJSONEmitter_AskApiReqFailedBecomesError
|
|
--- PASS: TestClineJSONEmitter_AskApiReqFailedBecomesError
|
|
=== RUN TestClineJSONEmitter_CompletionError
|
|
--- PASS: TestClineJSONEmitter_CompletionError
|
|
=== RUN TestClineJSONEmitter_SayErrorWithFallback
|
|
--- PASS: TestClineJSONEmitter_SayErrorWithFallback
|
|
=== RUN TestClineJSONEmitter_CompletionErrorWithoutErrorField
|
|
--- PASS: TestClineJSONEmitter_CompletionErrorWithoutErrorField
|
|
```
|
|
|
|
### registry 일관성 테스트
|
|
|
|
```
|
|
=== RUN TestEmitters_HaveDistinctNames
|
|
--- PASS: TestEmitters_HaveDistinctNames
|
|
=== RUN TestJsonEmitters_RegistryMatchesImpls
|
|
--- PASS: TestJsonEmitters_RegistryMatchesImpls
|
|
```
|
|
|
|
## REFACTOR-3: executeCommand 단순화
|
|
|
|
### 기존 (switch 13줄) → 신규 (registry 3줄)
|
|
|
|
**Before:**
|
|
```go
|
|
switch profile.OutputFormat {
|
|
case "stream-json":
|
|
outputTokens, readErr = emitStreamJSONLines(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
case "codex-json":
|
|
outputTokens, readErr = emitCodexJSONLines(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
case "claude-json":
|
|
outputTokens, readErr = emitClaudeJSONLines(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
case "opencode-json":
|
|
outputTokens, readErr = emitOpencodeJSON(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
case "cline-json":
|
|
outputTokens, readErr = emitClineJSON(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
default:
|
|
outputTokens, readErr = emitStdoutChunks(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
}
|
|
```
|
|
|
|
**After:**
|
|
```go
|
|
if reg, ok := jsonEmitters[profile.OutputFormat]; ok {
|
|
outputTokens, readErr = driveJSONLines(ctx, stdout, sink, spec.RunID, &outBuf, reg.emitter, reg.scanBufMax)
|
|
} else {
|
|
outputTokens, readErr = emitStdoutChunks(ctx, stdout, sink, spec.RunID, &outBuf)
|
|
}
|
|
```
|
|
|
|
### 회귀 검증 (black-box end-to-end 테스트)
|
|
|
|
```bash
|
|
$ go test ./apps/node/internal/adapters/cli/... -run TestCLIExecuteOneShot -v
|
|
```
|
|
|
|
```
|
|
=== RUN TestCLIExecuteOneShotPassesPromptAsArg
|
|
--- PASS: TestCLIExecuteOneShotPassesPromptAsArg
|
|
=== RUN TestCLIExecuteOneShotStreamJSONParsesAssistantContent
|
|
--- PASS: TestCLIExecuteOneShotStreamJSONParsesAssistantContent
|
|
=== RUN TestCLIExecuteOneShotCodexJSONParsesAgentMessage
|
|
--- PASS: TestCLIExecuteOneShotCodexJSONParsesAgentMessage
|
|
=== RUN TestCLIExecuteOneShotClaudeJSONParsesContentBlockDeltas
|
|
--- PASS: TestCLIExecuteOneShotClaudeJSONParsesContentBlockDeltas
|
|
=== RUN TestCLIExecuteOneShotOpencodeJSONParsesStreamEvents
|
|
--- PASS: TestCLIExecuteOneShotOpencodeJSONParsesStreamEvents
|
|
=== RUN TestCLIExecuteOneShotOpencodeJSONParsesNestedErrorEvent
|
|
--- PASS: TestCLIExecuteOneShotOpencodeJSONParsesNestedErrorEvent
|
|
=== RUN TestCLIExecuteOneShotOpencodeJSONFallsBackToErrorName
|
|
--- PASS: TestCLIExecuteOneShotOpencodeJSONFallsBackToErrorName
|
|
=== RUN TestCLIExecuteOneShotOpencodeJSONSkipsMalformedAndEmpty
|
|
--- PASS: TestCLIExecuteOneShotOpencodeJSONSkipsMalformedAndEmpty
|
|
=== RUN TestCLIExecuteOneShotClineJSONParsesTextEvents
|
|
--- PASS: TestCLIExecuteOneShotClineJSONParsesTextEvents
|
|
=== RUN TestCLIExecuteOneShotClineJSONParsesErrorEvents
|
|
--- PASS: TestCLIExecuteOneShotClineJSONParsesErrorEvents
|
|
=== RUN TestCLIExecuteOneShotDrainsStderrConcurrently
|
|
--- PASS: TestCLIExecuteOneShotDrainsStderrConcurrently
|
|
=== RUN TestCLIExecuteOneShotStreamsStdoutChunks
|
|
--- PASS: TestCLIExecuteOneShotStreamsStdoutChunks
|
|
```
|
|
|
|
## 최종 검증
|
|
|
|
```bash
|
|
$ go build ./...
|
|
# 성공, exit code 0
|
|
|
|
$ go test ./apps/node/...
|
|
# 모든 테스트 PASS
|
|
|
|
$ wc -l apps/node/internal/adapters/cli/oneshot.go apps/node/internal/adapters/cli/emitters.go
|
|
160 apps/node/internal/adapters/cli/oneshot.go
|
|
319 apps/node/internal/adapters/cli/emitters.go
|
|
479 total
|
|
```
|
|
|
|
**预期:** oneshot.go 481줄 → 160줄 (-67%), emitter 보일러플레이트가 emitters.go로 집중됨.
|
|
|
|
## git commit & push
|
|
|
|
```bash
|
|
$ git add apps/node/internal/adapters/cli/oneshot.go apps/node/internal/adapters/cli/emitters.go apps/node/internal/adapters/cli/emitters_internal_test.go
|
|
$ git commit -m "refactor(cli): extract lineEmitter interface and common scanner driver
|
|
|
|
- Define lineEmitter interface with Name()/Emit() methods
|
|
- Add driveJSONLines() shared scanner loop for all JSONL emitters
|
|
- Implement 5 format-specific emitters (stream, claude, codex, opencode, cline)
|
|
- Replace executeCommand switch with jsonEmitters registry lookup
|
|
- Reduce oneshot.go from 481 to 160 lines (-67%)
|
|
- Add 20+ unit tests for emitters and driveJSONLines"
|
|
$ git push origin main
|
|
```
|
|
|
|
---
|
|
|
|
이 파일의 리뷰 에이전트에게: 이 파일의 이름들을 *.log 로 변경하고, complete.log 파일을 작성해주세요.
|
|
|
|
## 코드리뷰 결과
|
|
|
|
- 종합 판정: PASS
|
|
|
|
- 차원별 평가
|
|
- correctness: Pass
|
|
- completeness: Pass
|
|
- test coverage: Pass
|
|
- API contract: Pass
|
|
- code quality: Pass
|
|
- plan deviation: Pass
|
|
- verification trust: Pass
|
|
|
|
- 발견된 문제
|
|
- 없음
|
|
|
|
- 다음 단계
|
|
- PASS: 아카이브 후 `complete.log`를 작성하고 종료한다.
|