365 lines
13 KiB
Text
365 lines
13 KiB
Text
<!-- task=cli_terminal_cycle plan=1 tag=REVIEW_API -->
|
|
|
|
# Persistent CLI 안정화 후속 수정
|
|
|
|
## 이 파일을 읽는 구현 에이전트에게
|
|
|
|
각 항목의 체크리스트를 완료하면 즉시 체크 표시한다. 중간/최종 검증 명령을 실제로 실행하고 출력을 `CODE_REVIEW.md`의 검증 결과 섹션에 붙여 넣는다. 계획과 다르게 구현해야 하는 부분이 생기면 이유를 `CODE_REVIEW.md`의 `계획 대비 변경 사항`에 남기고, 모든 구현 항목의 실제 동작과 수동 검증 결과를 기록한다.
|
|
|
|
## 배경
|
|
|
|
1차 구현은 CLI profile 설정과 node-start persistent process 골격을 만들었지만, persistent response 경계와 실패 전파가 실제 Claude CLI 원격 제어 목표에 맞지 않는다. 특히 현재 구현은 첫 출력 전에 idle timer가 만료될 수 있고, persistent process 종료를 run 실패로 저장하지 않는다. 또한 adapter lifecycle 시작 후 bootstrap이 실패하는 경로에서 프로세스 정리가 누락될 수 있다.
|
|
|
|
## 의존 관계 및 구현 순서
|
|
|
|
1. [REVIEW_API-1]에서 persistent response boundary를 먼저 바로잡는다.
|
|
2. [REVIEW_API-2]에서 persistent process exit/error를 error event + non-nil error로 통일한다.
|
|
3. [REVIEW_API-3]에서 node bootstrap lifecycle 시작 순서와 실패 cleanup을 정리한다.
|
|
|
|
---
|
|
|
|
### [REVIEW_API-1] Persistent CLI idle complete는 첫 출력 이후에만 시작
|
|
|
|
#### 문제
|
|
|
|
`apps/node/internal/adapters/cli/cli.go:211-243`:
|
|
```go
|
|
timer := time.NewTimer(idleTimeout)
|
|
defer timer.Stop()
|
|
outputTokens := 0
|
|
|
|
for {
|
|
select {
|
|
case <-ctx.Done():
|
|
return ctx.Err()
|
|
case out, ok := <-sess.output:
|
|
// delta emit
|
|
if !timer.Stop() {
|
|
select {
|
|
case <-timer.C:
|
|
default:
|
|
}
|
|
}
|
|
timer.Reset(idleTimeout)
|
|
case <-timer.C:
|
|
return sink.Emit(ctx, runtime.RuntimeEvent{
|
|
RunID: spec.RunID,
|
|
Type: runtime.EventTypeComplete,
|
|
```
|
|
|
|
타이머가 prompt write 직후 시작된다. 계획은 "첫 output 이후 `ResponseIdleTimeoutMS` 동안 새 output이 없으면 complete"였으나, 현재는 Claude가 첫 응답을 `response_idle_timeout_ms`보다 늦게 내면 빈 응답으로 run이 완료된다. 뒤늦은 출력은 session output channel에 남아 다음 run의 delta로 섞일 수 있다.
|
|
|
|
#### 해결 방법
|
|
|
|
첫 output 수신 전에는 idle timer channel을 nil로 두고, 첫 delta를 emit한 뒤에만 timer를 arm한다. 첫 출력이 없는 경우는 `node.Node`가 만든 `ctx` timeout(`RunRequest.TimeoutSec`)이나 cancel request로만 종료된다.
|
|
|
|
Before (`apps/node/internal/adapters/cli/cli.go:211-243`):
|
|
```go
|
|
timer := time.NewTimer(idleTimeout)
|
|
defer timer.Stop()
|
|
...
|
|
case out, ok := <-sess.output:
|
|
...
|
|
timer.Reset(idleTimeout)
|
|
case <-timer.C:
|
|
return sink.Emit(ctx, runtime.RuntimeEvent{Type: runtime.EventTypeComplete})
|
|
```
|
|
|
|
After:
|
|
```go
|
|
var idleTimer *time.Timer
|
|
var idleC <-chan time.Time
|
|
defer func() {
|
|
if idleTimer != nil {
|
|
idleTimer.Stop()
|
|
}
|
|
}()
|
|
...
|
|
case out, ok := <-sess.output:
|
|
...
|
|
if idleTimer == nil {
|
|
idleTimer = time.NewTimer(idleTimeout)
|
|
} else {
|
|
if !idleTimer.Stop() {
|
|
select {
|
|
case <-idleTimer.C:
|
|
default:
|
|
}
|
|
}
|
|
idleTimer.Reset(idleTimeout)
|
|
}
|
|
idleC = idleTimer.C
|
|
case <-idleC:
|
|
return emitComplete(ctx, sink, spec.RunID, prompt, outputTokens)
|
|
```
|
|
|
|
`emitComplete` helper를 만들면 [REVIEW_API-2]의 error helper와 함께 이벤트 생성 중복을 줄일 수 있다. helper가 과하다고 판단되면 현재 inline 구조를 유지해도 된다.
|
|
|
|
#### 수정 파일 및 체크리스트
|
|
|
|
- [ ] `apps/node/internal/adapters/cli/cli.go` - persistent idle timer를 첫 output 이후에만 arm하도록 변경
|
|
- [ ] `apps/node/internal/adapters/cli/cli.go` - complete event 생성 중복을 줄이는 helper 추가 또는 기존 inline 구조 정리
|
|
- [ ] `apps/node/internal/adapters/cli/cli_test.go` - slow first output 회귀 테스트 추가
|
|
|
|
#### 테스트 작성
|
|
|
|
작성:
|
|
|
|
- `apps/node/internal/adapters/cli/cli_test.go` - `TestCLIExecutePersistentWaitsForSlowFirstOutput`
|
|
- `runtime.GOOS == "windows"`면 skip한다.
|
|
- persistent terminal profile command는 `sh -c 'stty -echo; while IFS= read -r line; do sleep 0.2; printf "reply:%s\n" "$line"; done'`를 사용한다.
|
|
- `ResponseIdleTimeoutMS`는 50ms처럼 첫 출력 지연보다 작게 설정한다.
|
|
- `Execute`에는 1초 timeout context를 넘긴다.
|
|
- 기대: `Execute`가 50ms에 빈 complete를 하지 않고, delta에 `reply:hello`를 포함한 뒤 complete한다.
|
|
|
|
#### 중간 검증
|
|
|
|
```bash
|
|
go test ./apps/node/internal/adapters/cli
|
|
```
|
|
|
|
기대 결과: 기존 one-shot/persistent 테스트와 slow first output 테스트가 모두 통과한다.
|
|
|
|
---
|
|
|
|
### [REVIEW_API-2] Persistent process 종료/오류를 run 실패로 전파
|
|
|
|
#### 문제
|
|
|
|
`apps/node/internal/adapters/cli/cli.go:219-227`:
|
|
```go
|
|
case out, ok := <-sess.output:
|
|
if !ok {
|
|
return sink.Emit(ctx, runtime.RuntimeEvent{
|
|
RunID: spec.RunID,
|
|
Type: runtime.EventTypeError,
|
|
Error: "persistent session process exited unexpectedly",
|
|
Timestamp: time.Now(),
|
|
})
|
|
}
|
|
```
|
|
|
|
error event를 emit하더라도 `sink.Emit`이 nil을 반환하면 `Execute`도 nil을 반환한다. `apps/node/internal/node/node.go:100-106`은 `Execute`가 non-nil error를 반환할 때만 run status를 failed로 저장하므로, process exit가 completed run으로 저장된다.
|
|
|
|
`apps/node/internal/adapters/cli/cli.go:253-267`:
|
|
```go
|
|
case err := <-sess.done:
|
|
msg := "cli execution complete"
|
|
if err != nil {
|
|
msg = fmt.Sprintf("process exited: %v", err)
|
|
}
|
|
return sink.Emit(ctx, runtime.RuntimeEvent{
|
|
RunID: spec.RunID,
|
|
Type: runtime.EventTypeComplete,
|
|
```
|
|
|
|
persistent process가 exit status를 반환해도 complete event로 전송된다. persistent session은 재사용이 전제이므로 process 종료는 정상 complete가 아니라 session failure이다.
|
|
|
|
또한 `apps/node/internal/adapters/cli/cli.go:334-346`은 startup drain 도중 process가 이미 종료되어도 session을 반환할 수 있다.
|
|
|
|
#### 해결 방법
|
|
|
|
persistent session failure를 처리하는 helper를 추가한다.
|
|
|
|
```go
|
|
func emitRuntimeError(ctx context.Context, sink runtime.EventSink, runID, msg string) error {
|
|
_ = sink.Emit(ctx, runtime.RuntimeEvent{
|
|
RunID: runID,
|
|
Type: runtime.EventTypeError,
|
|
Error: msg,
|
|
Timestamp: time.Now(),
|
|
})
|
|
return fmt.Errorf("cli adapter: %s", msg)
|
|
}
|
|
```
|
|
|
|
output channel close, `sess.done` 수신, prompt write failure는 error event를 emit하고 non-nil error를 반환한다. `sess.done`에서 `err == nil`이어도 persistent session이 종료되어 재사용 불가능하므로 error로 처리한다.
|
|
|
|
Before (`apps/node/internal/adapters/cli/cli.go:253-267`):
|
|
```go
|
|
case err := <-sess.done:
|
|
msg := "cli execution complete"
|
|
if err != nil {
|
|
msg = fmt.Sprintf("process exited: %v", err)
|
|
}
|
|
return sink.Emit(ctx, runtime.RuntimeEvent{
|
|
Type: runtime.EventTypeComplete,
|
|
Message: msg,
|
|
})
|
|
```
|
|
|
|
After:
|
|
```go
|
|
case err := <-sess.done:
|
|
msg := "persistent session process exited"
|
|
if err != nil {
|
|
msg = fmt.Sprintf("persistent session process exited: %v", err)
|
|
}
|
|
return emitRuntimeError(ctx, sink, spec.RunID, msg)
|
|
```
|
|
|
|
`startProfileSession`은 `drainUntilIdle` 이후 non-blocking으로 `doneCh`를 확인한다. 이미 종료된 process면 close/kill 정리를 하고 error를 반환해 node startup이 실패하도록 한다.
|
|
|
|
```go
|
|
if profile.StartupIdleTimeoutMS > 0 {
|
|
drainUntilIdle(outputCh, time.Duration(profile.StartupIdleTimeoutMS)*time.Millisecond, logger, name)
|
|
}
|
|
select {
|
|
case err := <-doneCh:
|
|
_ = closeFn()
|
|
if cmd.Process != nil {
|
|
_ = cmd.Process.Kill()
|
|
}
|
|
if err == nil {
|
|
return nil, fmt.Errorf("process exited during startup")
|
|
}
|
|
return nil, fmt.Errorf("process exited during startup: %w", err)
|
|
default:
|
|
}
|
|
```
|
|
|
|
#### 수정 파일 및 체크리스트
|
|
|
|
- [ ] `apps/node/internal/adapters/cli/cli.go` - persistent failure helper 추가
|
|
- [ ] `apps/node/internal/adapters/cli/cli.go` - output close, `sess.done`, write failure에서 error event + non-nil error 반환
|
|
- [ ] `apps/node/internal/adapters/cli/cli.go` - startup drain 이후 이미 종료된 process를 start error로 반환
|
|
- [ ] `apps/node/internal/adapters/cli/cli_test.go` - process exit/error 회귀 테스트 추가
|
|
|
|
#### 테스트 작성
|
|
|
|
작성:
|
|
|
|
- `apps/node/internal/adapters/cli/cli_test.go` - `TestCLIExecutePersistentProcessExitReturnsError`
|
|
- command는 `sh -c 'stty -echo; while IFS= read -r line; do printf "before-exit\n"; exit 2; done'`를 사용한다.
|
|
- `Execute`가 non-nil error를 반환하는지, emitted events에 `error`가 포함되고 마지막 event가 `complete`가 아닌지 확인한다.
|
|
- `apps/node/internal/adapters/cli/cli_test.go` - `TestCLIStartPersistentReturnsErrorWhenProcessExitsDuringStartup`
|
|
- command는 `sh -c 'exit 2'`, `Persistent=true`, `Terminal=true`, `StartupIdleTimeoutMS=50`으로 구성한다.
|
|
- `Start`가 error를 반환하는지 확인한다.
|
|
|
|
#### 중간 검증
|
|
|
|
```bash
|
|
go test ./apps/node/internal/adapters/cli
|
|
```
|
|
|
|
기대 결과: process exit/error 테스트가 실패 없이 통과하고, 기존 persistent happy path도 유지된다.
|
|
|
|
---
|
|
|
|
### [REVIEW_API-3] Bootstrap에서 adapter start 이후 실패 cleanup 보장
|
|
|
|
#### 문제
|
|
|
|
`apps/node/internal/bootstrap/module.go:50-63`:
|
|
```go
|
|
if err := reg.Start(ctx); err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: start adapters: %w", err)
|
|
}
|
|
|
|
dsn, err := storeDSN(result.Config.GetRuntime().GetWorkspaceRoot())
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return err
|
|
}
|
|
st, err = store.New(dsn, logger)
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: store: %w", err)
|
|
}
|
|
```
|
|
|
|
adapter lifecycle을 store 초기화보다 먼저 시작한다. 이후 `storeDSN` 또는 `store.New`가 실패하면 session만 닫고 `reg.Stop`을 호출하지 않아 persistent CLI process가 남을 수 있다. 원래 plan은 store 생성 후 handler attach 전에 `reg.Start(ctx)`를 호출하도록 했다.
|
|
|
|
#### 해결 방법
|
|
|
|
`reg.Start(ctx)`를 store 생성 이후, router/node 생성 이전으로 옮긴다. `reg.Start` 실패 시 session과 store를 모두 닫는다. 이 순서면 store 초기화 실패 전에 persistent process가 시작되지 않는다.
|
|
|
|
Before (`apps/node/internal/bootstrap/module.go:44-63`):
|
|
```go
|
|
reg, err = adapters.BuildFromPayload(result.Config, logger)
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: build adapters: %w", err)
|
|
}
|
|
|
|
if err := reg.Start(ctx); err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: start adapters: %w", err)
|
|
}
|
|
|
|
dsn, err := storeDSN(result.Config.GetRuntime().GetWorkspaceRoot())
|
|
...
|
|
st, err = store.New(dsn, logger)
|
|
```
|
|
|
|
After:
|
|
```go
|
|
reg, err = adapters.BuildFromPayload(result.Config, logger)
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: build adapters: %w", err)
|
|
}
|
|
|
|
dsn, err := storeDSN(result.Config.GetRuntime().GetWorkspaceRoot())
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return err
|
|
}
|
|
st, err = store.New(dsn, logger)
|
|
if err != nil {
|
|
_ = result.Session.Close()
|
|
return fmt.Errorf("bootstrap: store: %w", err)
|
|
}
|
|
|
|
if err := reg.Start(ctx); err != nil {
|
|
_ = result.Session.Close()
|
|
_ = st.Close()
|
|
return fmt.Errorf("bootstrap: start adapters: %w", err)
|
|
}
|
|
```
|
|
|
|
#### 수정 파일 및 체크리스트
|
|
|
|
- [ ] `apps/node/internal/bootstrap/module.go` - `reg.Start(ctx)`를 store 생성 이후로 이동
|
|
- [ ] `apps/node/internal/bootstrap/module.go` - `reg.Start` 실패 시 session과 store를 모두 정리
|
|
- [ ] `apps/node/internal/bootstrap/module_test.go` - store setup 실패 전에 persistent command가 시작되지 않는지 회귀 테스트 추가
|
|
|
|
#### 테스트 작성
|
|
|
|
작성:
|
|
|
|
- `apps/node/internal/bootstrap/module_test.go` - `TestModuleDoesNotStartAdaptersBeforeStoreReady`
|
|
- 기존 `apps/node/internal/transport/integration_test.go`의 mock edge server 패턴을 참고한다.
|
|
- fake edge는 registration response로 `NodeConfigPayload`를 반환하되, runtime `WorkspaceRoot`는 이미 존재하는 파일 경로로 설정해 `storeDSN`의 `os.MkdirAll`이 실패하게 한다.
|
|
- payload에는 `cli` persistent profile을 넣고 command는 `sh -c 'touch "$MARKER"; sleep 30'`, env는 `MARKER=<temp marker path>`로 설정한다.
|
|
- `fx.New(Module(cfg))` 또는 동등한 fx lifecycle start를 실행해 startup error를 확인한다.
|
|
- 기대: error가 발생하고 marker file이 생성되지 않는다. 현재 구현처럼 `reg.Start`가 store 이전이면 marker가 생성되어 테스트가 실패한다.
|
|
- Unix shell이 필요한 경우 `runtime.GOOS == "windows"`에서 skip한다.
|
|
|
|
#### 중간 검증
|
|
|
|
```bash
|
|
go test ./apps/node/internal/bootstrap ./apps/node/internal/adapters/cli ./apps/node/...
|
|
```
|
|
|
|
기대 결과: bootstrap cleanup 회귀 테스트와 node package 테스트가 모두 통과한다.
|
|
|
|
## 수정 파일 요약
|
|
|
|
| 파일 경로 | 작업 항목 |
|
|
|-----------|-----------|
|
|
| `apps/node/internal/adapters/cli/cli.go` | [REVIEW_API-1], [REVIEW_API-2] |
|
|
| `apps/node/internal/adapters/cli/cli_test.go` | [REVIEW_API-1], [REVIEW_API-2] |
|
|
| `apps/node/internal/bootstrap/module.go` | [REVIEW_API-3] |
|
|
| `apps/node/internal/bootstrap/module_test.go` | [REVIEW_API-3] |
|
|
|
|
## 최종 검증
|
|
|
|
```bash
|
|
go test ./apps/node/internal/adapters/cli
|
|
go test ./apps/node/internal/bootstrap ./apps/node/...
|
|
go test ./...
|
|
```
|
|
|
|
기대 결과: 모든 테스트가 통과하고, persistent CLI slow-first-output/process-exit/bootstrap-cleanup 회귀 테스트가 실패 없이 동작한다.
|